On RISC-V, small data objects are put in the `.sdata` section by
default. This causes the `reorder-data-writable-ptload.c` test to fail
since it hard-codes the section to optimize to `.data`.
This patch passes the `-fPIC -pie` flags to clang to ensure the objects
are added to `.data` on RISC-V.
RISC-V supports mapping syms for code that encode the exact ISA for
which the code is valid. They have the form `$x<ISA>` where `<ISA>` is
the textual encoding of an ISA specification.
BOLT currently doesn't recognize these mapping symbols causing many
binaries compiled with newer versions of GCC (which emits them) to not
be properly processed. This patch makes sure BOLT recognizes them as
code markers.
Note that LLVM does not emit these kinds of mapping symbols yet so the
test is based on a binary produced by GCC.
Currently minimal alignment of function is hardcoded to 2 bytes.
Add 2 more cases:
1. In case BF is data in code return the alignment of CI as minimal
alignment
2. For aarch64 and riscv platforms return the minimal value of 4 (added
test for aarch64)
Otherwise fallback to returning the 2 as it previously was.
This could happen, for example, when instrumenting an AArch64 binary on
an x86 host because the instrumentation library is always built for the
host.
Note that this check will probably need to be refined in the future as
merely having the same architecture does not guarantee objects can be
linked. For example, on RISC-V, the float ABI of all objects should
match.
In instruction encoding, the relative offset address of the PC is
signed, that is, the number of positive offset bits and the number of
negative offset bits is asymmetric. Therefore, the maximum and minimum
values are used to replace Mask to determine the boundary.
Co-authored-by: qijitao <qijitao@hisilicon.com>
On RISC-V, there are certain relocations that target a specific
instruction instead of a more abstract location like a function or basic
block. Take the following example that loads a value from symbol `foo`:
```
nop
1: auipc t0, %pcrel_hi(foo)
ld t0, %pcrel_lo(1b)(t0)
```
This results in two relocation:
- auipc: `R_RISCV_PCREL_HI20` referencing `foo`;
- ld: `R_RISCV_PCREL_LO12_I` referencing to local label `1` which points
to the auipc instruction.
It is of utmost importance that the `R_RISCV_PCREL_LO12_I` keeps
referring to the auipc instruction; if not, the program will fail to
assemble. However, BOLT currently does not guarantee this.
BOLT currently assumes that all local symbols are jump targets and
always starts a new basic block at symbol locations. The example above
results in a CFG the looks like this:
```
.BB0:
nop
.BB1:
auipc t0, %pcrel_hi(foo)
ld t0, %pcrel_lo(.BB1)(t0)
```
While this currently works (i.e., the `R_RISCV_PCREL_LO12_I` relocation
points to the correct instruction), it has two downsides:
- Too many basic blocks are created (the example above is logically only
one yet two are created);
- If instructions are inserted in `.BB1` (e.g., by instrumentation),
things will break since the label will not point to the auipc anymore.
This patch proposes to fix this issue by teaching BOLT to track labels
that should always point to a specific instruction. This is implemented
as follows:
- Add a new annotation type (`kLabel`) that allows us to annotate
instructions with an `MCSymbol *`;
- Whenever we encounter a relocation type that is used to refer to a
specific instruction (`Relocation::isInstructionReference`), we
register it without a symbol;
- During disassembly, whenever we encounter an instruction with such a
relocation, create a symbol for its target and store it in an offset
to symbol map (to ensure multiple relocations referencing the same
instruction use the same label);
- After disassembly, iterate this map to attach labels to instructions
via the new annotation type;
- During emission, emit these labels right before the instruction.
I believe the use of annotations works quite well for this use case as
it allows us to reliably track instruction labels. If we were to store
them as offsets in basic blocks, it would be error prone to keep them
updated whenever instructions are inserted or removed.
I have chosen to add labels as first-class annotations (as opposed to a
generic one) because the documentation of `MCAnnotation` suggests that
generic annotations are to be used for optional metadata that can be
discarded without affecting correctness. As this is not the case for
labels, a first-class annotation seemed more appropriate.
Long tail calls use the following instruction sequence on RISC-V:
```
1: auipc xi, %pcrel_hi(sym)
jalr zero, %pcrel_lo(1b)(xi)
```
Since the second instruction in isolation looks like an indirect branch,
this confused BOLT and most functions containing a long tail call got
marked with "unknown control flow" and didn't get optimized as a
consequence.
This patch fixes this by detecting long tail call sequence in
`analyzeIndirectBranch`. `FixRISCVCallsPass` also had to be updated to
expand long tail calls to `PseudoTAIL` instead of `PseudoCALL`.
Besides this, this patch also fixes a minor issue with compressed tail
calls (`c.jr`) not being detected.
Note that I had to change `BinaryFunction::postProcessIndirectBranches`
slightly: the documentation of `MCPlusBuilder::analyzeIndirectBranch`
mentions that the [`Begin`, `End`) range contains the instructions
immediately preceding `Instruction`. However, in
`postProcessIndirectBranches`, *all* the instructions in the BB where
passed in the range. This made it difficult to find the preceding
instruction so I made sure *only* the preceding instructions are passed.
Handle the following relocations related to TLS local-exec and
initial-exec:
- R_RISCV_TLS_GOT_HI20
- R_RISCV_TPREL_HI20
- R_RISCV_TPREL_ADD
- R_RISCV_TPREL_LO12_I
- R_RISCV_TPREL_LO12_S
In addition, GNU ld has a quirk where after TLS le relaxation, two
unofficial relocation types may be emitted:
- R_RISCV_TPREL_I
- R_RISCV_TPREL_S
Since they are unofficial (defined in the reserved range of relocation
types), LLVM does not define them. Hence, I've defined them locally in
BOLT in a private namespace.
In large code model, the address of GOT is calculated by the
static linker via R_X86_GOTPC64 reloc applied against a MOVABSQ
instruction. In the final binary, it can be disassembled as a regular
immediate, but because such immediate is the result of PC-relative
pointer arithmetic, we need to parse this relocation and update this
calculation whenever we move code, otherwise we break the code trying
to read GOT.
A test case showing how GOT is accessed was provided.
Reviewed By: #bolt, maksfb
Differential Revision: https://reviews.llvm.org/D158911
The new relocation offset in .relr section patching was calculated
wrong previously. Pass the new file offset to lambda instead of
re-calculating it in it. Test removes relocation from mytext section,
so in case of wrong offset calculation we won't emit right addend value
in expected place, i.e. on the new relocation offset.
Differential Revision: https://reviews.llvm.org/D159543
This patch implements the `getCalleeSavedRegs` function for AArch64,
addressing the issue where the "not implemented" error occurs when
both the `--assume-abi` option and options related to the
RegAnalysis Pass (e.g., `--indirect-call-promotion=all`) are enabled.
According to ARMv8-a architecture reference manual B2.10.5 software
must avoid having any explicit memory accesses between exclusive load
and associated store instruction. Otherwise exclusive monitor might
clear the exclusivity without application-related cause which may
result in the deadloop. Disable instrumentation for such functions,
since between exclusive load and store there might be branches and we
would insert instrumentation snippet which contains loads and stores.
The better solution would be to analyze with BFS finding the exact BBs
between load and store and not instrumenting them. Or even better to
recognize such sequences and replace them with more complex one, e.g.
loading value non exclusively, and for the brach where exclusive store
is made make exclusive load and store sequentially, but for now just
disable instrumentation for such functions completely.
Differential Revision: https://reviews.llvm.org/D159520
`FixRISCVCallsPass` changes all different forms of calls to `PseudoCALL`
instructions. However, the original call's annotations were lost in the
process.
This patch fixes this by moving all annotations from the old to the new
call. `MCPlusBuilder::moveAnnotations` had to be made public for this.
D159460 regressed the bugfix in D156644. Fix that and emit a warning.
Add a test case.
Reviewed By: #bolt, maksfb
Differential Revision: https://reviews.llvm.org/D159529
If the R_AARCH64_CALL26 against a symbol that has a lower address, then
encodeValueAArch64 will return a wrong value.
Reviewed By: Kepontry, yota9
Differential Revision: https://reviews.llvm.org/D159513
When current state is `CFG_Finalized`, function `validateCFG()` should return true directly.
Reviewed By: maksfb, yota9, Kepontry
Differential Revision: https://reviews.llvm.org/D159410
Calls on RISC-V are typically compiled to `auipc`/`jalr` pairs to allow
a maximum target range (32-bit pc-relative). In order to optimize calls
to near targets, linker relaxation may replace those pairs with, for
example, single `jal` instructions.
To allow BOLT to freely reassign function addresses in relaxed binaries,
this patch proposes the following approach:
- Expand all relaxed calls back to `auipc`/`jalr`;
- Rely on JITLink to relax those back to shorter forms where possible.
This is implemented by detecting all possible call instructions and
replacing them with `PseudoCALL` (or `PseudoTAIL`) instructions. The
RISC-V backend then expands those and adds the necessary relocations for
relaxation.
Since BOLT generally ignores pseudo instruction, this patch makes
`MCPlusBuilder::isPseudo` virtual so that `RISCVMCPlusBuilder` can
override it to exclude `PseudoCALL` and `PseudoTAIL`.
To ensure JITLink knows about the correct section addresses while
relaxing, reassignment of addresses has been moved to a post-allocation
pass. Note that this is probably the time it had to be done in the
first place since in `notifyResolved` (where it was done before), all
symbols are supposed to be resolved already.
Depends on D159082
Reviewed By: maksfb
Differential Revision: https://reviews.llvm.org/D159089
When linker relaxation is enabled on RISC-V, every branch has a relocation and a
corresponding symbol in the symbol table. BOLT currently registers all these
symbols as secondary entry points causing almost every function to be marked as
multi entry on RISC-V.
This patch modifies `adjustFunctionBoundaries` to ignore these symbols.
Note that I currently try to detect them by checking if a symbol's name
starts with the private label prefix as defined by `MCAsmInfo`. Since
I'm not entirely sure what multi-entry functions look like on different
targets, please check if this condition is correct. Maybe it could make
sense to only check this on RISC-V?
Reviewed By: maksfb
Differential Revision: https://reviews.llvm.org/D159285
Fix tests that are failing in cross-compilation after D151920
(https://lab.llvm.org/buildbot/#/builders/221/builds/17715):
- instrumentation-ind-call, basic-instrumentation: add -mno-outline-atomics flag to runtime lib
- bolt-address-translation-internal-call, internal-call-instrument: add %cflags
- meta-merge-fdata: restrict to x86_64
Reviewed By: Amir
Differential Revision: https://reviews.llvm.org/D159094
Since the issue with trap value is fixed in D158191, it now should pass
on both platforms.
Reviewed By: maksfb
Differential Revision: https://reviews.llvm.org/D158899
`MCInstrDesc` provides the `mayLoad` and `mayStore` flags that seem
appropriate to use as a target-independent way to implement `isLoad` and
`isStore`.
I believe this is currently good enough to use for the RISC-V target as
well. I've provided a test for this that checks the generated dyno
stats (which seems to be the only thing both `isLoad` and `isStore` are
used for).
Reviewed By: maksfb
Differential Revision: https://reviews.llvm.org/D159266
Since the test executes instrumented version of the binary, move it under
runtime/X86. Note that it can be adjusted to also run under AArch64 now that
instrumentation is supported.
Reviewed By: #bolt, maksfb
Differential Revision: https://reviews.llvm.org/D159298
Fine-tuning hash computation for stale matching:
- introducing a new "loose" basic block hash that allows to match many more blocks than before;
- tweaking params of the inference algorithm that find (slightly) better solutions;
- added more meaningful tests for stale matching.
Tested the changes on several open-source benchmarks (clang, rocksdb, chrome)
and one prod workload using different compiler modes (LTO/PGO etc). There is
always an improvement in the quality of inferred profiles.
(The current implementation is still not optimal but the diff is a step forward;
I am open to further suggestions)
Reviewed By: Amir
Differential Revision: https://reviews.llvm.org/D156278
The relationship of X86 registers is shown in the diagram. BL and BH do
not have a direct alias relationship. However, if the BH register cannot be
swapped, then the BX/EBX/RBX registers cannot be swapped as well, which
means that BL register also cannot be swapped. Therefore, in the presence
of BX/EBX/RBX registers, BL and BH have an alias relationship.
┌────────────────┐
│ RBX │
├────┬───────────┤
│ │ EBX │
├────┴──┬────────┤
│ │ BX │
├───────┼───┬────┤
│ │BH │BL │
└───────┴───┴────┘
Reviewed By: rafauler
Differential Revision: https://reviews.llvm.org/D155098
The implementation is based on the X86 version, with the same code
of symbol and addend extraction. The differences include the
support for RelType `R_AARCH64_CALL26` and the deletion of 8-bit
relocation.
Reviewed By: rafauler
Differential Revision: https://reviews.llvm.org/D156018
I noticed that `-reorder-functions=exec-count` doesn't work as expected due to
a bug in the comparison function (which isn't symmetric). It is questionable
whether anyone would want to ever use the sorting method (as sorting by say
density is much better in all cases) but it is probably better to fix the bug.
Reviewed By: Amir
Differential Revision: https://reviews.llvm.org/D152959
Compiler can generate DIE References that are invalid. Previously BOLT could
assert when writing out IR to .debug_info. Changed where DIE offsets are changed
so that it's always done. Thus making sure that assert is not triggered.
Added more specific warnings, and ability to print out invalid referenced DIE
offset when verbosity >=1.
Reviewed By: Amir
Differential Revision: https://reviews.llvm.org/D157746
This bug crept in when CU partitioning was introduced. It manifests itself when
there are CUs that use location lists that come before CUs that are part of
thin-lto. BOLT processes CUs with cross CU references first (these are produced
by thin-lto). When we wrote out all the location lists we did it in original
order. Since DWARF4 uses offsets directly in to .debug_loc those offsets in DIEs
became wrong.
Reviewed By: Amir
Differential Revision: https://reviews.llvm.org/D157908
Changed to creating a new index all the time. This code was legacy of when we
couldn't change the size of .debug_info, and led to subtle bugs where index for
new entries was pointing to a wrong address.
Reviewed By: Amir
Differential Revision: https://reviews.llvm.org/D157356
Changed to creating a new index all the time. This code was legacy of when we
couldn't change the size of .debug_info, and led to subtle bugs where index for
new entries was pointing to a wrong address.
Reviewed By: Amir
Differential Revision: https://reviews.llvm.org/D157355
Fixed a bug where when Skelton CU had DW_AT_ranges, it the output CU DW_AT_ranges
offset was relative, and not absolute.
Reviewed By: maksfb
Differential Revision: https://reviews.llvm.org/D156958
