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
As discussed in D159266, for some instructions it's impossible to know
statically if they will load/store (e.g., predicated instructions).
Therefore, mayLoad/mayStore are more appropriate names.
`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
If `Itr` is the last element and then `std::next(Itr)` will be
`Range.end()`, so that the statement `std::next(Itr)->second` is
a UB.
Reviewed By: yota9, maksfb
Differential Revision: https://reviews.llvm.org/D159177
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
BOLT uses `MCAsmLayout` to calculate the output values of functions and
basic blocks. This means output values are calculated based on a
pre-linking state and any changes to symbol values during linking will
cause incorrect values to be used.
This issue can be triggered by enabling linker relaxation on RISC-V.
Since linker relaxation can remove instructions, symbol values may
change. This causes, among other things, the symbol table created by
BOLT in the output executable to be incorrect.
This patch solves this issue by using `BOLTLinker` to get symbol values
instead of `MCAsmLayout`. This way, output values are calculated based
on a post-linking state. To make sure the linker can update all
necessary symbols, this patch also makes sure all these symbols are not
marked as temporary so that they end-up in the object file's symbol
table.
Note that this patch only deals with symbols of binary functions
(`BinaryFunction::updateOutputValues`). The technique described above
turned out to be too expensive for basic block symbols so those are
handled differently in D155604.
Reviewed By: maksfb
Differential Revision: https://reviews.llvm.org/D154604
When parsing AddressMap and there is a conflict in keys,
where two entries share the same key, consider the first entry as the
correct one, instead of the last. This matches previous behavior in
BOLT and covers case such as BOLT creating a new basic block but
sharing the same input offset of the previous (or entry) basic
block. In this case, instead of translating debuginfo to use the newly
created BB, translate using the BB that was originally read from
input. This will increase our chances of getting debuginfo right.
Tested via binary comparison in tests:
X86/dwarf4-df-input-lowpc-ranges.test
X86/dwarf5-df-input-lowpc-ranges.test
Reviewed By: #bolt, maksfb, jobnoorman
Differential Revision: https://reviews.llvm.org/D158686
This commit adds support for AArch64 in instrumentation runtime library,
including AArch64 system calls.
Also this commit divides syscalls into target-specific files.
Reviewed By: rafauler, yota9
Differential Revision: https://reviews.llvm.org/D151942
This commit adds support for generation of getter counters for AArch64 MacOS.
Continuation of work D151899
Reviewed By: rafauleir, yota9
Differential Revision: https://reviews.llvm.org/D151901
This commit adds code generation for AArch64 instrumentation,
including direct and indirect calls support.
Reviewed By: rafauler, yota9
Differential Revision: https://reviews.llvm.org/D151899
The trap value used by BOLT was assumed to be single-byte instruction.
It made some functions unaligned on AArch64(e.g exceptions-instrumentation test)
and caused emission failures. Fix that by changing fill value to StringRef.
Reviewed By: rafauler
Differential Revision: https://reviews.llvm.org/D158191
Because indirect call tables use static addresses for call sites, but pc
values recorded by runtime may be subject to ASLR in PIE, we couldn't
find indirect call descriptions by their runtime address in PIE. It
resulted in [unknown] entries in profile for all indirect calls. We need
to substract base address of .text from runtime addresses to get the
corresponding static addresses. Here we create a getter for base address
of .text and substract it's return value from recorded PC values. It
converts them to static addresses, which then may be used to find the
corresponding indirect call descriptions.
Reviewed By: rafauler
Differential Revision: https://reviews.llvm.org/D154121
When a binary is instrumented with --instrumentation-sleep-time and
instrumentation-wait-forks options and lauched, the profile is
periodically written until all the forks die. The problem is that we
cannot wait for the whole process tree, and we have no way to tell when
it's safe to read the profile. Hovewer, if we keep profile open
throughout the life of the process tree, we can use fuser to determine
when writing is finished.
Reviewed By: rafauler
Differential Revision: https://reviews.llvm.org/D154436
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
This commit splits the createRelocation function for the X86 architecture
into two parts, retaining the first half and moving the second half to a
new function called extractFixupExpr. The purpose of this change is to make
extractFixupExpr a shared function between AArch64 and X86 architectures,
increasing code reusability and maintainability.
Child revision: https://reviews.llvm.org/D156018
Reviewed By: Amir
Differential Revision: https://reviews.llvm.org/D157217
BOLT uses MCAsmLayout to calculate the output values of basic blocks.
This means output values are calculated based on a pre-linking state and
any changes to symbol values during linking will cause incorrect values
to be used.
This issue was first addressed in D154604 by adding all basic block
symbols to the symbol table for the linker to resolve them. However, the
runtime overhead of handling this huge symbol table turned out to be
prohibitively large.
This patch solves the issue in a different way. First, a temporary
section containing [input address, output symbol] pairs is emitted to the
intermediary object file. The linker will resolve all these references
so we end up with a section of [input address, output address] pairs.
This section is then parsed and used to:
- Replace BinaryBasicBlock::OffsetTranslationTable
- Replace BinaryFunction::InputOffsetToAddressMap
- Update BinaryBasicBlock::OutputAddressRange
Note that the reason this is more performant than the previous attempt
is that these symbol references do not cause entries to be added to the
symbol table. Instead, section-relative references are used for the
relocations.
Reviewed By: maksfb
Differential Revision: https://reviews.llvm.org/D155604
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
This is purely to make debugging easier for developers. Now that we moved to IR
the print out of DIEs is lacking. This function will lazily parse DIE and use
DWARFDie dump function.
Reviewed By: Amir
Differential Revision: https://reviews.llvm.org/D157354
Solve the endless loop caused by iterative guess. The main function of this option is guessEdgeByIterativeApproach, where the do while loop involves guessPredEdgeCounts and guessSuccessEdgeCounts. In some scenarios, the do while loop will fall into an endless loop. The reason is that although the GuessedPredEdgeCounts function has guessed the pred-edges counts, GuessedArcs does not insert the corresponding BB block, resulting in the changed variable always being true.
Reviewed By: rafauler
Differential Revision: https://reviews.llvm.org/D154922
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
Now that we have new DWARF Rewriter we can remove DW_AT_low_pc when converting
DW_AT_low_pc/DW_AT_high_pc to DW_AT_ranges. Which closer follows DWARF spec.
Leaving CU DW_AT_low_pc in place. Reading the spec I think it's needed.
Reviewed By: maksfb
Differential Revision: https://reviews.llvm.org/D156957
Clang can generate DW_TAG_inlined_subroutine with low_pc 0. With split dwarf
this led to range offset being a negative number.
Reviewed By: maksfb
Differential Revision: https://reviews.llvm.org/D156742
BOLT-ERROR and BOLT-WARNING messages are output to stderr which is not captured
by piping to FileCheck. Redirect stderr to stdout to fix that in tests.
Reviewed By: #bolt, maksfb
Differential Revision: https://reviews.llvm.org/D156340
Use short loop instead of duplicating the code for setHasProfileAvailable.
Reviewed By: #bolt, maksfb
Differential Revision: https://reviews.llvm.org/D154749
We identify instructions to be instrumented based on Offset annotation.
BOLT "expands" conditional tail calls into a conditional jump to a basic block
with unconditional tail call. Move Offset annotation from former CTC to the tail
call.
For expanded CTC we keep Offset attached to the original instruction which is
converted into a regular conditional jump, while leaving the newly created tail
call without an Offset annotation. This leads to attempting the instrumentation
of the conditional jump which points to the basic block with an inherited input
offset thus creating an invalid edge description. At the same time, the newly
created tail call is skipped entirely which means we're not creating a call
description for it.
If we instead reassign Offset annotation from the conditional jump to the tail
call we fix both issues. The conditional jump will be skipped not creating an
invalid edge description, while tail call will be handled properly (unformly
with regular calls).
Reviewed By: #bolt, maksfb
Differential Revision: https://reviews.llvm.org/D156389
Work around the issue of multiple profiles per function.
Can happen with a stale profile which has separate profiles
that in a new binary got merged and became aliases.
Reviewed By: #bolt, maksfb
Differential Revision: https://reviews.llvm.org/D156644
This is a new algorithm for function layout (reordering) based on the call graph
extracted from a profile data; see diffs down the stack for more details.
This layout is very similar to the existing hfsort+, but perhaps a little better
on some benchmarks. The goals of the change is as follows:
(i) rename and replace hfsort+ with a newer (hopefully better) implementation.
I'd prefer to keep both algs together for some time to simplify evaluation and
transition, but do want to remove hfsort+ once we're confident that there are
no regressions.
(ii) unify the implementation of code layout algorithms across LLVM. Currently
Passes/HfsortPlus.cpp and Utils/CodeLayout.cpp share many implementation-specific
details; this diff unifies the code.
Reviewed By: Amir
Differential Revision: https://reviews.llvm.org/D153039
When output range is only one entry, and input is low_pc/high_pc do not convert
to ranges. This helps with size of .debug_ranges/.debug_rnglists. It also helps
when either low_pc/high_pc is 0. We not generating potentially invalid ranges
that result in LLDB error.
Also fixed handling of DW_AT_subprogram with ranges. This can be created with
-fbasic-block-sections=all.
Reviewed By: maksfb
Differential Revision: https://reviews.llvm.org/D156374
