Summary:
This diff change reorderBasicBlocks pass to run in parallel,
it does so by adding locks to the fix branches function,
and creating temporary MCCodeEmitters when estimating basic block code size.
(cherry picked from FBD16161149)
Summary:
If two indirect branches use the same jump table, we need to
detect this and duplicate dump tables so we can modify this CFG
correctly. This is necessary for instrumentation and shrink wrapping.
For the latter, we only detect this and bail, fixing this old known
issue with shrink wrapping.
Other minor changes to support better instrumentation: add an option
to instrument only hot functions, add LOCK prefix to instrumentation
increment instruction, speed up splitting critical edges by avoiding
calling recomputeLandingPads() unnecessarily.
(cherry picked from FBD16101312)
Summary:
Heuristic that creates a jump table for every memory access,
including those we do not match against a pattern in an indirect jump,
is too permissive and has false positives. Guard this logic under
strict mode until we figure out a better strategy.
(cherry picked from FBD16192205)
Summary:
Each time we run some work in parallel over the list of functions in bolt, we manage a thread pool, task scheduling and perform some work to manage the granularity of the tasks based on the type of the work we do.
In this task, I am creating an interface where all those details are abstracted out, the user provides the function that will run on each function, and some policy parameters that setup the scheduling and granularity configurations.
This will make it easier to implement parallel tasks, and eliminate redundant coding efforts.
(cherry picked from FBD16116077)
Summary:
This diff parallelize the STPClean() function reducing its runtime from 5 seconds to 0.4 on HHVM,
Making the runtime for the frame optimizer goes down to 33 seconds on HHVM.
(cherry picked from FBD15914371)
Summary:
This diff includes two main changes:
1) When creating an annotation, a dedicated annotation allocator can be used, instead of the default allocator. This allows some annotation to be deallocated right after the end of their usage completely. Furthermore, having the ability to use dedicated allocators allows running SPT in parallel where each task uses a different allocator.
2) SPT is parallelized.
(cherry picked from FBD15913492)
Summary: We select the top hot targets for indirect call promotion. But since we only have frequency for targets, not for actual jump table indices, we have to merge indices that share the same actual target. In order to do that we sort targets by pointer of target symbol before merging, which introduces instability. Later we stable sort merged targets by frequency. Due to the instability of sorting pointers, and depending on how many indices each merged target has, we could end up with unstable ICP. This commit changes the 2nd pass sorting to prioritize targets with fewer indices, and higher mispredicts, in addition to higher frequency. It improves stability of ICP, and also exposes more ICP because targets with fewer indices has better chance of getting promoted.
(cherry picked from FBD16099701)
Summary:
Check that a symbol address is less than the next function
address before considering it for a secondary entry.
(cherry picked from FBD16056468)
Summary:
In strict relocation mode we rely on relocations to represent all
possible entry points into a function. Most of the code generated by
tested compilers (gcc and clang) will result in relocations against
any internal labels for jump tables and for computed goto tables.
In situations where we cannot properly reconstruct a jump table, or when
we cannot determine a table that guides an indirect jump, e.g. when
multiple computed goto tables are used, we conservatively assume that
the indirect jump can end up at any possible basic block referenced by
relocations.
In strict mode, simple functions may include the aforementioned
instructions with unknown control flow with a conservative list of
destinations added to the containing basic block. This allows us to
expand coverage of simple functions and to enable code reordering
optimizations for more functions.
The strict mode is recommended when BOLT is used with a well-formed
code generated by a compiler.
To use the strict mode, add "-strict" on the command line.
Another effect of this diff, is that with relocations, we will always
replace the immediate operand of an instruction with a symbol if the
relocation exists against this operand.
Also this diff fixes issues with Clang compiled with -fpic.
(cherry picked from FBD15872849)
Summary:
A relocation can have an addend that makes it look as the relocated
value is in a different section from the symbol being relocated.
E.g., a relocation against a variable in .rodata could have a negative
offset that will make it look like it is against a symbol in .text
(a section that typically precedes .rodata).
Unless the relocation is against a section symbol, we know
exactly the symbol that is being relocated and there is no issue.
However, when the linker leaves only a section relocation (i.e. a
relocation against a section symbol when a temporary original symbol
gets deleted), we have to guess the relocated symbol, and can falsely
detect a function reference in the case described above.
The fix is to keep a section relocation if the corresponding
relocated value falls into a different section, and to detect and
ignore false function reference.
(cherry picked from FBD16030791)
Summary: BOLT operates in relocation mode by default when .reloc is in the binary. This changes disables relocation mode for heatmap generation so we can use that for more cases. There's a small separate change that ignores zero-sized symbol in zero-sized code section during function discovery.
(cherry picked from FBD16009610)
Summary:
An instrumentation pass that modifies the input binary to
generate a profile after execution finishes. It modifies branches to
increment counters stored in the process memory and injects a new
function that dumps this data to an fdata file, readable by BOLT.
This instrumentation is experimental and currently uses a naive
approach where every branch is instrumented. This is not ideal for
runtime performance, but should be good enough for us to
evaluate/debug LBR profile quality against instrumentation.
Does not support instrumenting indirect calls yet, only direct
calls, direct branches and indirect local branches.
(cherry picked from FBD15998096)
Summary:
Make BOLT ignore empty functions (those containing no instructions,
despite having some space allocated to it filled with zeroes).
(cherry picked from FBD15981683)
Summary:
Profile bias may happen depending on the hardware counter used
to trigger LBR sampling, on the hardware implementation and as an
intrinsic characteristic of relying on LBRs. Since we infer fall-through
execution and these non-taken branches take zero hardware resources to
be represented, LBR-based profile likely overrepresents paths with fall
throughs and underrepresents paths with many taken branches. This patch
adds an option to print statistics about profile bias so we can better
understand these biases.
The goal is to analyze differences in the sum of the frequency of all
incoming edges in a basic block versus the sum of all outgoing. In an
ideally sampled profile, these differences should be close to zero. With
this option, the user gets the mean of these differences in flow as a
percentage of the input flow. For example, if this number is 15%, it
means, on average, a block observed 15% more or less flow going out of
it in comparison with the flow going in. We also print the standard
deviation so we can have an idea of how spread apart are different
measurements of flow differences. If variance is low, it means the
average bias is happening across all blocks, which is compatible with
using LBRs. If the variance is high, it means some blocks in the profile
have a much higher bias than others, which is compatible with using a
biased event such as cycles to sample LBRs because it overrepresents
paths that end in an expensive instruction.
(cherry picked from FBD15790517)
Summary:
ICF consumes 10-15% of bolt runtime, for HHVM that is around 45 seconds.
this diff perform some parallelization for the pass to make it faster.
A 60% reduction in the ICF runtime is measured on the parallel version for HHVM.
(cherry picked from FBD15589515)
Summary:
Now that we populate jump tables after all functions are disassembled,
we can check for instruction boundaries corresponding to jump table
entries. No need to delegate this task to postProcessJumpTables().
(cherry picked from FBD15814762)
Summary:
During the initial disassembly pass, only identify jump tables
without populating the contents. Later, after all functions have been
disassembled, we have a better idea of jump table boundaries and can do
a better job of populating their entries.
As a result, we no longer have embedded jump tables (i.e. a jump table
that is parter of another jump table). If we ever need to keep
sequential jump tables inseparable during the output, we can always
add such functionality later.
Fixesfacebookincubator/BOLT#56.
(cherry picked from FBD15800427)
Summary:
During frame analysis, the functions do not change, and stack pointer tracking
does not need to be performed more than one time.
The current implementation performs the SPT analysis multiple times per
function during the frame analysis, we ca eliminate such computation redundancy.
On HHVM with -frame-opts=hot, this save around a minute which is 40% of the
frame optimization runtime. (129s to 76s).
fdata should be passed for a reasonable evaluation (we need the call graph).
However, this comes at a memory cost, around 2G to the peak when only -frame-opt=hot only is used but,
When all the usual flags are passed, the effect is to the peak is only 200K (measured from one test).
Update:
When jemalloc is used the base became way better and the following runtime are observed:
[jemalloc]
hhvm 85 --> 72.
clang 27 --> 23.
[malloc]
hhvm 129 --> 76.
clang 34 --> 27.
(cherry picked from FBD15707003)
Summary:
Add an option to get extra profile trace using the recorded event PC.
The trace goes from the latest LBR record destination to the event PC.
(cherry picked from FBD15711804)
Summary:
We used to handle PC-relative address references differently from direct
address references. As a result, some cases, such as escaped function
label address, were not handled when dealing with absolute (non-PIC)
code. This diff moves processing of an address reference into
BinaryContext::handleAddressRef() which is called for both PIC and
non-PIC code.
(cherry picked from FBD15643535)
Summary:
Compile Bolt using std 14.
We want that to be able to use some threading the locking tools that do not exists in std 11.
(cherry picked from FBD15671736)
Summary:
Similarly to how the compiler relies on DWARF to map samples, so
it is possible to collect profile data in binaries optimized by PGO
techniques and retrofit data to be used in a representation of the program
that was not optimized by PGO, this diff implements an option in BOLT to
encode a table in the output binary that allows us to map data collected
in optimized binaries back to the address space used in the input binary
(where the profile is useful, since we do not support running BOLT on a
binary already optimized by BOLT). The goal is to offer an option to
support BOLT in scenarios where it is not easy to run a special deployment of
the binary with a version that was not optimized by BOLT just for data
collection.
This feature is enabled with the -enable-bat flag. BAT stands for BOLT
Address Translation, which refers to the process of mapping output to
input addresses.
(cherry picked from FBD15531860)
Summary:
Options such as `-print-only`, `-skip-funcs`, etc. now take regular
expressions. Internally, the option is converted to '^funcname$' form
prior to regex matching. This ensures that names without special
symbols will match exactly, i.e. "foo" will not match "foo123".
(cherry picked from FBD15551930)
Summary:
Run analyzeIndirectBranch() using basic block boundaries instead of
running ad-hoc validation of the jump table assumptions.
(cherry picked from FBD15465034)
Summary:
Move handling of interprocedural references to BinaryContext.
Post-process indirect branches immediately after the CFG is built.
This is almost NFC. Since indirect branches are now post-processed
before the profile data is processed it interferes with the way the
profile data in YAML format is handled.
(cherry picked from FBD15456003)
Summary:
Add an option:
-memcpy1-spec=func1,func2:cs1,func3:cs1:cs2,...
to specialize calls to memcpy() in listed functions (the name could be
supplied in regex) for size 1. The optimization will dynamically check
if the size argument equals to 1 and execute a one byte copy, otherwise
it will call memcpy() as usual. Specific call sites could be indicated
after ":" using their numeric count from the start of the function.
(cherry picked from FBD15428936)
Summary:
SDT markers that appears as nops in the assembly, are preserved and not eliminated.
Functions with SDT markers are also flagged. Inlining and folding are disabled for
functions that have SDT markers.
(cherry picked from FBD15379799)
Summary:
Parse statically defined tracepoints(SDT) markers from the ELF file, and store them.
Add an option to print SDTs (-print-sdt).
Add test case for parsing and printing SDTs.
(cherry picked from FBD15366712)
Summary:
Previously, ICP worked with a budget of N targets to convert to
direct calls. As long as the frequency of up to N of the hottest targets
surpassed a given fraction (threshold) of the total frequency, say, 90%,
then the optimization would convert a number of targets (up to N) to
direct calls. Otherwise, it would completely abort processing this call
site. The intent was to convert a given fraction of the indirect call
site frequency to use direct calls instead, but this ends up being a
"all or nothing" strategy.
In this patch we change this to operate with the same strategy seem in
LLVM's ICP, with two thresholds. The idea is that the hottest target of
an indirect call site will be compared against these two thresholds: one
checks its frequency relative to the total frequency of the original
indirect call site, and the other checks its frequency relative to the
remaining, unconverted targets (excluding the hottest targets that were
already converted to direct calls). The remaining threshold is typically
set higher than the total threshold. This allows us more control over
ICP.
I expose two pairs of knobs, one for jump tables and another for
indirect calls.
To improve the promotion of hot jump table indices when we have memory
profile, I also fix a bug that could cause us to promote extra indices
besides the hottest ones as seen in the memory profile. When we have the
memory profile, I reapply the dual threshold checks to the memory
profile which specifies exactly which indices are hot. I then update N,
the number of targets to be promoted, based on this new information, and
update frequency information.
To allow us to work with smaller profiles, I also created an option in
perf2bolt to filter out memory samples outside the statically allocated
area of the binary (heap/stack). This option is on by default.
(cherry picked from FBD15187832)
Summary:
Make BinaryContext responsible for creation and management of
JumpTables. This will be used for detection and resolution of jump table
conflicts across functions.
(cherry picked from FBD15196017)
Summary:
perf tool requires the input data to be owned by the current user or
root, otherwise it rejects the input. Use `-f` option to override this
behavior.
(cherry picked from FBD15160678)
Summary:
While checking for a size of a jump table, we've used containing
section as a boundary. This worked for most cases as typically jump
tables are not marked with symbol table entries. However, the compiler
may generate objects for indirect goto's.
(cherry picked from FBD15158905)
Summary:
We used to ignore debug sections by default, but we kept them in the
binary which led to invalid debug information in the output. It's better
to strip debug info and print a warning to the user.
Note: we are not updating debug info by default due to high memory
requirements for large applications.
(cherry picked from FBD15128947)
Summary:
Commit "Update symbols for secondary entry points" introduced
a bug by using getBinaryFunctionContainingAddress() instead of
getBinaryFunctionAtAddress() regarding ICF'd functions. Only the latter
would fetch the correct BinaryFunction object for addresses of functions
that were ICF'd. As a result of this bug, the dynamic symbol table was
not updated for function symbols that were folded by ICF.
(cherry picked from FBD15112941)
Summary:
In non-relocation mode we may execute multiple re-write passes either
because we need to split large functions or update debug information for
large functions (in this context large functions are functions that do
not fit into the original function boundaries after optimizations).
When we execute another pass, we reset RewriteInstance and run most of
the steps such as disassembly and profile matching for the 2nd or 3rd
time. However, when we match a profile, we check `Used` flag, and don't
use the profile for the 2nd time. Since we didn't reset the flag while
resetting the rest of the states, we ignored profile for all functions.
Resetting the flag in-between rewrite passes solves the problem.
(cherry picked from FBD15110959)
Summary:
For pre-aggregated profile, we were using the number of records in the
profile for `NumTraces` ignoring the counts per record. As a result,
the reported percentage of mismatched traces was bogus.
(cherry picked from FBD15093123)
Summary:
Enable -hot-text by default if reordering functions.
Also fail immediately if function reordering is specified on the command
line in non-relocation mode.
(cherry picked from FBD15095178)
Summary: BOLT works as a series of patches rebased onto upstream LLVM at revision `f137ed238db`. Some of these patches introduce unnecessary whitespace changes or includes. Remove these to minimize the diff with upstream LLVM.
(cherry picked from FBD15064122)
Summary:
Update the output ELF symbol table for symbols representing
secondary entry points for functions. Previously, those were left
unchanged in the symtab.
(cherry picked from FBD15010517)
Summary:
`size_t` is platform-dependent, and on macOS it is defined as
`unsigned long long`. This is not the same type as is used in many calls
to templated functions that expect the same type. As a result, on macOS,
calls to `std::max` fail because a template function that takes
`uint64_t, unsigned long long` cannot be found.
To work around the issue:
* Specify explicit `std::max` and `std::min` functions where necessary,
to work around the compiler trying (and failing) to find a suitable
instantiation.
* For lambda return types, specify an explicit return type where necessary.
* For `operator ==()` calls, use an explicit cast where necessary.
(cherry picked from FBD15030283)
Summary:
When attempting to build llvm-bolt with `-DLLVM_ENABLE_TARGETS="X86"`, I
encountered an error:
```
CMake Error at cmake/modules/AddLLVM.cmake:559 (add_dependencies):
The dependency target "AArch64CommonTableGen" of target
"LLVMBOLTTargetAArch64" does not exist.
Call Stack (most recent call first):
cmake/modules/AddLLVM.cmake:607 (llvm_add_library)
tools/llvm-bolt/src/Target/AArch64/CMakeLists.txt:1 (add_llvm_library)
```
The issue is that the `llvm-bolt/src/Target/AArch64` subdirectory is
added by CMake unconditionally. The LLVM project, on the other hand,
only adds the subdirectories that are enabled, by using a `foreach` loop
over `LLVM_TARGETS_TO_BUILD`. Copying that same loop, from
`llvm/lib/Target/CMakeLists.txt`, to this project avoids the error.
(cherry picked from FBD15030236)
Summary:
Iterating over SmallPtrSet is non-deterministic. Change it to
SmallSetVector. Similarly, do not sort a vector of ProgramPoint when
computing the dominance frontier, as ProgramPoint uses the pointer value
to determine order. Use a SmallSetVector there too to avoid duplicates
instead of sorting + uniqueing.
(cherry picked from FBD14992085)
Summary:
If a function size indicated in FDE is different from the one in the
symbol table, we can keep processing the function as we are using the
max size for internal purposes. Typically this happens for
assembly-written functions with padding at the end. This padding is not
included in FDE, but it is in the symbol table.
(cherry picked from FBD14987653)
Summary:
This adds very basic and limited support for split functions.
In non-relocation mode, split functions are ignored, while their debug
info is properly updated. No support in the relocation mode yet.
Split functions consist of a main body and one or more fragments.
For fragments, the main part is called their parent. Any fragment
could only be entered via its parent or another fragment.
The short-term goal is to correctly update debug information for split
functions, while the long-term goal is to have a complete support
including full optimization. Note that if we don't detect split
bodies, we would have to add multiple entry points via tail calls,
which we would rather avoid.
Parent functions and fragments are represented by a `BinaryFunction`
and are marked accordingly. For now they are marked as non-simple, and
thus only supported in non-relocation mode. Once we start building a
CFG, it should be a common graph (i.e. the one that includes all
fragments) in the parent function.
The function discovery is unchanged, except for the detection of
`\.cold\.` pattern in the function name, which automatically marks the
function as a fragment of another function.
Because of the local function name ambiguity, we cannot rely on the
function name to establish child fragment and parent relationship.
Instead we rely on disassembly processing.
`BinaryContext::getBinaryFunctionContainingAddress()` now returns a
parent function if an address from its fragment is passed.
There's no jump table support at the moment. Jump tables can have
source and destinations in both fragment and parent.
Parent functions that enter their fragments via C++ exception handling
mechanism are not yet supported.
(cherry picked from FBD14970569)
Summary:
On some platforms
`llvm::make_error_code(std::errc::no_such_process) == std::errc::no_such_process`
evaluates to false.
(cherry picked from FBD14944405)
Summary:
It's possible to pass a profile in invalid format to BOLT, and we
silently ignore it. This could cause a regression as such scenario can
go undetected. We should abort processing if no valid data was seen in
the profile and issue a warning if it was partially invalid.
(cherry picked from FBD14941211)
Summary:
If a function was already marked as non-simple, there's no reason to
issue a warning that it has a reference in the middle of an
instruction. Besides, sometimes there wouldn't be instructions
disassembled at a given entry, and the warning would be incorrect.
(cherry picked from FBD14938227)
Summary:
In binutils 2.30 a bfd linker accidentally started modifying some
relocations on output under `-q/--emit-relocs` by turning on
R_X86_64_converted_reloc_bit. As a result, BOLT ignored such
relocations and failed to correctly update the binary.
This diff filters out R_X86_64_converted_reloc_bit from the relocation
type.
(cherry picked from FBD14907832)
Summary:
For easier analysis of the hottest targets of jump tables it helps to
have basic block successors sorted based on the taken frequency.
(cherry picked from FBD14856640)
Summary:
If processing the perf.data in LBR mode but the data was
collected without -j, currently we confusingly report all samples
to mismatch the input binary, even though the samples match but
lack LBR info. Change perf2bolt to detect this scenario and print
a helpful message instructing the user to collect data with LBR.
(cherry picked from FBD14817732)
Summary:
While updating DWARF, we used to convert address ranges for functions
into DW_AT_ranges format, even if the ranges were not split and still
had a simple [low, high) form. We had to do this because functions with
contiguous ranges could be sharing an abbrev with non-contiguous range
function, and we had to convert the abbrev.
It turns out, that the excessive usage of DW_AT_ranges may lead to
internal core dumps in gdb in the presence of .gdb_index.
I still don't know the root cause of it, but reducing the number
DW_AT_ranges used by DW_TAG_subprogram DIEs does alleviate the
issue.
We can keep a simple range for DIEs that are guaranteed not to
share an abbrev with any non-contiguous function. Hence we have to
postpone the update of function ranges until we've seen all DIEs.
Note that DIEs from different compilation units could share the same
abbrev, and hence we have to process DIEs from all compilation units.
(cherry picked from FBD14814043)
Summary:
Some instructions in assembly-written functions could reference 8-byte
constants from another instructions using 4-byte offsets, presumably to
save a couple of bytes.
Detect such cases, and skip processing such functions until we teach
BOLT how to handle references into a middle of instruction.
(cherry picked from FBD14768212)
Summary:
A long due refactoring that makes interfaces cleaner and less awkward.
Mainly makes the future work way easier.
(cherry picked from FBD14766284)
Summary:
When we patch .debug_abbrev we issue many duplicate patches. Instead of
storing these patches as a vector, use a hash map. This saves some
processing time and memory.
(cherry picked from FBD14691292)
Summary:
In non-relocation mode we were accidentally emitting section headers for
every single jump table. This happened with default
`-jump-tables=basic`.
(cherry picked from FBD14653282)
Summary:
While using "-hot-text" option, we might not get enough cold text to
fill up the last huge page, and we can get data allocated on this page
producing undesirable effects. To prevent this from happening, always
make sure to allocate enough space past __hot_end.
(cherry picked from FBD14575100)
Summary:
While removing redundant local symbols, we used new section index to
lookup the corresponding section in the old section table. As a result,
we used to either not remove the correct symbols, or remove the wrong
ones.
(cherry picked from FBD14552047)
Summary:
We used to use existing symbol binding while duplicating and renaming
cold fragment symbols. As a result, some of those were emitted with
global binding. This confuses gdb, and it starts treating those symbols
as additional entry points.
The fix is to always emit such symbols with a local binding. This also
means that we have to sort static symbol table before emission to make
sure local symbols precede all others.
(cherry picked from FBD14529265)
Summary:
Create a separate pass for assigning functions to sections. Detect
functions originating from special sections (by default .stub and
.mover) and place them into ".text.mover" if "-hot-text" options is
specified.
Cold functions are isolated from hot functions even when no function
re-ordering is specified.
(cherry picked from FBD14512628)
Summary:
GDB does not like if the first entry in the line info table after
end_sequence entry is not marked with is_stmt. If this happens, it will
not print the correct line number information for such address. Note
that everything works fine starting with the first address marked
with is_stmt.
This could happen if the first instruction in the cold section wasn't
marked with is_stmt.
The fix is to always emit debug line info for the first instruction
in any function fragment with is_stmt flag.
(cherry picked from FBD14516629)
Summary:
This refactoring makes it easier to create new code sections and control
code placement. As an example, cold code is being placed into
".text.cold" which is emitted independently from ".text", and the final
address assignment becomes more flexible.
Previously, in non-relocation mode we used to emit temporary section
name into .shstrtab. This resulted in unnecessary bloat of this section.
There was unnecessary padding emitted at the end of text section. After
fixing this, the output binary becomes smaller.
I had to change the way exception handling tables are re-written
as the current infra does not support cross-section label difference.
This means we have to emit absolute landing pad addresses, which might
not work for PIE binaries. I'm going to address this once I investigate
the current exception handling issues in PIEs.
This diff temporarily disables "-hot-functions-at-end" option.
(cherry picked from FBD14475693)
Summary: As part of our heuristics to decode an indirect branch, if we
suspect the branch is an indirect tail call, we add its probable target
to the BC::InterproceduralReferences vector to detect functions with
more than one entry point. However, if this probable target is not in an
allocatable section, we were asserting. Remove this assertion and
change the code to conditionally store to InterproceduralReferences
instead. The probable target could be garbage at this point because
of analyzeIndirectBranch failing to identify the load instruction that
has the memory address of the target, so we should tolerate this.
(cherry picked from FBD14432821)
Summary:
Add heatmap subcommand to produce heatmaps based on perf.data with LBR.
The output is produced in colored ASCII format.
llvm-bolt heatmap -p perf.data <executable>
-block-size=<uint> - size of a heat map block in bytes (default 64)
-line-size=<uint> - number of entries per line (default 256)
-max-address=<uint> - maximum address considered valid for heatmap
(default 4GB)
-o=<string> - heatmap output file (default stdout)
(cherry picked from FBD13969992)
Summary:
For non-simple function we can miss a reference to a jump table or
to an indirect goto table. If we move the jump table, the missed
reference will not get updated, and the corresponding indirect jump
will end up in the old (wrong) location. Updating the original jump
table in-place should take care of the issue.
(cherry picked from FBD13849776)
Summary:
While converting perf profile, we only need CFG for functions that were
profiled and can skip building CFG for the rest. This saves us some
processing time and memory.
Breakdown processing of perf.data into two steps. The first
step parses the data, saves it in intermediate format, and marks
functions with the profile. The second step attributes the profile to
functions with CFG. When we disassemble and build CFG for functions in
aggregate-only mode, we skip functions without the profile.
(cherry picked from FBD13706697)
Summary:
Improve tracking of forked processes.
If a process corresponding to the input binary has forked/started
before 'perf record' was initiated, then the full name of the binary
will be recorded in a corresponding MMAP2 event. We've being handling
such cases well so far.
However, if the process was forked after 'perf record' has started, and
execve(2) wasn't called afterwards, then there will be no MMAP2 event
recorded corresponding to the mapping of the main binary (unrelated
MMAP2 events could still be recorded).
To track such cases, we need to parse 'perf script --show-task-events'
command output, and to scan for PERF_RECORD_FORK events, and then add
forked process PIDs to the list associated with the input binary. If
the fork event was followed by an exec event (PERF_RECORD_COMM exec)
of a different binary, then the forked PID should be ignored. If the
exec event was associated with our input binary, then the correct MMAP2
event was recorded and parsed.
To track if the event occurred before or after 'perf record', we parse
event's time. This helps us to differentiate some events. E.g. the exec
event is only registered correctly if it happened after perf recording
has started (otherwise the "exec" part is missing), and thus we only
record forks with non-zero time stamps.
(cherry picked from FBD13250904)
Summary:
Use newly added function size estimation to measure the effectiveness
and guide function splitting. Two new tuning options are added:
-split-threshold=<uint>
split function only if its main size is reduced by more than given
amount of bytes. Default value: 0, i.e. split iff the size is reduced.
Note that on some architectures the size can increase after splitting.
-split-align-threshold=<uint>
when deciding to split a function, apply this alignment while doing
the size comparison (see -split-threshold). Default value: 2.
(cherry picked from FBD13136352)
Summary:
Add BinaryContext::calculateEmittedSize() that ephemerally emits code
to allow precise estimation of the function size. Relaxation and
macro-op alignment adjustments are taken into account.
(cherry picked from FBD13092139)
Summary:
On x86 the difference between long and short jump instructions could be
either 4 or 3 bytes, depending if it's a conditional jump or not.
For a basic block with 2 jump instructions, if we know that one of
the successors is in a different code region, then we can make it
a target of an unconditional jump instruction. This will save 1 byte
in case the conditional jump happens to be a short one.
(cherry picked from FBD13078139)
Summary:
When Clang is boot-strapped with (Thin)LTO, it may produce a code
fragment similar to below:
.LFT663334 (6 instructions, align : 1)
Predecessors: .LFT663333
00000538: movb $0x1, %al
0000053a: movl %eax, -0x2c(%rbp)
0000053d: movl $"_ZN5clang6Parser12ConsumeParenEv/1", %ecx
00000542: testb $0x1, %cl
00000545: movq -0x40(%rbp), %r14
00000549: je .Ltmp1071462
Successors: .Ltmp1071462, .LFT663335
.LFT663335 (2 instructions, align : 1)
Predecessors: .LFT663334
0000054b: movq (%r12), %rax
0000054f: movq .Ltmp0(%rax), %rcx
Successors: .Ltmp1071462
.Ltmp1071462 (7 instructions, align : 1)
Predecessors: .LFT663334, .LFT663335
00000556: movq %r12, %rdi
00000559: callq *%rcx
.......
The code above is making a call by dereferencing a pointer to a member
function. A pointer to a member function could either be a regular
function, or a virtual function. To differentiate between the two, AMD64
ABI (originated from Itanium ABI) uses the last bit of the pointer. The
call instruction sequence varies depending if the function is virtual or
not, and the pointer's last bit is checked. If it's "1" then the value
of the pointer (minus 1) is used as an offset in the object vtable to
get the address of the function, otherwise the pointer is used directly
as a function address.
In this specific case, a de-virtualization is taking place, but it's not
complete. Compiler knows that the member function pointer is actually a
non-virtual function _ZN5clang6Parser12ConsumeParenEv (aka
"clang::Parser::ConsumeParen()"). However, it keeps the (dead) code that
checks the last bit of _ZN5clang6Parser12ConsumeParenEv, and furthermore
keeps the code (unreachable/dead) to make a virtual call while using
(_ZN5clang6Parser12ConsumeParenEv - 1) as an offset into the vtable.
This is obviously wrong, but since the code is unreachable, it will
never affect the runtime correctness.
The value "_ZN5clang6Parser12ConsumeParenEv - 1" falls into a last byte
of a function preceding _ZN5clang6Parser12ConsumeParenEv, and BOLT
creates a label ".Ltmp0" pointing to this last byte that is referenced
in by the instruction sequence above. It just happens that the last byte
is also in the middle of the last instruction, and as a result, BOLT
never emits the label, hence resulting in the error message "Undefined
temporary symbol".
The workaround is to detect non-pc-relative relocations from code
pointing to some (fptr - 1). Note that this is not completely
error-prone, but non-pc-relative references from code into a middle of
a function are quite rare, and chances that in a normal situation they
will point to a byte preceding some function address are virtually zero.
(cherry picked from FBD13030310)
Summary:
Special case GOT relocs to ignore addend subtracting
logic in analyzeRelocation, since the addend does not refer to the
target of the instruction being analyzed. Also make the code honor
the comments in the special case about zeroed out ExtractValue but
non-zero addend.
Fixfacebookincubator/BOLT#40
(cherry picked from FBD10355019)
Summary:
This pull request fixes two compiler warnings:
- missing `break;` in a switch-case statement in RegAnalysis.cpp (-Wimplicit-fallthrough warning)
- misleading indentation in BinaryContext.cpp (-Wmisleading-indentation warning)
Pull Request resolved: https://github.com/facebookincubator/BOLT/pull/39
GitHub Author: Andreas Ziegler <andreas.ziegler@fau.de>
(cherry picked from FBD10202092)
Summary:
lld may generate relocations without associated symbols. Instead of
rejecting binaries with such relocations, we can re-create the symbol
the relocation is against based on the extracted value.
(cherry picked from FBD10054576)
Summary:
Previously, we were expanding eligible branches with stubs. After
expansion, we were computing which stubs were unnecessary and removing them,
assuming ranges were shortening as code is removed. The problem with this
approach is that for branches that refer to code that is not managed by
BOLT, the distance to that location can increase and we can end up with an
out-of-range branch.
This rewrites the pass to be simpler, only increasing size and expanding code
with stubs as needed after each iteration, stopping when code stops increasing.
Besides this rewrite, the stub-insertion pass now supports stubs grouping
similar to what the linker does, allowing different functions to share the
same veneer that jumps to a common callee. It also fixes a bug in the previous
implementation that, in very large functions that use TBZ/TBNZ (+-32KB range),
it would mistakenly try to reuse a local stub BB that is out of range.
This includes a change to allow hot functions to be put at the end of the
.text section, closer to the heap, requiring no veneers to jump to JITted
code. And finally it enables eliminate veneers pass by default.
(cherry picked from FBD10023158)
Summary:
If we reuse text section under `-use-old-text` option, then there's no
need to rename it. Tools, such as perf, seem to not like binaries
without `.text`.
Additionally, check if the code fits into `.text` using the page
alignment, otherwise we were skipping the alignment relying on the user
detecting the warning message. This could have resulted in unexpected
performance drops.
Also add `-no-huge-pages` option to use regular page size for code
alignment purposes (i.e. 4KiB instead of 2MiB).
(cherry picked from FBD10024670)
Summary:
While creating BinaryData objects we used to process all symbol table
entries. However, some symbols could belong to non-allocatable sections,
and thus we have to ignore them for the purpose of analyzing in-memory
data.
(cherry picked from FBD9666511)
Summary:
For jump tables ICP was using profile from the jump table itself which
doesn't work correct if the jump table is re-used at different code
locations.
(cherry picked from FBD9618774)
Summary:
While running ICF pass we have skipped merging profile data for jump
tables. We were only updating profile in the CFG. Fix that.
(cherry picked from FBD9595523)
Summary:
Do not truncate the binary name for comparison purposes as the binary
name we are getting from "perf script" is no longer truncated.
(cherry picked from FBD9596409)
Summary:
After optimizing a target of a jump table, ICP was not updating edge
counts corresponding to that target. As a result the edge could be left
hot and negatively influence the code layout.
(cherry picked from FBD9524396)
Summary:
In some rare cases a compiler may generate DWARF that contains an empty
CU DIE that references a debug line fragment. That fragment will contain
no file name information, and we fail to register it. Then, as a result,
DW_AT_stmt_list is not updated for the CU. This may cause some
DWARF-processing tools to segfault.
As a solution/workaround, we register "<unknown>" file name for such
debug line tables.
(cherry picked from FBD9526705)
Summary:
The build-id is used by tools to uniquely identify binaries. Update
the output binary build-id with a different number to make it
distinguishable from the input binary. This implementation just flips
the last build-id bit.
(cherry picked from FBD9235336)
Summary:
When updating CFI for a function that was optimized by
shrink-wrapping, if the function had no frame pointers, the CFI update
algorithm was incorrect.
(cherry picked from FBD9328658)
Summary:
Position-independent binaries may have runtime relocations of type
R_X86_64_RELATIVE that need an update if they were pointing to one of
the functions that we have relocated.
(cherry picked from FBD9374164)
Summary:
Processing profile data for binaries with flexible load address (such as
position-independent executables and shared objects) requires adjusting
binary addresses depending on the base load address.
For every PID the mapping will be more or less unique when executing
with ASLR enabled, thus we have to keep the mapping record for all PIDs
associated with the binary. Then we adjust the addresses based on those
mappings.
(cherry picked from FBD9368566)
Summary:
Switch from using `perf script --show-task-events` to
`perf script --show-mmap-events` for associating a binary with PIDs in
perf.data. The output of the former command does not provide enough
information for PIE/.so processing.
(cherry picked from FBD9346586)
Summary:
A recent commit broke our tests because it was depending on
getNumNonPseudos() at a very late stage of our optimization pipeline.
The problem was in a instruction deletion member function in
BinaryBasicBlock that was not updating the number of pseudos after
deletion. Fix this.
(cherry picked from FBD9305972)
Summary:
Couple of updates:
1) Handle address pattern with segment register.
2) Assume R11 available for PLT calls always.
3) Add CFI state to each BB.
4) early exit getMacroOpFusionPair if Instruction.size() <2.
(cherry picked from FBD9172426)
Summary:
Sometimes GCC can generate code where one of jump table entries
is being used by an indirect branch with a fixed memory reference,
such as "jmp *(JT+8)". If we don't convert such branches to direct ones
and move jump tables, then the indirect branch will reference the old
table value and will end up at the non-updated destination, possibly
causing a runtime crash.
This fix converts such indirect branches into direct ones.
For now we mark functions containing indirect branches with fixed
destination as non-simple to prevent unreachable code elimination
problem triggered by related dead/unreachable jump table.
(cherry picked from FBD9192363)
Summary:
Relocation value verification was failing for IFUNC as the real value
used for relocation wasn't the symbol value, but a corresponding PLT
entry.
Relax the verification and skip any symbols of ST_Other type.
(cherry picked from FBD9123741)
Summary:
containsRange() functions were incorrectly checking for an empty range
at the end of containing object. I.e. [a,b) was reporting true for
containing [b,b).
(cherry picked from FBD9074643)
Summary:
TLS segment provide a template for initializing thread-local storage
for every new thread. It consists of initialized and uninitialized
parts. The uninitialized part of TLS, .tbss, is completely meaningless
from a binary analysis perspective. It doesn't take any space in the
file, or in memory. Note that this is different from a regular .bss
section that takes space in memory.
We should not place .tbss into a list of allocatable sections, otherwise
it may cause conflicts with objects contained in the next section.
(cherry picked from FBD9074056)
Summary:
For large binaries, cache+ algorithm adds a noticeable overhead in
comparison with cache. This modification restricts search space of the
optimization, which makes cache+ as fast as cache for all tested binaries.
There is a tiny (in the order of 0.01%) regression in cache-related metrics,
but this is not noticeable in practice.
(cherry picked from FBD8369968)
Summary:
The regular perf2bolt aggregation job is to read perf output directly.
However, if the data is coming from a database instead of perf, one
could write a query to produce a pre-aggregated file. This function
deals with this case.
The pre-aggregated file contains aggregated LBR data, but without binary
knowledge. BOLT will parse it and, using information from the
disassembled binary, augment it with fall-through edge frequency
information. After this step is finished, this data can be either
written to disk to be consumed by BOLT later, or can be used by BOLT
immediately if kept in memory.
File format syntax:
{B|F|f} [<start_id>:]<start_offset> [<end_id>:]<end_offset> <count>
[<mispred_count>]
B - indicates an aggregated branch
F - an aggregated fall-through (trace)
f - an aggregated fall-through with external origin - used to disambiguate
between a return hitting a basic block head and a regular internal
jump to the block
<start_id> - build id of the object containing the start address. We can
skip it for the main binary and use "X" for an unknown object. This will
save some space and facilitate human parsing.
<start_offset> - hex offset from the object base load address (0 for the
main executable unless it's PIE) to the start address.
<end_id>, <end_offset> - same for the end address.
<count> - total aggregated count of the branch or a fall-through.
<mispred_count> - the number of times the branch was mispredicted.
Omitted for fall-throughs.
Example
F 41be50 41be50 3
F 41be90 41be90 4
f 41be90 41be90 7
B 4b1942 39b57f0 3 0
B 4b196f 4b19e0 2 0
(cherry picked from FBD8887182)
Summary:
This diff have the API needed to inject functions using bolt.
In relocation mode injected functions are emitted between the cold and the hot functions,
In non-reloc mode injected functions are emitted a next text section.
(cherry picked from FBD8715965)
Summary:
If the input binary does not have a build-id and the name does not match
any file names in perf.data, then reject the binary, and issue an error
message suggesting to rename it to one of the listed names from
perf.data.
(cherry picked from FBD8846181)
Summary:
Recent compiler tool chains can produce build-ids that are less than 40
characters long. Linux perf, however, always outputs 40 characters,
expanding the string with 0's as needed. Fix the matching by only
checking the string prefix.
(cherry picked from FBD8839452)
Summary:
Rework the logic we use for managing references to constant
islands. Defer the creation of the cold versions to when we split the
function and will need them.
(cherry picked from FBD8228803)
Summary:
llvm-dwarfdump is relying on getRelocatedSection() to return
section_end() for ELF files of types other than relocatable objects.
We've changed the function to return relocatable section for other
types of ELF files. As a result, llvm-dwarfdump started re-processing
relocations for sections that already had relocations applied, e.g. in
executable files, and this resulted in wrong values reported.
As a workaround/solution, we make this function return relocated section
for executable (and any non-relocatable objects) files only if the
section is allocatable.
(cherry picked from FBD8760175)
Summary:
As reported in GH-28 `perf` can produce `-` symbol for misprediction bit
if the bit is not supported by the kernel/HW. In this case we can ignore
the bit.
(cherry picked from FBD8786827)
Summary:
When a given function B, located after function A, references
one of A's basic blocks, it registers a new global symbol at the
reference address and update A's Labels vector via
BinaryFunction::addEntryPoint(). However, we don't update A's branch
targets at this point. So we end up with an inconsistent CFG, where the
basic block names are global symbols, but the internal branch operands
are still referencing the old local name of the corresponding blocks
that got promoted to an entry point. This patch fix this by detecting
this situation in addEntryPoint and iterating over all instructions,
looking for references to the old symbol and replacing them to use the
new global symbol (since this is now an entry point).
Fixesfacebookincubator/BOLT#26
(cherry picked from FBD8728407)
Summary:
While removing unreachable blocks, we may decide to remove a
block that is listed as a target in a jump table entry. If we do that,
this label will be then undefined and LLVM assembler will crash.
Mitigate this for now by not removing such blocks, as we don't support
removing unnecessary jump tables yet.
Fixesfacebookincubator/BOLT#20
(cherry picked from FBD8730269)
Summary:
If the encoding is not specified in CIE augmentation string, then it
should be DW_EH_PE_absptr instead of DW_EH_PE_omit.
(cherry picked from FBD8740274)
Summary:
In release build without assertions MCInst::dump() is undefined and
causes link time failure.
Fixesfacebookincubator/BOLT#27.
(cherry picked from FBD8732905)
Summary:
Check if the input binary ELF type. Reject any binary not of
ET_EXEC type, including position-independent executables (PIEs).
Also print the first function containing PIC jump table.
(cherry picked from FBD8707274)
Summary:
Ignore 'S' in augmentation string on input. It just marks a signal
frame. All we have to do is propagate it.
Fixesfacebookincubator/BOLT#21
This was already in LLVM trunk rL331738. Update llvm.patch.
(cherry picked from FBD8707222)
Summary:
GCC 8 can generate jump tables with just 2 entries. Modify our heuristic
to accept it. We still assert that there's more than one entry.
(cherry picked from FBD8709416)
Summary:
Add support for functions with internal calls, necessary for
handling Intel MKL library and some code observed in google core dumper
library.
This is not optimizing these functions, but only identifying them,
running analyses to assure we will not break those functions if we move
them, and then "freezing" these functions (marking as not simple so Bolt
will not try to reorder it or touch it in any way).
(cherry picked from FBD8364381)
Summary:
When processing binary with -debug mode in some cases, BD could be nullptr. It will be better to fail later on assert than here with segfault.
Closes https://github.com/facebookincubator/BOLT/pull/18
GitHub Author: Alexander Gryanko <xpahos@gmail.com>
(cherry picked from FBD8650719)
Summary:
This option only works in relocation mode. In non-relocation
mode, it generates invalid references that cause MCStreamer to fail.
Disable this flag if the user requested and print a warning.
(cherry picked from FBD8625990)
Summary:
Create folders and setup to make LIT run BOLT-only tests. Add
a test example. This will add a new make/ninja rule "check-bolt" that
the user can invoke to run LIT on this folder.
(cherry picked from FBD8595786)
Summary:
BOLT heuristics failed to work if false PIC jump table entries were
accepted when they were pointing inside a function, but not at
an instruction boundary.
This fix checks if the destination falls at instruction boundary, and
if it does not, it truncates the jump table. This, of course, still does not
guarantee that the entry corresponds to a real destination, and we can
have "false positive" entry(ies). However, it shouldn't affect
correctness of the function, but the CFG may have edges that are never
taken. We may update an incorrect jump table entry, corresponding to an
unrelated data, and for that reason we force moving of jump tables if a
PIC jump table was detected.
(cherry picked from FBD8559588)
Summary:
Don't report all data objects with hash collisions by default. Only
report the summary, and use -v=1 for providing the full list.
(cherry picked from FBD8372241)
Summary:
This diff replaces the addresses in all the {SYMBOLat,HOLEat,DATAat} symbols with hash values based on the data contained in the symbol. It should make the profiling data for anonymous symbols robust to address changes.
The only small problem with this approach is that the hashed name for padding symbols of the same size collide frequently. This shouldn't be a big deal since it would be weird if those symbols were hot.
On a test run with hhvm there were 26 collisions (out of ~338k symbols). Most of the collisions were from small (2,4,8 byte) objects.
(cherry picked from FBD7134261)
Summary:
This diff introduces a modification of cache+ block ordering algorithm,
which reordered and merges cold blocks in a function with the goal of reducing
the number of (non-fallthrough) jumps, and thus, the code size.
(cherry picked from FBD8044978)
Summary:
Add "-inline-memcpy" option to inline calls to memcpy() using
"rep movsb" instruction. The pass is X86-specific.
Calls to _memcpy8 are optimized too using a special return value
(dest+size).
The implementation is very primitive in that it does not track liveness
of %rax after return, and no %rcx substitution. This is going to get
improved if we find the optimization to be useful.
(cherry picked from FBD8211890)
Summary:
In AArch64, when the binary gets large, the linker inserts
stubs with 3 instructions: ADRP to load the PC-relative address of
a page; ADD to add the offset of the page; and a branch instruction
to do an indirect jump to the contents of X16 (the linker-reserved
reg). The problem is that the linker does not issue a relocation for
this (since this is not code coming from the assembler), so BOLT has
no idea what is the real target, unless it recognizes these instructions
and extract the target by combining the operands of the instructions
from the stub. This diff does exactly that.
(cherry picked from FBD7882653)
Summary:
If the input binary for perf2bolt has a build-id and perf data has
recorded build-ids, then try to match them. Adjust the file name if
build-ids match to cover cases where the binary was renamed after data
collection. If there's no matching build-id report an error and exit.
While scanning task events, truncate the name to 15 characters prior to
matching, since that's how names are reported by perf.
(cherry picked from FBD8034436)
Summary:
Option `-report-bad-layout=N` prints top N functions with layouts
that have cold blocks placed in the middle of hot blocks. The sorting is
based on execution_count / number_of_basic_blocks formula.
(cherry picked from FBD8051950)
Summary:
Application code can reference functions in a non-standard way, e.g.
using arithmetic and bitmask operations on them. One example is if a
program checks if a function is below a certain address or within
a certain address range to perform a low-level optimization or generate
a proper code (JIT).
Instead of relying on a relocation value (symbol+addend), we use only
the symbol value, and then check if the value is inside the function.
If it is, we treat it as a code reference against location within the
function, otherwise we handle it as a non-standard function reference
and issue a warning.
(cherry picked from FBD7996274)
Summary:
When we make changes to MCInst opcodes (or get changes from upstream),
a hash value for BinaryFunction changes. As a result, we are unable
to match profile generated by a previous version of BOLT.
Add option `-profile-ignore-hash` to match profile while ignoring
function hash value. With this option we match functions with common
names using the number of basic blocks.
(cherry picked from FBD7983269)
Summary:
To accurately account for PLT optimization, each PLT call should be
counted as an extra indirect call instruction, which in turn is
a load, a call, an indirect call, and instruction entry in dyno stats.
(cherry picked from FBD7978980)
Summary:
While working with a binary in non-relocations mode, I realized
some cache metrics are not computed correctly. Hence, this fix.
In addition, logging the number of functions with modified ordering of
basic blocks, which is helpful for analysis.
(cherry picked from FBD7975392)
Summary:
Enable BOLT to reorder data sections in a binary based on memory
profiling data.
This diff adds a new pass to BOLT that can reorder data sections for
better locality based on memory profiling data. For now, the algorithm
to order data is primitive and just relies on the frequency of loads to
order the contents of a section. We could probably do a lot better by
looking at what functions use the hot data and grouping together hot
data that is used by a single function (or cluster of functions).
Block ordering might give some hints on how to order the data better as
well.
The new pass has two basic modes: inplace and split (when inplace is
false). The default is split since inplace hasn't really been tested
much. When splitting is on, the cold data is copied to a "cold" version
of the section while the hot data is kept in the original section, e.g.
for .rodata, .rodata will contain the hot data and .bolt.org.rodata will
contain the cold bits. In inplace mode, the section contents are
reordered inplace. In either mode, all relocations to data within that
section are updated to reflect new data locations.
Things to improve:
- The current algorithm is really dumb and doesn't seem to lead to any
wins. It certainly could use some improvement.
- Private symbols can have data that leaks over to an adjacent symbol,
e.g. a string that has a common suffix can start in one symbol and
leak over (with the common suffix) into the next. For now, we punt on
adjacent private symbols.
- Handle ambiguous relocations better. Section relocations that point
to the boundary of two symbols will prevent the adjacent symbols from
being moved because we can't tell which symbol the relocation is for.
- Handle jump tables. Right now jump table support must be basic if
data reordering is enabled.
- Being able to handle TLS. A good amount of data access in some
binaries are happening in TLS. It would be worthwhile to be able to
reorder any TLS sections too.
- Handle sections with writeable data. This hasn't been tested so
probably won't work. We could try to prevent false sharing in
writeable sections as well.
- A pie in the sky goal would be to use DWARF info to reorder types.
(cherry picked from FBD6792876)
Summary:
The default is not changing, i.e. we are not aligning code within a
function by default.
New meaning of options for aligning basic blocks:
-align-blocks
triggers basic block alignment based on profile
-preserve-blocks-alignment
tries to preserve basic block alignment seen on input
Tuning options for "-align-blocks":
-align-blocks-min-size=<uint>
blocks smaller than the specified size wouldn't be aligned
-align-blocks-threshold=<uint>
align only blocks with frequency larger than containing function
execution frequency specified in percent. E.g. 1000 means aligning
blocks that are 10 times more frequently executed than the containing
function.
(cherry picked from FBD7921980)
Summary:
BOLT sources are being moved under tools/llvm-bolt/src
and tools/llvm-bolt will contain more files such as LICENSE.txt,
README.txt, etc.
Remove trailing white spaces from our sources.
Create llvm.patch by running
> git diff f137ed238db11440f03083b1c88b7ffc0f4af65e include lib > \
tools/llvm-bolt/llvm.patch
README.txt has instructions on checking out sources and applying the
patch.
(cherry picked from FBD7878380)
Summary:
New profile writer was crashing as functions were lacking a profile
flags. Fix it by requiring flags when marking function as profiled.
Generate new profile for clang. The new profile has more coverage and
results in better overall improvement from BOLT. It was generated by
merging multiple runs of:
% perf record -e cycles:u -j any,u -F32000 -- \
./clang bf.cpp -O2 -std=c++11 -c -o /tmp/bf.o
(cherry picked from FBD7798580)
Summary:
Refactor MCInst comparison code to support target-dependent
functionality. This was necessary because AArch64 uses MCTargetExprs
that only the AArch64 backend knows how to unpack it and compare. Also
fix a bug where a relocation against a constant island would make BOLT
create a fixed reference against a code location in a similar way to
read-only data, so when we asked to -use-old-text, the code would break
for this particular HHVM function
(_ZN5folly2io4zlib18defaultZlibOptionsEv) because the reference now
contains invalid data, since the original .text was overwritten. Finally,
fix a bug with -update-debug-sections on AArch64 where the update
loop wasn't expecting a function with zero basic blocks, which can
happen on AArch64 because some functions contain just a constant
island.
(cherry picked from FBD7679244)
Summary: Modifying parameters of block reordering algorithm that result in better performance. Additionally extending some cache-related metrics
(cherry picked from FBD7578336)
Summary:
Whenever building BOLT in an AArch64 box, we need to make sure
we do not run tests that are excluse for x86. This diff also adds a tag
for expensive tests, so the user can disable them, which is useful when
using a memory-constrained machine to run BOLT tests. It also removes
ifdefs that caused BOLT to behave diferently when running in a non-x86
host. Finally, it changes a case where we depended on updated libstdc++
implementation for insert to make the codebase more friendly with boxes
that do not have the newer version of the lib.
(cherry picked from FBD7625715)
Summary:
Restore the optimization with some modifications:
* Only enabled in relocation mode.
* Covers instructions other than TEST/CMP.
* Prints missed macro-fusion opportunities for input.
* By default enabled for all hot code.
* Without profile enabled for all code.
The new command-line option:
-align-macro-fusion - fix instruction alignment for macro-fusion (x86 relocation mode)
=none - do not insert alignment no-ops for macro-fusion
=hot - only insert alignment no-ops on hot execution paths (default)
=all - always align instructions to allow macro-fusion
(cherry picked from FBD7644042)
Summary:
Since BOLT can use relocations in the binary automatically, it's not
always clear if we are operating in relocation mode or not. This diff
adds "BOLT-INFO" message indicating if the relocation mode in ON.
(cherry picked from FBD7557492)
Summary:
Expanded YAML profile format to support different kinds of profile
including LBR and non-LBR (and memevents in the future).
The profile now starts with a header that includes the profile
description. "profile-flags" field includes either "lbr" or "sample",
but not both at the same time. It could also include "memevent" in
addition to other flags.
For now, the only way to generate non-LBR YAML profile is through
conversion. Once task is done, it should be possible to use
perf2bolt for it.
(cherry picked from FBD7595693)
Summary:
merge-fdata now operates on .fdata files in YAML format. The previous
format is not supported, which means that non-LBR data could not be
merged and memory data has to be merged with "cat" command.
(cherry picked from FBD7544031)
Summary:
This diff has 3 fixes. First fixes the way relocations are read
and interpreted for AArch64, so the references are preserved correctly.
Second, it fixes constant islands to be able to live in the very first
address of a function (which means there is no code, but this function
contains just a constant island).
Third, it fixes function splitting to do not outline entry points for
AArch64. This was done because some functions may load pointers to its
internal basic blocks, issueing a short-range ADR instruction to do so
without its pair ADRP (since the size of the function is supposed to
be small). But when we move this block to a cold region, that is not
the case anymore. Since blocks with a reference are marked as entry
points, we conservatively disable outlining for them in AArch64.
(cherry picked from FBD7505067)
Summary:
Change the way annotations are stored and processed.
Embed annotation type/index into immediate value stored as an operand.
This limits the effective range of values that could be stored as
annotations to 56 bits, which is still plenty for most integer types
that we use and for pointers on real systems. High 8 bits are reserved
for storing annotation type/index.
Expand the interface for general annotations to include reference to
annotations by index. The main purpose of this interface is to improve
performance of annotations that are used by heavy (>O(N)) algorithms,
such as data flow analysis.
For -frame-opt pass, new memory usage and processing times are slightly
better compared to those before refactoring.
(cherry picked from FBD7492017)
Summary:
Use MCPlus::getNumPrimeOperands() to get the real number of operands
on MCInst. Alternatively, use MCInstrDesc::getNumOperands().
(cherry picked from FBD7507666)
Summary:
When we erase invalid/unreachable basic blocks, we have to remove them
from a list of predecessors of regular blocks, otherwise the CFG will be
left in a broken state containing references to removed basic blocks.
(cherry picked from FBD7464292)
Summary:
We verify that relocation information matches a value stored in a
binary, i.e. "ExtractedValue == SymbolValue + Addend". However, because
of the size of the relocation, and the fact that an addend is always
of type int64_t, we have to sign-extend the extracted value, and then we
might get mismatch in higher bits in certain scenarios. Hence, we should
only compare values that are truncated to a relocation size.
Discovered while processing hhvm binary with modified compiler flags.
(cherry picked from FBD7462559)
Summary:
Getting a forward iterator from reverse iterator was implemented
incorrectly. For some reason erase worked on it, but it's clearly wrong
and printing the instruction (before the deletion) results in an error.
(cherry picked from FBD7457457)
Summary:
Changes that we made to MCInst, MCOperand, MCExpr, etc. are now all
moved into tools/llvm-bolt. That required a change to the way we handle
annotations and any extra operands for MCInst.
Any MCPlus information is now attached via an extra operand of type
MCInst with an opcode ANNOTATION_LABEL. Since this operand is MCInst, we
attach extra info as operands to this instruction. For first-level
annotations use functions to access the information, such as
getConditionalTailCall() or getEHInfo(), etc. For the rest, optional or
second-class annotations, use a general named-annotation interface such
as getAnnotationAs<uint64_t>(Inst, "Count").
I did a test on HHVM binary, and a memory consumption went down a little
bit while the runtime remained the same.
(cherry picked from FBD7405412)
Summary:
Some improvements for CFG construction:
- getting rid of fallthrough-inferrence, as this is already
done DataAggregator;
- adjusting block counts for blocks with non-zero outgoing edges
to make sure they're not outlined;
- making sure that all functions (including non-simple ones) are
reordered and placed in the hot section.
The main goal of the diff is to make sure that constructed CFG graphs
exactly correspond to the input profile data.
(cherry picked from FBD7323205)
Summary:
The binary had some unexpected ovelapping symbols:
.str.34.llvm.2944770977690351622/1 address = 0x48e9ec7, next address =
0x48e9ed2, size = 21
PG.LC135/1 address = 0x48e9ed2, next address = 0x48e9eef, size = 29
BOLT wasn't expecting this type of overlap when generating HOLE symbols,
so it was asserting. I've changed the code to deal with this case.
I'll need to change the reordering pass to mark these types of symbols
as unmoveable for now.
(cherry picked from FBD7304195)
Summary: This assertion was making sure that when we patched up symbol sizes that we wouldn't modify the size of a symbol that has already had its size set. The issue here is that private symbols are sometimes composed of multiple objects internally (e.g. jump tables). In this particular case a jump table started at the same address as the private data blob it was contained in. Currently, there isn't any good way of differentiating symbols that start at the same address (except possibly using multimaps for certain data structures). I'm hacking around it by modifying the assertion to ignore jump tables and skip setting the size when it has already been set. This shouldn't affect any existing optimizations since the only thing that depended on sizes is data reordering and that currently ignores jump tables and private data blobs.
(cherry picked from FBD7269207)
Summary:
Refactor architecture-specific code out of llvm into llvm-bolt.
Introduce MCPlusBuilder, a class that is taking over MCInstrAnalysis
responsibilities, i.e. creating, analyzing, and modifying instructions.
To access the builder use BC->MIB, i.e. substitute MIA with MIB.
MIB is an acronym for MCInstBuilder, that's what MCPlusBuilder used
to be. The name stuck, and I find it better than MPB.
Instructions are still MCInst, and a bunch of BOLT-specific code still
lives in LLVM, but the staff under Target/* is significantly reduced.
(cherry picked from FBD7300101)
Summary:
In new ORC, the sequence of how sections are allocated and loaded is
changed. Now everything is delayed until emitAndFinalize() is called,
and all actions are supposed to happen via notification functors.
There are two functors that we pass to new ObjectLinkingLayer object.
One is used to notify when objects are loaded, and the other - once they
are finalized. We use the first one to remap sections to proper
addresses, and that's the earliest place where we can do it. However,
ORC decides to update symbols right before that, and as a result they
are updated with non-mapped values.
There are two possible fixes for that. This diff postpones the update to
the symbol table until the notifier is called. I don't know what other
tools depend on the existing sequence, and the proper fix may involve
creating a third notifier to be called before the symbol table update.
(cherry picked from FBD7280973)
Summary:
Rebased version revealed a mistake when computing the dataflow
for the "remove-unused-stores" optimization. This is disabled in prod but
it doesn't hurt to fix it, so the tests for the rebased bolt go green
again.
(cherry picked from FBD7253418)
Summary:
This is a simple bolt-based tool that instantiates two
RewriteInstances objects and compares them. Add a method to
RewriteInstance to enable us to compare two objects. Include a mechanism
to match functions from binary 1 to binary 2 and finally print the
largest differences in profiling data from one binary to another.
(cherry picked from FBD6517076)
Summary:
This makes it possible to do adjustments to all functions based on
information gained during disassembly. E.g. if we detect an entry point
after the CFG for a function is constructed, we have to take a
conservative approach, and mark such function as non-simple. Now we have
this information before building the CFG. This could also be used to do
other processing/post-processing on disassembled functions that might
affect CFG construction of other functions (e.g. early detection of
functions that never return).
The drawback of this approach is that we lose cache locality and some
processing performance as a result. I've measured 5 second difference
on HHVM binary.
(cherry picked from FBD7258466)
Summary:
This is preparation work for static data reordering.
I've created a new class called BinaryData which represents a symbol
contained in a section. It records almost all the information relevant
for dealing with data, e.g. names, address, size, alignment, profiling
data, etc. BinaryContext still stores and manages BinaryData objects
similar to how it managed symbols and global addresses before. The
interfaces are not changed too drastically from before either. There is
a bit of overlap between BinaryData and BinaryFunction. I would have
liked to do some more refactoring to make a BinaryFunctionFragment that
subclassed from BinaryData and then have BinaryFunction be composed or
associated with BinaryFunctionFragments.
I've also attempted to use (symbol + offset) for when addresses are
pointing into the middle of symbols with known sizes. This changes the
simplify rodata loads optimization slightly since the expression on an
instruction can now also be a (symbol + offset) rather than just a symbol.
One of the overall goals for this refactoring is to make sure every
relocation is associated with a BinaryData object. This requires adding
"hole" BinaryData's wherever there are gaps in a section's address space.
Most of the holes seem to be data that has no associated symbol info. In
this case we can't do any better than lumping all the adjacent hole
symbols into one big symbol (there may be more than one actual data
object that contributes to a hole). At least the combined holes should
be moveable.
Jump tables have similar issues. They appear to mostly be sub-objects
for top level local symbols. The main problem is that we can't recognize
jump tables at the time we scan the symbol table, we have to wait til
disassembly. When a jump table is discovered we add it as a sub-object
to the existing local symbol. If there are one or more existing
BinaryData's that appear in the address range of a newly created jump
table, those are added as sub-objects as well.
(cherry picked from FBD6362544)
Summary:
Fix a few ShrinkWrapping bugs:
- Using push-pop mode in a function that required aligned stack
- Correctly update the edges in jump tables after splitting critical
edges
- Fix stack pointer restores based on RBP + offset, when we change the
stack layout in push-pop mode.
(cherry picked from FBD6755232)
Summary:
Fix a bug introduced by rebasing with respect to aligned ULEBs.
This wasn't breaking anything but it is good to keep LDSA aligned.
(cherry picked from FBD7094742)
Summary:
This is a big refactoring of the section handling code. I've removed the SectionInfoMap and NoteSectionInfo and stored all the associated info about sections in BinaryContext and BinarySection classes. BinarySections should now hold all the info we care about for each section. They can be initialized from SectionRefs but don't necessarily require one to be created. There are only one or two spots that needed access to the original SectionRef to work properly.
The trickiest part was making sure RewriteInstance.cpp iterated over the proper sets of sections for each of it's different types of processing. The different sets are broken down roughly as allocatable and non-alloctable and "registered" (I couldn't think up a better name). "Registered" means that the section has been updated to include output information, i.e. contents, file offset/address, new size, etc. It may help to have special iterators on BinaryContext to iterate over the different classes to make things easier. I can do that if you guys think it is worthwhile.
I found pointee_iterator in the llvm ADT code. Use that for iterating over BBs in BinaryFunction rather than the custom iterator class.
(cherry picked from FBD6879086)
Summary:
When we move a jump table to either hot or cold new section
(-jump-tables=move), we rely on a number of taken branches from the table
to decide if it's hot or cold. However, if the function is non-simple, we
always get 0 count, and always move the table to the cold section.
Instead, we should make a conservative decision based on the execution
count of the function.
(cherry picked from FBD7058127)
Summary:
Speed of cache+ by skipping mallocs on vectors.
Although this change speeds up the algorithm by 2x, this is still not
enough for some binaries where some functions have ~2500 hot basic
blocks. Hence, introduce a threshold for expensive optimizations in
CachePlusReorderAlgorithm. If the number of hot basic blocks exceeds
the threshold (2048 by default), we use a cheaper version, which is
quite fast.
(cherry picked from FBD6928075)
Summary:
Do a better job of recording fall-through branches in new profile mode
(-prof-compat-mode=0). For this we need to record offsets for all
instructions that are last in the containing basic block.
Change the way we convert conditional tail calls. Now we never reverse
the condition. This is required for better profile matching.
The original approach of preserving the direction was controversial
to start with.
Add "-infer-fall-throughs" option (on by default) to allow disabling
inference of fall-through edge counts.
(cherry picked from FBD6994293)
Summary:
Prioritize functions with 100% name match when doing LTO "fuzzy"
name matching. Avoid re-assigning profile to a function.
(cherry picked from FBD6992179)
Summary:
In relocation mode trap on entry to any function that has AVX-512
instructions. This is controlled by "-trap-avx512" option which is on
by default. If the option is disabled and AVX-512 instruction is seen
in relocation mode, then we abort while re-writing the binary.
(cherry picked from FBD6893165)
Summary:
This commit includes all code necessary to make BOLT working again
after the rebase. This includes a redesign of the EHFrame work,
cherry-pick of the 3dnow disassembly work, compilation error fixes,
and port of the debug_info work. The macroop fusion feature is not
ported yet.
The rebased version has minor changes to the "executed instructions"
dynostats counter because REP prefixes are considered a part of the
instruction it applies to. Also, some X86 instructions had the "mayLoad"
tablegen property removed, which BOLT uses to identify and account
for loads, thus reducing the total number of loads reported by
dynostats. This was observed in X86::MOVDQUmr. TRAP instructions are
not terminators anymore, changing our CFG. This commit adds compensation
to preserve this old behavior and minimize tests changes. debug_info
sections are now slightly larger. The discriminator field in the line
table is slightly different due to a change upstream. New profiles
generated with the other bolt are incompatible with this version
because of different hash values calculated for functions, so they will
be considered 100% stale. This commit changes the corresponding test
to XFAIL so it can be updated. The hash function changes because it
relies on raw opcode values, which change according to the opcodes
described in the X86 tablegen files. When processing HHVM, bolt was
observed to be using about 800MB more memory in the rebased version
and being about 5% slower.
(cherry picked from FBD7078072)
Summary:
This fixes the increased memory consumption introduced in an earlier
diff while I was working on new profiling infra.
The increase came from a delayed release of memory allocated to
intermediate structures used to build CFG. In this diff we release
them ASAP, and don't keep them for all functions at the same time.
(cherry picked from FBD6890067)
Summary:
Limiting "Offset" annotation only to instructions that actually
need it, improves the memory consumption on HHVM binary by 1GB.
(cherry picked from FBD6878943)
Summary:
SCTC was incorrectly swapping BranchInfo when reversing the branch condition. This was wrong because when we remove the successor BB later, it removes the BranchInfo for that BB. In this case the successor would be the BB with the stats we had just swapped.
Instead leave BranchInfo as it is and read the branch count from the false or true branch depending on whether we reverse or replace the branch, respectively. The call to removeSuccessor later will remove the unused BranchInfo we no longer care about.
(cherry picked from FBD6876799)
Summary: Register all sections with BinaryContext. Store all sections in a set ordered by (address, size, name). Add two separate maps to lookup sections by address or by name. Non-allocatable sections are not stored in the address->section map since they all "start" at 0.
(cherry picked from FBD6862973)
Summary:
Handle types CU list in `updateGdbIndexSection`.
It looks like the types part of `.gdb_index` isn't empty when `-fdebug-types-section` is used. So instead of aborting, we copy the part to new `.gdb_index` section.
(cherry picked from FBD6770460)
Summary:
When we read profile for functions, we initialize counts for entry
blocks first, and then populate counts for all blocks based
on incoming edges.
During the second phase we ignore the entry blocks because we expect
them to be already initialized. For the primary entry at offset 0 it's
the correct thing to do, since we treat all incoming branches as calls
or tail calls. However, for secondary entries we only consider external
edges to be from calls and don't increase entry count if an edge
originates from inside the function. Thus we need to update the
secondary entry basic block counts with internal edges too.
(cherry picked from FBD6836817)
Summary:
A test is asserting on impossible addresses coming from
perf.data, instead of just reporting it as bad data. Fix this behavior.
(cherry picked from FBD6835590)
Summary:
Speeding up cache+ algorithm.
The idea is to find and merge "fallthrough" successors before main
optimization. For a pair of blocks, A and B, block B is the fallthrough
successor of A, if (i) all jumps (based on profile) from A goes to B
and (ii) all jumps to B are from A.
Such blocks should be adjacent in an optimal ordering, and should
not be considered for splitting. (This gives the speed up).
The gap between cache and cache+ reduced from ~2m to ~1m.
(cherry picked from FBD6799900)
Summary:
Refactor the relocation anaylsis code. It should be a little better at validating
that the relocation value matches up with the symbol address + addend stored in the
relocation (except on aarch64). It is also a little better at finding the symbol
address used to do the lookup in BinaryContext, rather than just using symbol
address + addend.
(cherry picked from FBD6814702)
Summary: Add BinarySection class that is a wrapper around SectionRef. This is refactoring work for static data reordering.
(cherry picked from FBD6792785)
Summary:
Rewrite how data/code markers are interpreted, so the code
can have constant islands essentially anywhere. This is necessary to
accomodate custom AArch64 assembly code coming from mozjpeg. Allow
any function to refer to the constant island owned by any other
function. When this happens, we pull the constant island from the
referred function and emit it as our own, so it will live nearby
the code that refers to it, allowing us to freely reorder functions
and code pieces. Make bolt more strict about not changing anything
in non-simple ARM functions, as we need to preserve offsets for
those functions we don't interpret their jump tables (currently
any function with jump tables in ARM is non-simple and is left
untouched).
(cherry picked from FBD6402324)
Summary:
A new profile that is more resilient to minor binary modifications.
BranchData is eliminated. For calls, the data is converted into instruction
annotations if the profile matches a function. If a profile cannot be matched,
AllCallSites data should have call sites profiles.
The new profile format is YAML, which is quite verbose. It still takes
less space than the older format because we avoid function name repetition.
The plan is to get rid of the old profile format eventually.
merge-fdata does not work with the new format yet.
(cherry picked from FBD6753747)
Summary:
Add a few new relocation types to support a wider variety of
binaries, add support for constant island duplication (so we can split
functions in large binaries) and make LongJmp pass really precise with
respect to layout, so we don't miss stubs insertions at the correct
places for really large binaries. In LongJmp, introduce "freeze"
annotations so fixBranches won't mess the jumps we carefully determined
that needed a stub.
(cherry picked from FBD6294390)
Summary:
A new block reordering algorithm, cache+, that is designed to optimize
i-cache performance.
On a high level, this algorithm is a greedy heuristic that merges
clusters (ordered sequences) of basic blocks, similarly to how it is
done in OptimizeCacheReorderAlgorithm. There are two important
differences: (a) the metric that is optimized in the procedure, and
(b) how two clusters are merged together.
Initially all clusters are isolated basic blocks. On every iteration,
we pick a pair of clusters whose merging yields the biggest increase
in the ExtTSP metric (see CacheMetrics.cpp for exact implementation),
which models how i-cache "friendly" a pecific cluster is. A pair of
clusters giving the maximum gain is merged to a new clusters. The
procedure stops when there is only one cluster left, or when merging
does not increase ExtTSP. In the latter case, the remaining clusters
are sorted by density.
An important aspect is the way two clusters are merged. Unlike earlier
algorithms (e.g., OptimizeCacheReorderAlgorithm or Pettis-Hansen), two
clusters, X and Y, are first split into three, X1, X2, and Y. Then we
consider all possible ways of gluing the three clusters (e.g., X1YX2,
X1X2Y, X2X1Y, X2YX1, YX1X2, YX2X1) and choose the one producing the
largest score. This improves the quality of the final result (the
search space is larger) while keeping the implementation sufficiently
fast.
(cherry picked from FBD6466264)
Summary:
Do not assign a LP to tail calls. They are not calls in the
view of an unwinder, they are just regular branches. We were hitting an
assertion in BinaryFunction::removeConditionalTailCalls() complaining
about landing pads in a CTC, however it was in fact a
builtin_unreachable being conservatively treated as a CTC.
(cherry picked from FBD6564957)
Summary:
The pass was previously copying data that would change after layout
because it had a relocation at the copied address.
(cherry picked from FBD6541334)
Summary:
Profile reading was tightly coupled with building CFG. Since I plan
to move to a new profile format that will be associated with CFG
it is critical to decouple the two phases.
We now have read profile right after the cfg was constructed, but
before it is "canonicalized", i.e. CTCs will till be there.
After reading the profile, we do a post-processing pass that fixes
CFG and does some post-processing for debug info, such as
inference of fall-throughs, which is still required with the current
format.
Another good reason for decoupling is that we can use profile with
CFG to more accurately record fall-through branches during
aggregation.
At the moment we use "Offset" annotations to facilitate location
of instructions corresponding to the profile. This might not be
super efficient. However, once we switch to the new profile format
the offsets would be no longer needed. We might keep them for
the aggregator, but if we have to trust LBR data that might
not be strictly necessary.
I've tried to make changes while keeping backwards compatibly. This makes
it easier to verify correctness of the changes, but that also means
that we lose accuracy of the profile.
Some refactoring is included.
Flag "-prof-compat-mode" (on by default) is used for bug-level
backwards compatibility. Disable it for more accurate tracing.
(cherry picked from FBD6506156)
Summary:
If relocations are available in the binary, use them by default.
If "-relocs" is specified, then require relocations for further
processing. Use "-relocs=0" to forcefully ignore relocations.
Instead of `opts::Relocs` use `BinaryContext::HasRelocations` to check
for the presence of the relocations.
(cherry picked from FBD6530023)
Summary:
The list of landing pads in BinaryBasicBlock was sorted by their address
in memory. As a result, the DFS order was not always deterministic.
The change is to store landing pads in the order they appear in invoke
instructions while keeping them unique.
Also, add Throwers verification to validateCFG().
(cherry picked from FBD6529032)
Summary:
Some helpful options:
-print-dyno-stats-only
while printing functions output dyno-stats and skip instructions
-report-stale
print a list of functions with a stale profile
(cherry picked from FBD6505141)
Summary:
Add a pass to rebalance the usage of REX prefixes, moving them
from the hot code path to the cold path whenever possible. To do this, we
rank the usage frequency of each register and exchange an X86 classic reg
with an extended one (which requires a REX prefix) whenever the classic
register is being used less times than the extended one. There are two
versions of this pass: regular one will only consider RBX as classic and
R12-R15 as extended registers because those are callee-saved, which means
their scope is local to the function and therefore they can be easily
interchanged within the function without further consequences. The
aggressive version relies on liveness analysis to detect if the value of
a register is being used as a caller-saved value (written to without
being read first), which also is eligible for reallocation. However, it
showed limited results and is not the default option because it is
expensive.
Currently, this pass does not update debug info. This means that if a
substitution is made, the AT_LOCATION of a variable inside a function may
be outdated and GDB will display the wrong value if you ask it to print
the value of the affected variable. Updating DWARF involves a painful
task of writing a new DWARF expression parser/writer similar to the one
we already have for CFI expressions. I'll defer the task of writing this
until we determine this optimization is enabled in production. So far,
it is experimental to be combined with other optimizations to help us
find a new set of optimizations that is beneficial.
(cherry picked from FBD6476659)
Summary: Load elimination for ICP wasn't handling nested jump tables correctly. It wasn't offseting the indices by the range of the nested table. I also wasn't computing some of the stats ICP correctly in all cases which was leading to weird results in the stats.
(cherry picked from FBD6453693)
Summary:
The diff introduces two measures for i-cache performance: a TSP measure (currently used for optimization) and an "extended" TSP measure that takes into account jumps between non-consecutive basic blocks. The two measures are computed for estimated addresses/sizes of basic blocks and for the actually omitted addresses/sizes.
Intuitively, the Extended-TSP metric quantifies the expected number of i-cache misses for a given ordering of basic blocks. It has 5 parameters:
- FallthroughWeight is the impact of fallthrough jumps on the score
- ForwardWeight is the impact of forward (but not fallthrough) jumps
- BackwardWeight is the impact of backward jumps
- ForwardDistance is the max distance of a forward jump affecting the score
- BackwardDistance is the max distance of a backward jump affecting the score
We're still learning the "best" values for the options but default values look reasonable so far.
(cherry picked from FBD6331418)
Summary:
Add a pass to identify indirect jumps to jump tables and reduce
their entries size from 8 to 4 bytes. For PIC jump tables, it will
convert the PIC code to non-PIC (since BOLT only processes static code,
it makes no sense to use expensive PIC-style jumps in static code). Add
corresponding improvements to register scavenging pass and add a MCInst
matcher machinery.
(cherry picked from FBD6421582)
Summary: The arithmetic shortening code on x86 was broken. It would sometimes shorten instructions with immediate operands that wouldn't fit into 8 bits.
(cherry picked from FBD6444699)
Summary: The icp-top-callsites option was using basic block counts to pick the top callsites while the ICP main loop was using branch info from the targets of each call. These numbers do not exactly match up so there was a dispcrepancy in computing the top calls. I've switch top callsites over to use the same stats as the main loop. The icp-always-on option was redundant with -icp-top-callsites=100, so I removed it.
(cherry picked from FBD6370977)
Summary: Add timers for non-optimization related phases. There are two new options, -time-build for disassembling functions and building CFGs, and -time-rewrite for phases in executeRewritePass().
(cherry picked from FBD6422006)
Summary:
Previously the perf2bolt aggregator was rejecting traces
finishing with REP RET (return instruction with REP prefix) as a
result of the migration from objdump output to LLVM disassembler,
which decodes REP as a separate instruction. Add code to detect
REP RET and treat it as a single return instruction.
(cherry picked from FBD6417496)
Summary:
Here's an implementation of an abstract instruction iterator for the branch/call
analysis code in MCInstrAnalysis. I'm posting it up to see what you guys think.
It's a bit sloppy with constness and probably needs more tidying up.
(cherry picked from FBD6244012)
Summary:
Use value profiling data to remove the method pointer loads from vtables when doing ICP at virtual function and jump table callsites.
The basic process is the following:
1. Work backwards from the callsite to find the most recent def of the call register.
2. Work back from the call register def to find the instruction where the vtable is loaded.
3. Find out of there is any value profiling data associated with the vtable load. If so, record all these addresses as potential vtables + method offsets.
4. Since the addresses extracted by #3 will be vtable + method offset, we need to figure out the method offset in order to determine the actual vtable base address. At this point I virtually execute all the instructions that occur between #3 and #2 that touch the method pointer register. The result of this execution should be the method offset.
5. Fetch the actual method address from the appropriate data section containing the vtable using the computed method offset. Make sure that this address maps to an actual function symbol.
6. Try to associate a vtable pointer with each target address in SymTargets. If every target has a vtable, then this is almost certainly a virtual method callsite.
7. Use the vtable address when generating the promoted call code. It's basically the same as regular ICP code except that the compare is against the vtable and not the method pointer. Additionally, the instructions to load up the method are dumped into the cold call block.
For jump tables, the basic idea is the same. I use the memory profiling data to find the hottest slots in the jumptable and then use that information to compute the indices of the hottest entries. We can then compare the index register to the hot index values and avoid the load from the jump table.
Note: I'm assuming the whole call is in a single BB. According to @rafaelauler, this isn't always the case on ARM. This also isn't always the case on X86 either. If there are non-trivial arguments that are passed by value, there could be branches in between the setup and the call. I'm going to leave fixing this until later since it makes things a bit more complicated.
I've also fixed a bug where ICP was introducing a conditional tail call. I made sure that SCTC fixes these up afterwards. I have no idea why I made it introduce a CTC in the first place.
(cherry picked from FBD6120768)
Summary:
When running hfsort+, we invalidate too many cache entries, which leads to inefficiencies. It seems we only need to invalidate cache for pairs of clusters (Into, X) and (X, Into) when modifying cluster Into (for all clusters X).
With the modification, we do not really need ShortCache, since it is computed only once per pair of clusters.
(cherry picked from FBD6341039)
Summary:
When RememberState CFI happens to be the last CFI in a basic block, we
used to set the state of the next basic block to a CFI prior to
executing RememberState instruction. This contradicts comments in
annotateCFIState() function and also differs form behaviour of
getCFIStateAtInstr(). As a result we were getting code like the
following:
.LBB0121166 (21 instructions, align : 1)
CFI State : 0
....
0000001a: !CFI $1 ; OpOffset Reg6 -16
0000001a: !CFI $2 ; OpRememberState
....
Successors: .Ltmp4167600, .Ltmp4167601
CFI State: 3
.Ltmp4167601 (13 instructions, align : 1)
CFI State : 2
....
Notice that the state at the entry of the 2nd basic block is less than
the state at the exit of the previous basic block.
In practice we have never seen basic blocks where RememberState was the
last CFI instruction in the basic block, and hence we've never run into
this issue before.
The fix is a synchronization of handling of last RememberState
instruction by annotateCFIState() and getCFIStateAtInstr().
In the example above, the CFI state at the entry to the second BB will
be 3 after this diff.
(cherry picked from FBD6314916)
Summary: Add selective control over peephole options. This makes it easier to test which ones might have a positive effect.
(cherry picked from FBD6289659)
Summary:
The logic to append an unconditional branch at the end of a block that had
the condition flipped on its conditional tail was broken. It should have
been looking at the successor to PredBB instead of BB. It also wasn't skipping
invalid blocks when finding the fallthrough block.
This fixes the SCTC bug uncovered by @spupyrev's work on block reordering.
(cherry picked from FBD6269493)
Summary:
With "-debug" flag we are using a dump in intermediate state when
basic block's list is initialized, but layout is not. In new isSplit()
funciton we were checking the size() which uses basic block list,
and then we were accessing the (uninitiazed) layout.
Instead of checking size() we should be checking layout_size().
(cherry picked from FBD6277770)
Summary:
A new 'compact' function aligner that takes function sizes in consideration. The approach is based on the following assumptions:
-- It is not desirable to introduce a large offset when aligning short functions, as it leads to a lot of "wasted" address space.
-- For longer functions, the offset can be larger than the default 32 bytes; However, using 64 bytes for the offset still worsen performance, as again a lot of address space is wasted.
-- Cold parts of functions can still use the default max-32 offset.
The algorithm is switched on/off by flag 'use-compact-aligner' and is controlled by parameters align-functions-max-bytes and align-cold-functions-max-bytes described above. In my tests the best performance is produced with '-use-compact-aligner=true -align-functions-max-bytes=48 -align-cold-functions-max-bytes=32'.
(cherry picked from FBD6194092)
Summary:
Enhance the basic infrastructure for relocation mode for
AArch64 to make a reasonably large program work after reordering (gcc).
Detect jump table patterns and skip optimizing functions with jump
tables in AArch64, as those will require extra future effort to fully
decode. To make these work in relocation mode, we skip changing
the function body and introduce a mode to preserve even the original
nops. By not changing any local offsets in the function, the input
original jump tables should just work.
Functions with no jump tables are optimized with BB reordering. No other
optimizations have been tested.
(cherry picked from FBD6130117)
Summary:
Fix a bug in reconstruction of an optimal path. When calculating the
best path we need to take into account a path from new "last" node
to the current last node.
Add "-tsp-threshold" (defaults to 10) to control when the TSP
algorithm should be used.
(cherry picked from FBD6253461)
Summary:
As we deal with incomplete addresses in address-computing
sequences of code in AArch64, we found it is easier to handle them in
relocation mode in the presence of relocations.
Incomplete addresses may mislead BOLT into thinking there are
instructions referring to a basic block when, in fact, this may be the
base address of a data reference. If the relocation is present, we can
easily spot such cases.
This diff contains extensions in relocation mode to understand and
deal with AArch64 relocations. It also adds code to process data inside
functions as marked by AArch64 ABI (symbol table entries named "$d").
In our code, this is called constant islands handling. Last, it extends
bughunter with a "cross" mode, in which the host generates the binaries
and the user test them (uploading to the target), useful when debugging
in AArch64.
(cherry picked from FBD6024570)
Summary:
Add functionality to support reordering bzip2 compiled to
AArch64, with function splitting but without relocations:
* Expand the AArch64 backend to support inverting branches and
analyzing branches so BOLT reordering machinery is able to shuffle
blocks and fix branches correctly;
* Add a new pass named LongJmp to add stubs whenever code needs to
jump to the cold area, when using function splitting, because of the
limited target encoding capability in AArch64 (as a RISC architecture).
(cherry picked from FBD5748184)
Summary:
Add basic AArch64 read/write capability to be able to
disassemble bzip2 for AArch64 compiled with gcc 5.4.0 and write
it back after going through the basic BOLT pipeline with no block
reordering (NOPs/unreachable blocks get removed).
This is not for relocation mode.
(cherry picked from FBD5701994)
Summary:
A few improvements for hfsort+ algorithm. The goal of the diff is (i) to make the resulting function order more i-cache "friendly" and (ii) fix a bug with incorrect input edge weights. A specific list of changes is as follows:
- The "samples" field of CallGraph.Node should be at least the sum of incoming edge weights. Fixed with a new method CallGraph::adjustArcWeights()
- A new optimization pass for hfsort+ in which pairs of functions that call each other with very high probability (>=0.99) are always merged. This improves the resulting i-cache but may worsen i-TLB. See a new method HFSortPlus::runPassOne()
- Adjusted optimization goal to make the resulting ordering more i-cache "friendly", see HFSortPlus::expectedCalls and HFSortPlus::mergeGain
- Functions w/o samples are now reordered too (they're placed at the end of the list of hot functions). These functions do appear in the call graph, as some of their basic blocks have samples in the LBR dataset. See HfSortPlus::initializeClusters
(cherry picked from FBD6248850)
Summary:
If you attempted to use a function filter on a binary when in relocation mode, the resulting binary would probably crash. This is because we weren't calling fixBranches on all functions. This was breaking bughunter.sh
I also strengthened the validation of basic blocks. The cond branch should always be non-null when there are two successors.
(cherry picked from FBD6261930)
Summary:
Refactor basic block reordering code out of the BinaryFunction.
BinaryFunction::isSplit() is now checking if the first and the last
blocks in the layout belong to the same fragment. As a result,
it no longer returns true for functions that have their cold part
optimized away.
Change type for returned "size" from unsigned to size_t.
Fix lines over 80 characters long.
(cherry picked from FBD6250825)
Summary:
Move the indirect branch analysis code from BinaryFunction to MCInstrAnalysis/X86MCTargetDesc.cpp.
In the process of doing this, I've added an MCRegInfo to MCInstrAnalysis which allowed me to remove a bunch of extra method parameters. I've also had to refactor how BinaryFunction held on to instructions/offsets so that it would be easy to pass a sequence of instructions to the analysis code (rather than a map keyed by offset).
Note: I think there are a bunch of MCInstrAnalysis methods that have a BitVector output parameter that could be changed to a return value since the size of the vector is based on the number of registers, i.e. from MCRegisterInfo. I haven't done this in order to keep the diff a more manageable size.
(cherry picked from FBD6213556)
Summary:
Add support for reading value profiling info from perf data. This diff adds support in DataReader/DataAggregator for value profiling data. Each event is recorded as two Locations (a PC and an address/value) and a count.
For now, I'm assuming that the value profiling data is in the same file as the usual BOLT profiling data. Collecting both at the same time seems to work.
(cherry picked from FBD6076877)
Summary: Arc->AvgOffset can be used for function/block ordering to distinguish between calls from the beggining of a function and calls from the end of the function. This makes a difference for large functions.
(cherry picked from FBD6094221)
Summary:
This will give us the ability to print annotations in a more meaningful way. Especially annotations that could be interpreted in multiple ways. I've added one register name printer for liveness analysis. We can update the other dataflow annotations as needed.
I also noticed that BitVector annotations were leaking since they contain heap allocated memory. I made removeAnnotation call the annotation destructor explicitly to mitigate this but it won't fix the problem when annotations are just dropped en masse.
(cherry picked from FBD6105999)
Summary:
When we calculate maximum function size we only used to rely on the
symbol table information, and ignore function info coming from FDEs.
Invalid maximum function size can lead to code emission over
the code of neighbouring function.
Fix this by considering FDE functions when determining the maximum
function size.
(cherry picked from FBD6025613)
Summary:
This diff is a preparation for decoupling function disassembly,
profile association, and CFG construction phases.
We used to have multiple ways to mark conditional tail calls with
annotations or TailCallOffsets map. Since CTC information is affecting
the correctness, it is justifiable to have it as a operand class for
instruction with a destination (0 is a valid one).
"Offset" annotation now replaces "EdgeCountData" and
"IndirectBranchData" annotations to extract profile data for any
given instruction.
Inlining for small functions was broken in a presence of
profiled (annotated) instructions and hence I had to remove
"-inline-small-functions" from the test case.
Also fix an issue with UNDEF section for created __hot_start/__hot_end
symbols. Now the symbols use ABS section.
(cherry picked from FBD6087284)
Summary:
This is a replacement of a previous diff. The implemented metric
('graph distance') is not very useful at the moment but I plan to add
more relevant metrics in the subsequent diff. This diff fixes some
obvious problems and moves the call of CalcMetrics::printAll to the
right place.
(cherry picked from FBD6072312)
Summary:
Add support to output both function order and section order files
as the former is useful for offloading functions sorting and
the latter is useful for linker script generation:
-generate-function-order=<file>
-generate-link-sections=<file>
(cherry picked from FBD6078446)
Summary:
Change output of "-generate-function-order=<file>" to match expected
format used for a linker script:
* Prefix function names with ".text".
* Strip internal suffix from local function names. E.g. for function
with names "foo/1" and "foo/foo.c/1" we will only output "foo".
* Output (with indentation) duplicate names for folded functions.
(cherry picked from FBD6071020)
Summary:
If "-hot-text" options is specified and the input binary did not
have __hot_start/__hot_end symbols, then add them to the symbol table.
(cherry picked from FBD6027737)
Summary:
Several benchmarks (hhvm, compilers) show that 32 provides a good
balance between I-Cache performance and iTLB misses.
(cherry picked from FBD6026476)
Summary:
Small fix - align the end of the descriptor string as well,
since readelf will detect when it is not aligned and print an error
instead of printing BOLT version and command line.
(cherry picked from FBD6023643)
Summary:
Follow ELF spec for NOTE sections when writing bolt info.
Since tools such as "readelf -n" will not recognize a custom code
identifying our new note section, we use GNU "gold linker version"
note, tricking readelf into printing bolt info.
(cherry picked from FBD6010153)
Summary:
Check the build-id of the input binary against the build-id of
the binary used during profiling data collection with perf, as reported
in perf.data. If they differ, issue a warning, since the user should use
exactly the same binary. If we cannot determine the build-id of either
the input binary or the one registered in the input perf.data, cancel the
build-id check but print a log message.
(cherry picked from FBD6001917)
Summary: In some (weird) cases, a Function is marked 'split' but doesn't contain any 'cold' basic block. In that case, the size of the last basic block of the function is computed incorrectly. Hence, this fix.
(cherry picked from FBD6012963)
Summary:
Perf is now outputting one less space, which broke our previous
(flaky) assumptions about field separators when processing the output
file. Make it more resilient by accepting any number of spaces before
reading LBR entries.
(cherry picked from FBD6014941)
Summary:
The presence of ld-temp.o symbol is somewhat indeterministic.
I couldn't find out exactly when it's generated, it could be
related to LTO vs ThinLTO, but not always.
If the symbol is there, it could affect names of most
of functions in LTO binary. The status of the symbol
may change between the binary the profile was collected on,
and the binary BOLT is called on. As a result, we may mismatch
many function names.
It is safe to ignore this symbol.
(cherry picked from FBD5908955)
Summary: It's possible that two basic blocks being conidered for SCTC are in a loop in the CFG. In this case a block that is both a predecessor and a successor may have been processed and marked invalid by a previous iteration of the SCTC loop. We should skip rewriting in this case.
(cherry picked from FBD5886721)
Summary:
Move the data aggregator logic from our python script to
our C++ LLVM/BOLT libs. This has a dramatic reduction in processing
time for profiling data (from 45 minutes for HHVM to 5 minutes) because
we directly use BOLT as a disassembler in order to validate traces found
in the LBR and to add the fallthrough counts. Previously, the python
approach relied on parsing the output objdump to check traces.
(cherry picked from FBD5761313)
Summary:
If conditional branch has been converted to conditional tail call,
it may be considered for SCTC optimization later since it will
appear as a tail call. We have to make sure that the tail call
we are considering is not a conditional branch.
(cherry picked from FBD5884777)
Summary:
A cold part of a function can start with a landing pad. As a
result, this landing pad will have offset 0 from the start
of the corresponding FDE, and it wouldn't get registered by
exception-handling runtime.
The solution is to use a different landing pad base address
(LPStart), such as (FDE_start - 1).
(cherry picked from FBD5876561)
Summary:
Fix two bugs. First, stack pointer tracking, the dataflow
analysis, was converging to the "superposition" state (meaning that at
this point there are multiple and conflicting states) too early in case
the entry state in the BB was "empty" AND there was an SP computation in
the block. In these cases, we need to propagate an "empty" value as well
and wait for an iteration where the input is not empty (only entry BBs
start with a non-empty well-defined value). Previously, it was
propagating "superposition", meaning there is a conflict of states in
this block, which is not true, since the input is empty and, therefore,
there is no preceding state to justify a collision of states.
Second, if SPT failed and has no idea about the stack values in a block
(if it is in the superposition state at a given point in a BB), shrink
wrapping should not attempt to insert computation into those blocks
that we do not understand what is happening. Fix it to bail on those
cases.
(cherry picked from FBD5858402)
Summary:
Add support to read profiles collected without LBR. This
involves adapting our data aggregator perf2bolt and adding support
in llvm-bolt itself to read this data.
This patch also introduces different options to convert basic block
execution count to edge count, so BOLT can operate with its regular
algorithms to perform basic block layout. The most successful approach
is the default one.
(cherry picked from FBD5664735)
Summary:
No special handling is required for TLS relocations types,
and if we see them in the binary we can safely ignore those
types.
(cherry picked from FBD5853889)
Summary:
After SCTC optimization fixDoubleJumps() was relying on CFG information
on the number of successors of a basic block. It ignored the fact that
conditional tail call had a successor outside of the function and
deleted a containing basic block.
Discovered while testing old HHVM with disabled jump tables.
(cherry picked from FBD5752903)
Summary:
Exceptions tables for PIC may contain indirect type references
that are also encoded using relative addresses.
This diff adds support for such encodings. We read PIC-style
type info table, and write it using new encoding.
(cherry picked from FBD5716060)
Summary:
Add an option to optimize PLT calls:
-plt - optimize PLT calls (requires linking with -znow)
=none - do not optimize PLT calls
=hot - optimize executed (hot) PLT calls
=all - optimize all PLT calls
When optimized, the calls are converted to use GOT reference
indirectly. GOT entries are guaranteed to contain a valid
function pointer if lazy binding is disabled - hence the
requirement for linker's -znow option.
Note: we can add an entry to .dynamic and drop a requirement
for -znow if we were moving .dynamic to a new segment.
(cherry picked from FBD5579789)
Summary:
We used to print dyno-stats after instruction lowering
which was skewing our metrics as tail calls were no longer
recognized as calls for one thing. The fix is to control
the point at which dyno-stats printing pass is run and run
it immediately before instruction lowering. In the future we
may decide to run the pass before some other intervening pass.
(cherry picked from FBD5605639)
Summary:
Fix issue in memcpy where one of its entry points was getting
no profiling data and was wrongly considered cold, being put in the cold
region.
(cherry picked from FBD5569156)
Summary:
SCTC was deleting an unconditional branch to a block in the
cold area because it was the next block in the layout vector. Fix the
condition to only delete such branches when source and target are in
the same allocation area (either both hot or both cold).
(cherry picked from FBD5570300)
Summary:
While converting code from __builtin_unreachable() we were asserting
that a basic block with a conditional jump and a single CFG successor
was the last one before converting the jump to an unconditional one.
However, if that code was executed after a conditional tail call
conversion in the same function, the original last basic block
will no longer be the last one in the post-conversion layout.
I'm disabling the assertion since it doesn't seem worth it to add
extra checks for the basic block that used to be the last one.
(cherry picked from FBD5570298)
Summary:
* Improve profile matching for LTO binaries that don't match 100%.
* Fix profile matching for '.LTHUNK*' functions.
* Add external outgoing branches (calls) for profile validation.
There's an improvement for 100% match profile and for stale LTO
profile. However, we are still not fully closing the gap with
stale profile when LTO is enabled.
(NOTE: I haven't updated all test cases yet)
(cherry picked from FBD5529293)
Summary:
Fix a bug while reading LSDA address in PIC format. The base address was
wrong for PC-relative value. There's more work involved in making PIC
code with C++ exceptions work.
(cherry picked from FBD5538755)
Summary:
Minor change. Reformat the def-in, live-out register strings so that Stoke can parse
without doing preprocessing.
(cherry picked from FBD5537421)
Summary:
Function execution count is very important. When calculating metric, we
should care more about functions which are known to be executed.
The correlations between this metric and both CPU time is slightly improved
to be close to 96% and the correlation between this metric and Cache Miss
remains the same 96%.
Thanks the suggestion from Sergey!
(cherry picked from FBD5494720)
Summary:
BOLT needs to be configured with the LLVM
AArch64 backend. If the backend is linked into the LLVM
library, start processing AArch64 binaries.
(cherry picked from FBD5489369)
Summary:
Create new .symtab and .strtab sections, so we can change their
sizes and not only patch them. Remove local symbols and add symbols to
identify the cold part of split functions.
(cherry picked from FBD5345460)
Summary:
Current existing Jump-Distance Metric (Previously named Call-Distance) will ignore some traversals.
This modified version adds those missing traversals back.
The correlation remains the same: around 97% correlation with CPU and
Cache Miss (which implies that even though some traversals are ignored,
it doesn't affect correlation that much.)
(cherry picked from FBD5369653)
Summary:
Make shrink-wrapping more stable. Changes:
* Correctly detect landing pads at the dominance frontier, bailing
on such cases because we are not prepared to split LPs that are target
of a critical edge.
* Disable FOP's store removal by default - this is experimental and
shouldn t go to prod because removing a store that we failed to detect
it's actually necessary is disastrous. This pass currently doesn't
have a great impact on the number of stores reduced, so it is not a
problem. Most stores reduced are due shrink wrapping anyway.
* Fix stack access identification - correctly estimate memory length of
weird instructions, bail if we don't know.
* Make rules for shrink-wrapping more strict: cancel shrink wrapping on
a number of cases when we are not 100% sure that we are dealing with a
regular callee-saved register.
* Add basic block folding to SW. Sometimes when splitting critical edges
we create a lot of redundant BBs with the same instructions, same
successor but different predecessor. Fold all identical BBs created by
splitting critical edges.
* Change defaults: now the threshold used to determine when to perform
SW is more conservative, to be sure we are moving a spill to a colder
area. This effort, along with BB folding, helps us to avoid hurting
icache performance by indiscriminately increasing code size.
(cherry picked from FBD5315086)
Summary:
Designed a new metric, which shows 93.46% correltation with Cache Miss
and 86% correlation with CPU Time.
Definition:
One can get all the traversal path for each function. And for each traversal,
we will define a distance. The distance represents how far two connected
basic blocks are. Therefore, for each traversal, I will go through the
basic blocks one by one, until the end of the traversal and sum up the
distance for the neighboring basic blocks.
Distance between two connected basic blocks is the distance of the
centers of two blocks in the binary file.
(cherry picked from FBD5242526)
Summary:
Strobelight is getting confused by local symbols that we do not
update in relocation mode. These symbols were preserved by the linker in
relocation mode in order support emitting relocations against local
labels, but they are unused.
Issue a quick fix to this by detecting such symbols and setting their
value to zero.
This patch also fixes an issue with the symbol table that was assigning
the wrong section index to symbols associated with the .text section.
(cherry picked from FBD5271277)
Summary:
Rewrote the guts of buildCallGraph. There are two new options to control how the CG is created. UsePerfData controls whether we use the perf data directly to construct the CG for functions with a stale profile. IgnoreRecursiveCalls omits recursive calls from the CG since they might be skewing results unfairly for heavily recursive functions.
I've changed the way BinaryFunction::estimateHotSize() works. If the function is marked as split, I count the size of all the non-cold blocks. This gives a different but more accurate answer than the old method.
I've improved and updated the CG build stats with extra information.
(cherry picked from FBD5224183)
Summary:
Some PUSH instructions may contain memory addresses pushed to
the stack. If this memory address is from an object in the stack, cancel
further frame analysis for this function since it may be escaping a
variable.
This fixes a bug with deleting used stores (in frameopt) in hhvm trunk.
(cherry picked from FBD5270590)
Summary:
SCTC is currently asserting (my fault :-) when running in
combination with hot jump table entries optimization. This optimization
sets the frequency for edges connecting basic blocks it creates and jump
table targets based on the execution count of the original BB containing
the indirect jump.
This is OK as an estimation, but it breaks our assumption that the sum of
the frequency of preds edges equals to our BB frequency. This happens
because the frequency of the BB is rarely equal to its outgoing edges
frequency.
SCTC, in turn, was updating the execution count for BBs with tail calls
by subtracting the frequency count of predecessor edges. Because hot
jump table entries optimization broke the BB exec count = sum(preds freq)
invariant, SCTC was asserting.
To trigger this, the input program must have a jump table where each
entry contains a tail call. This happens in the HHVM binary for func
_ZN4HPHP11collections5issetEPNS_10ObjectDataEPKNS_10TypedValueE.
(cherry picked from FBD5222504)
Summary:
Add a new positional option onto bolt: "-print-function-statistics=<uint64>"
which prints information about block ordering for requested number of functions.
(cherry picked from FBD5105323)
Summary:
There's good news and bad news.
The good news is that this fixes the caching mechanism used by hfsort+ so that we always get the correct end results, i.e. the order is the same whether the cache is enabled or not.
The bad news is that it takes about the same amount of time as the original to run. (~6min)
The good news is that I can make some improvements on this implementation which I'll put up in another diff.
The problem with the old caching mechanism is that it was caching values that were dependent on adjacent sets of clusters. It only invalidated the clusters being merged and none of other clusters that might have been affected. This version computes the adjacency information up front and updates it after every merge, rather than recomputing it for each iteration. It uses the adjacency data to properly invalidate any cached values.
(cherry picked from FBD5203023)
Summary:
Don't treat conditional tail calls as branches for dynostats. Count
taken conditional tails calls as calls. Change SCTC to report dynamic
numbers after it is done.
(cherry picked from FBD5203708)
Summary: hfsort+ was trying to access the back() of an empty vector when no perf data is present. Just add a guard around that code.
(cherry picked from FBD5206962)
Summary:
Since we are stripping non-allocatable relocation sections from
the binary and adding new sections it changes section indices
in the binary. Some sections refer to other sections by their index
which is stored in sh_link or sh_info field. Hence we need to update
these field.
In the past update of indices was done ad-hoc and as we started
adding more complex updates to section header table the update
mechanism became broken in some cases. As a result, we were putting
wrong indices into sh_link/sh_info.
The broken case was discovered while investigating a problem with
a stripped BOLTed binary. In BOLTed binary .rela.plt was incorrectly
pointing to one of the debug sections and strip command removed
the debug section together with .rela section that was referencing it.
The new update mechanism computes complete old to new section index
mapping and updates sh_link/sh_info fields based on the mapping
before writing section header entries into an output file.
(cherry picked from FBD5207378)
Summary:
Split FrameAnalysis into FrameAnalysis and RegAnalysis, since
some optimizations only require register information about functions,
not frame information. Refactor callgraph walking code into the
CallGraphWalker class, allowing any analysis that depend on the call
graph to easily traverse it via a visitor pattern. Also fix
LivenessAnalysis, which was broken because it was not considering
registers read into callees and incorporating this into caller.
(cherry picked from FBD5177901)
Summary:
Add an implementation for shrink wrapping, a frame optimization
that moves callee-saved register spills from hot prologues to cold
successors.
(cherry picked from FBD4983706)
Summary:
Fix issues discovered while testing LTO mode with bfd linker:
* Correctly update absolute function references from code
with addend.
* Support .got.plt section generated by bfd linker.
* Support quirks of .tbss section.
* Don't ignore functions if the size in FDE doesn't match the
size in the symbol table. Instead keep processing using the
maximum indicated size.
(cherry picked from FBD5178831)
Summary:
Make hfsort+ algorithm deterministic.
We only had a test for hfsort. Since hfsort+ is going to be the default, I've added a test for that too.
(cherry picked from FBD5143143)
Summary:
Do some additional refactoring of the CallGraph class. Add a BinaryFunctionCallGraph class that has the BOLT specific bits. This is in preparation to moving the generic CallGraph class into a library that both BOLT and HHVM can use.
Make data members of CallGraph private and add the appropriate accessor methods.
(cherry picked from FBD5143468)
Summary:
Clang generates an empty debug location list, which doesn't make sense,
but we probably shouldn't assert on it and instead issue a warning
in verbosity mode. There is only a single empty location list in the
whole llvm binary.
(cherry picked from FBD5166666)
Summary:
I've factored out the call graph code from dataflow and function reordering code and done a few small renames/cleanups. I've also moved the function reordering pass into a separate file because it was starting to get big.
I've got more refactoring planned for hfsort/call graph but this is a start.
(cherry picked from FBD5140771)
Summary: I put the const_cast<BinaryFunction *>(this) on the wrong version of getBasicBlockAfter(). It's on the right one now.
(cherry picked from FBD5159127)
Summary:
Some DWARF tags (such as GNU_call_site and label) reference instruction
addresses in the input binary. When we update debug info we need to
update these tags too with new addresses.
Also fix base address used for calculation of output addresses in
relocation mode.
(cherry picked from FBD5155814)
Summary:
When producing address ranges and location lists for debug info
add a post-processing step that sorts them and merges adjacent
entries.
Fix a memory allocation/free issue for .debug_ranges section.
(cherry picked from FBD5130583)
Summary: Add -generate-function-order=<filename> option to write the computed function order to a file. We can read this order in later rather than recomputing each time we process a binary with BOLT.
(cherry picked from FBD5127915)
Summary:
Optinally add a .bolt_info notes section containing BOLT revision and command line args.
The new section is controlled by the -add-bolt-info flag which is on by default.
(cherry picked from FBD5125890)
Summary:
This diff is similar to Bill's diff for optimizing jump tables
(and is built on top of it), but it differs in the strategy used to
optimize the jump table. The previous approach loads the target address
from the jump table and compare it to check if it is a hot target. This
accomplishes branch misprediction reduction by promote the indirect jmp
to a (more predictable) direct jmp.
load %r10, JMPTABLE
cmp %r10, HOTTARGET
je HOTTARGET
ijmp [JMPTABLE + %index * scale]
The idea in this diff is instead to make dcache better by avoiding the
load of the jump table, leaving branch mispredictions as a secondary
target. To do this we compare the index used in the indirect jmp and if
it matches a known hot entry, it performs a direct jump to the target.
cmp %index, HOTINDEX
je CORRESPONDING_TARGET
ijmp [JMPTABLE + %index * scale]
The downside of this approach is that we may have multiple indices
associated with a single target, but we only have profiling to show
which targets are hot and we have no clue about which indices are hot.
INDEX TARGET
0 4004f8
8 4004f8
10 4003d0
18 4004f8
Profiling data:
TARGET COUNT
4004f8 10020
4003d0 17
In this example, we know 4004f8 is hot, but to make a direct call to it
we need to check for indices 0, 8 and 18 -- 3 comparisons instead of 1.
Therefore, once we know a target is hot, we must generate code to
compare against all possible indices associated with this target because
we don't know which index is the hot one (IF there's a hotter index).
cmp %index, 0
je 4004f8
cmp %index, 8
je 4004f8
cmp %index, 18
je 4004f8
(... up to N comparisons as in --indirect-call-promotion-topn=N )
ijmp [JMPTABLE + %index * scale]
(cherry picked from FBD5005620)
Summary:
SCTC was sometimes adding unconditional branches to fallthrough blocks.
This diff checks to see if the unconditional branch is really necessary, e.g.
it's not to a fallthrough block.
(cherry picked from FBD5098493)
Summary:
Multiple improvements to debug info handling:
* Add support for relocation mode.
* Speed-up processing.
* Reduce memory consumption.
* Bug fixes.
The high-level idea behind the new debug handling is that we don't save
intermediate state for ranges and location lists. Instead we depend
on function and basic block address transformations to update the info
as a final post-processing step.
For HHVM in non-relocation mode the peak memory went down from 55GB to 35GB. Processing time went from over 6 minutes to under 5 minutes.
(cherry picked from FBD5113431)
Summary:
This diff introduces a common infrastructure for performing
dataflow analyses in BinaryFunctions as well as a few analyses that are
useful in a variety of scenarios. The largest user of this
infrastructure so far is shrink wrapping, which will be added in a
separate diff.
(cherry picked from FBD4983671)
Summary:
When we see a compilation unit with continuous range on input,
it has two attributes: DW_AT_low_pc and DW_AT_high_pc. We convert the
range to a non-continuous one and change the attributes to
DW_AT_ranges and DW_AT_producer. However, gdb seems to expect
every compilation unit to have a base address specified via
DW_AT_low_pc, even when its value is always 0. Otherwise gdb will
not show proper debug info for such modules.
With this diff we produce DW_AT_ranges followed by DW_AT_low_pc.
The problem is that the first attribute takes DW_FORM_sec_offset
which is exactly 4 bytes, and in many cases we are left with
12 bytes to fill in. We used to fill this space with DW_AT_producer,
which took an arbitrary-length field. For DW_AT_low_pc we can
use a trick of using DW_FORM_udata (unsigned ULEB128 encoded
integer) which can take up to 12 bytes, even when the value is 0.
(cherry picked from FBD5109798)
Summary:
Add jump table support to ICP. The optimization is basically the same
as ICP for tail calls. The big difference is that the profiling data
comes from the jump table and the targets are local symbols rather than
global.
I've removed an instruction from ICP for tail calls. The code used to
have a conditional jump to a block with a direct jump to the target, i.e.
B1: cmp foo,(%rax)
jne B3
B2: jmp foo
B3: ...
this code is now:
B1: cmp foo,(%rax)
je foo
B2: ...
The other changes in this diff:
- Move ICP + new jump table support to separate file in Passes.
- Improve the CFG validation to handle jump tables.
- Fix the double jump peephole so that the successor of the modified
block is updated properly. Also make sure that any existing branches
in the block are modified to properly reflect the new CFG.
- Add an invocation of the double jump peephole to SCTC. This allows
us to remove a call to peepholes/UCE occurring after fixBranches() in
the pass manager.
- Miscellaneous cleanups to BOLT output.
(cherry picked from FBD4727757)
Summary:
GOLD linker removes .debug_aranges while generating .gdb_index.
Some tools however rely on the presence of this section.
Add an option to generate .debug_aranges if it was removed,
or keep it in the file if it was present.
Generally speaking .debug_aranges duplicates information present
in .gdb_index addresses table.
(cherry picked from FBD5084808)
Summary:
We had the ability to add allocatable sections before. This diff
expands this capability to non-allocatable sections.
(cherry picked from FBD5082018)
Summary:
When we have a conditional branch past the end of function (a result
of a call to__builtin_unreachable()), we replace the branch with nop,
but keep branch information for validation purposes. If that branch
has a recorded profile we mistakenly create an additional successor
to a containing basic block (a 3rd successor).
Instead of adding the branch to FTBranches list we should be adding
to IgnoredBranches.
(cherry picked from FBD4912840)
Summary:
While writing non-allocatable sections we had an assumption that the
size of such section is congruent to the alignment, as typically
such sections are a collections of fixed-sized elements. .gdb_index
breaks this assumption.
This diff removes the assertion that was triggered by a presence of
.gdb_index section, and makes sure that we insert a padding if we are
appending to a section with a size not congruent to section alignment.
(cherry picked from FBD4844553)
Summary:
Relocations can be created for non-allocatable (aka Note) sections.
To start using this for debug info, the emission has to be moved
earlier in the pipeline for relocation processing to kick in.
(cherry picked from FBD4835204)
Summary:
When we merge the original branch counts we have to make sure
both of them have a profile. Otherwise set the count to COUNT_NO_PROFILE.
The misprediction count should be 0.
(cherry picked from FBD4837774)
Summary:
I split some of this out from the jumptable diff since it fixes the
double jump peephole.
I've changed the pass manager so that UCE and peepholes are not called
after SCTC. I've incorporated a call to the double jump fixer to SCTC
since it is needed to fix things up afterwards.
While working on fixing the double jump peephole I discovered a few
useless conditional branches that could be removed as well. I highly
doubt that removing them will improve perf at all but it does seem
odd to leave in useless conditional branches.
There are also some minor logging improvements.
(cherry picked from FBD4751875)
Summary:
When inlining, if a callee has debug info and a caller does not
(i.e. a containing compilation unit was compiled without "-g"), we try
to update a nonexistent compilation unit. Instead we should skip
updating debug info in such cases.
Minor refactoring of line number emitting code.
(cherry picked from FBD4823982)
Summary:
Each BOLT-specific option now belongs to BoltCategory or BoltOptCategory.
Use alphabetical order for options in source code (does not affect
output).
The result is a cleaner output of "llvm-bolt -help" which does not
include any unrelated llvm options and is close to the following:
.....
BOLT generic options:
-data=<string> - <data file>
-dyno-stats - print execution info based on profile
-hot-text - hot text symbols support (relocation mode)
-o=<string> - <output file>
-relocs - relocation mode - use relocations to move functions in the binary
-update-debug-sections - update DWARF debug sections of the executable
-use-gnu-stack - use GNU_STACK program header for new segment (workaround for issues with strip/objcopy)
-use-old-text - re-use space in old .text if possible (relocation mode)
-v=<uint> - set verbosity level for diagnostic output
BOLT optimization options:
-align-blocks - try to align BBs inserting nops
-align-functions=<uint> - align functions at a given value (relocation mode)
-align-functions-max-bytes=<uint> - maximum number of bytes to use to align functions
-boost-macroops - try to boost macro-op fusions by avoiding the cache-line boundary
-eliminate-unreachable - eliminate unreachable code
-frame-opt - optimize stack frame accesses
......
(cherry picked from FBD4793684)
Summary:
If we specify "-relocs" flag and an input has no relocations we
proceed with assumptions that relocations were there and break the
binary.
Detect the condition above, and reject the input.
(cherry picked from FBD4761239)
Summary:
ICP was letting through call targets that weren't symbols. This diff
filters out the non-symbol targets before running ICP.
(cherry picked from FBD4735358)
Summary:
Add option '-print-only=func1,func2,...' to print only functions
of interest. The rest of the functions are still processed and
optimized (e.g. inlined), but only the ones on the list are printed.
(cherry picked from FBD4734610)
Summary:
In non-relocation mode we shouldn't attemtp to change ELF
entry point.
What made matters worse - it broke '-max-funcs=' and '-funcs=' options
since an entry function more often than not was excluded from the list
of processed functions, and we were setting entry point to 0.
(cherry picked from FBD4720044)
Summary:
Reduce verbosity of dynostats to make them more readable.
* Don't print "before" dynostats twice.
* Detect if dynostats have changed after optimization and print
before/after only if at least one metric have changed. Otherwise
just print dynostats once and indicate "no change".
* If any given metric hasn't changed, then print the difference as
"(=)" as opposed to (+0.0%).
(cherry picked from FBD4705920)
Summary:
While running on a recent test binary BOLT failed with an error. We were
trying to process '__hot_end' (which is not really a function), and asserted
that it had no basic blocks.
This diff marks functions with empty basic blocks list as non-simple since
there's no need to process them.
(cherry picked from FBD4696517)
Summary:
The stats for call sites that are not included in the call graph were broken.
The intention is to count the total number of call sites vs. the number of call sites that are ignored because they have targets that are not BinaryFunctions.
Also add a new test for hfsort.
(cherry picked from FBD4668631)
Summary:
Fix validateCFG to handle BBs that were generated from code that used
_builtin_unreachable().
Add -verify-cfg option to run CFG validation after every optimization
pass.
(cherry picked from FBD4641174)
Summary:
Sometimes a code written in assembly will have unmarked data (such as
constants) embedded into text.
Typically such data falls into a "padding" address space of a function.
This diffs detects such references, and adjusts the padding space to
prevent overwriting of code in data.
Note that in relocation mode we prefer to overwrite the original code
(-use-old-text) and thus cannot simply ignore data in text.
(cherry picked from FBD4662780)
Summary:
Calls to __builtin_unreachable() can result in a inconsistent CFG.
It was possible for basic block to end with a conditional branche
and have a single successor. Or there could exist non-terminated
basic block without successors.
We also often treated conditional jumps with destination past the end
of a function as conditional tail calls. This can be prevented
reliably at least when the byte past the end of the function does
not belong to the next function.
This diff includes several changes:
* At disassembly stage jumps past the end of a function are converted
into 'nops'. This is done only for cases when we can guarantee that
the jump is not a tail call. Conversion to nop is required since the
instruction could be referenced either by exception handling
tables and/or debug info. Nops are later removed.
* In CFG insert 'ret' into non-terminated basic blocks without
successors (this almost never happens).
* Conditional jumps at the end of the function are removed from
CFG. The block will still have a single successor.
* Cases where a destination of a jump instruction is the start
of the next function, are still conservatively handled as
(conditional) tail calls.
(cherry picked from FBD4655046)
Summary:
The new interface for handling Call Frame Information:
* CFI state at any point in a function (in CFG state) is defined by
CFI state at basic block entry and CFI instructions inside the
block. The state is independent of basic blocks layout order
(this is implied by CFG state but wasn't always true in the past).
* Use BinaryBasicBlock::getCFIStateAtInstr(const MCInst *Inst) to
get CFI state at any given instruction in the program.
* No need to call fixCFIState() after any given pass. fixCFIState()
is called only once during function finalization, and any function
transformations after that point are prohibited.
* When introducing new basic blocks, make sure CFI state at entry
is set correctly and matches CFI instructions in the basic block
(if any).
* When splitting basic blocks, use getCFIStateAtInstr() to get
a state at the split point, and set the new basic block's CFI
state to this value.
Introduce CFG_Finalized state to indicate that no further optimizations
are allowed on the function. This state is reached after we have synced
CFI instructions and updated EH info.
Rename "-print-after-fixup" option to "-print-finalized".
This diffs fixes CFI for cases when we split conditional tail calls,
and for indirect call promotion optimization.
(cherry picked from FBD4629307)
Summary:
Fix inconsistent override keyword usages and initializes a
missing field of a Relocation object when using braced initializers.
(cherry picked from FBD4622856)
Summary:
Add pass to strip 'repz' prefix from 'repz retq' sequence. The prefix
is not used in Intel CPUs afaik. The pass is on by default.
(cherry picked from FBD4610329)
Summary:
We use code skew in non-relocation mode since functions have fixed
addresses, and internal alignment has to be adjusted wrt the skew.
However in relocation mode it interferes with effective code
alignment, and has to be disabled. I missed it when was re-basing
the relocation diff.
(cherry picked from FBD4599670)
Summary:
In a prev diff I added an option to update jump tables in-place (on by default)
and accidentally broke the default handling of jump tables in relocation
mode. The update should be happening semi-automatically, but because
we ignore relocations for jump tables it wasn't happening (derp).
Since we mostly use '-jump-tables=move' this hasn't been noticed for
some time.
This diff gets rid of IgnoredRelocations and removes relocations
from a relocation set when they are no longer needed. If relocations
are created later for jump tables they are no longer ignored.
(cherry picked from FBD4595159)
Summary:
gcc5 can generate new types of relocations that give linker a freedom
to substitute instructions. These relocations are PC-relative, and
since we manually process such relocations they don't present
much of a problem.
Additionally, detect non-pc-relative access from code into a middle of
a function. Occasionally I've seen such code, but don't know exactly
how to trigger its generation. Just issue a warning for now.
(cherry picked from FBD4566473)
Summary:
To minimize size of the output code we should emit tail calls
that are as short as possible. For this we have to convert a synthetic
TAILJMPd into JMP_1 instruction. This should be one of the last passes
as most of analysis passes could break since tail calls will no longer
be marked as such.
The total size of the code is smaller, but not by much - hot text was
reduced by 192 bytes.
(cherry picked from FBD4557804)
Summary:
Some functions coming from assembly may not have been marked
with size. We assume the size to include all bytes up to
the next function/object in the file. As a result,
function body will include any padding inserted by the linker.
If linker inserts 0-value bytes this could be misinterpreted
as invalid instruction and BOLT will bail out on such functions
in non-relocation mode, and give up on a binary in relocation
mode.
This diff detects zero-padding, ignores it, and continues processing
as normal.
(cherry picked from FBD4528893)
Summary:
Whenever input binary is suspected to have been sanitized we print an error
message and exit. I've checked that "__asan_init*" symbol
presence is the most conservative way to detect "sanitization".
(cherry picked from FBD4525478)
Summary:
Re-write section header string table to reflect new names
given to sections. Old sections get ".bolt.org" prefix.
E.g. when we write ".eh_frame" section, we keep the old copy
but rename it to ".bolt.org.eh_frame".
Note: the new code section is named ".bolt.text" - it contains split
function bodies, while original ".text" name is left unchanged.
(cherry picked from FBD4524935)
Summary:
Perform indirect call promotion optimization in BOLT.
The code scans the instructions during CFG creation for all
indirect calls. Right now indirect tail calls are not handled
since the functions are marked not simple. The offsets of the
indirect calls are stored for later use by the ICP pass.
The indirect call promotion pass visits each indirect call and
examines the BranchData for each. If the most frequent targets
from that callsite exceed the specified threshold (default 90%),
the call is promoted. Otherwise, it is ignored. By default,
only one target is considered at each callsite.
When an candiate callsite is processed, we modify the callsite
to test for the most common call targets before calling through
the original generic call mechanism.
The CFG and layout are modified by ICP.
A few new command line options have been added:
-indirect-call-promotion
-indirect-call-promotion-threshold=<percentage>
-indirect-call-promotion-topn=<int>
The threshold is the minimum frequency of a call target needed
before ICP is triggered.
The topn option controls the number of targets to consider for
each callsite, e.g. ICP is triggered if topn=2 and the total
requency of the top two call targets exceeds the threshold.
Example of ICP:
C++ code:
int B_count = 0;
int C_count = 0;
struct A { virtual void foo() = 0; }
struct B : public A { virtual void foo() { ++B_count; }; };
struct C : public A { virtual void foo() { ++C_count; }; };
A* a = ...
a->foo();
...
original:
400863: 49 8b 07 mov (%r15),%rax
400866: 4c 89 ff mov %r15,%rdi
400869: ff 10 callq *(%rax)
40086b: 41 83 e6 01 and $0x1,%r14d
40086f: 4d 89 e6 mov %r12,%r14
400872: 4c 0f 44 f5 cmove %rbp,%r14
400876: 4c 89 f7 mov %r14,%rdi
...
after ICP:
40085e: 49 8b 07 mov (%r15),%rax
400861: 4c 89 ff mov %r15,%rdi
400864: 49 ba e0 0b 40 00 00 movabs $0x400be0,%r10
40086b: 00 00 00
40086e: 4c 3b 10 cmp (%rax),%r10
400871: 75 29 jne 40089c <main+0x9c>
400873: 41 ff d2 callq *%r10
400876: 41 83 e6 01 and $0x1,%r14d
40087a: 4d 89 e6 mov %r12,%r14
40087d: 4c 0f 44 f5 cmove %rbp,%r14
400881: 4c 89 f7 mov %r14,%rdi
...
40089c: ff 10 callq *(%rax)
40089e: eb d6 jmp 400876 <main+0x76>
(cherry picked from FBD3612218)
Summary:
Add an option to overwrite jump tables without moving and make it a
default:
-jump-tables - jump tables support (default=basic)
=none - do not optimize functions with jump tables
=basic - optimize functions with jump tables
=move - move jump tables to a separate section
=split - split jump tables section into hot and cold based on
function execution frequency
=aggressive - aggressively split jump tables section based on usage of
the tables
(cherry picked from FBD4448499)
Summary:
Add a new dataflow analysis to recover the value of RSP at a
given point of the program. This value is expressed as an offset from
the CFA. Use this information to detect redundant load in memory
accesses performed via RSP as well, not only RBP as done previously.
Bail when RSP value (as an offset of the CFA) can't be reliably
determined with a simple dataflow analysis.
(cherry picked from FBD4372261)
Summary:
Report stale functions percentage with respect to all profiled
functions instead of all simple functions in the binary.
The new reporting format should make it more apparent if the
profile is out-of-date. Compare:
BOLT-INFO: 341 (16.7% of all profiled) functions have invalid (possibly
stale) profile.
vs old:
BOLT-INFO: 341 (0.3%) functions have invalid (possibly stale) profile.
(cherry picked from FBD4451746)
Summary:
Due to a clowntown on my part we were generating wrong ranges
when an empty range was seen on input. We were basically expanding
the range to include all basic blocks following such range and setting
wrong sizes at the same time.
Add "-dump-cu" option to llvm-dwarfdump that allows to look at debug
info of a single compile unit only. Saves time if we are only interested
in a subset of information.
(cherry picked from FBD4430989)
Summary:
In-non relocation mode, when we run ICF the second time,
we fold the same functions again since they were not
removed from the function set. This diff marks them as
folded and ignores them during ICF optimization. Note
that we still want to optimize such functions since they
are potentially called from the code not covered by BOLT
in non-relocation mode.
Folded functions are also excluded from dyno stats with
this diff
Also print the number of times folded functions were called.
When 2 functions - f1() and f2() are folded, that number
would be min(call_frequency(f1), call_frequency(f2)).
(cherry picked from FBD4399993)
Summary:
Re-worked the way ICF operates. The pass now checks for more than just
call instructions, but also for all references including function
pointers. Jump tables are handled too.
(cherry picked from FBD4372491)
Summary:
This is a first attempt to perform data flow analyses on bolt
and try to rebuild the stack frame for functions. The goal of the frame
optimization pass is to detect instructions that are accessing stack and,
if loading values, evaluate whether this load is redundant and we can
substitute the memory operation for a register load or immediate load.
To find opportunities, this pass also builds a map of clobbered registers
by function, so we use this in our analysis at call sites. If a call site
is found out to not clobber a caller-saved register but the caller is
spilling it anyway to the stack (to comply with the ABI), we should
detect these cases and remove this unnecessary move.
(cherry picked from FBD4337238)
Summary:
An optimization to simplify conditional tail calls by removing unnecessary branches. It adds the following two command line options:
-simplify-conditional-tail-calls - simplify conditional tail calls by removing unnecessary jumps
-sctc-mode - mode for simplify conditional tail calls
=always - always perform sctc
=preserve - only perform sctc when branch direction is preserved
=heuristic - use branch prediction data to control sctc
This optimization considers both of the following cases:
foo: ...
jcc L1 original
...
L1: jmp bar # TAILJMP
->
foo: ...
jcc bar iff jcc L1 is expected
...
L1 is unreachable
OR
foo: ...
jcc L2
L1: jmp dest # TAILJMP
L2: ...
->
foo: jncc dest # TAILJMP
L2: ...
L1 is unreachable
For this particular case, the first basic block ends with a conditional branch and has two successors, one fall-through and one for when the condition is true. The target of the conditional is a basic block with a single unconditional branch (i.e. tail call) to another function. We don't care about the contents of the fall-through block.
(cherry picked from FBD3719617)
Summary:
Previously NamedRegionTimer's constructor was being called
with no local variable associated with it owing to a typo. We need a
local variable to keep track of the time spent in the scope. At the
end of the scope, the destructor will be called an then the timer will
stop.
(cherry picked from FBD4301844)
Summary:
As we begin to work on optimization passes for bolt, it is important to
keep track of the time spent in each of these to measure their
contribution to the time bolt takes to finish rewriting a program.
(cherry picked from FBD4301136)
Summary:
The CFI instructions parser in libDebugInfo was relying on
undefined behavior to parse operands by assuming the order function
parameters are evaluated in a function call site is defined (it is
not). This patch fix this and makes our clang and gcc tests agree.
It also fixes wrong LIT tests in our codebase with respect to the
order of DW_CFA_def_cfa operands.
(cherry picked from FBD4255227)
Summary:
Clang's Address Sanitizer caught this leak where MCAsmBackend
and MCObjectWriter instances were being created but not freed. Fix this.
(cherry picked from FBD4249941)
Summary:
This is part of a series of clean-up patches to make bolt
cleanly compile with clang 4.0. This patch fixes an error where clang
will fail to compile because it does not support passing a
const_iterator to std::vector<T>::emplace(Iter, ...).
(cherry picked from FBD4242546)
Summary:
This is part of a series of clean-up patches to make bolt
cleanly compile with clang 4.0. This patch fixes the following warning:
moving a temporary object prevents copy elision
(cherry picked from FBD4242236)
Summary:
This is part of a series of clean-up patches to make bolt
cleanly compile with clang 4.0. This patch fixes the following warning:
default label in switch which covers all enumeration values
(cherry picked from FBD4242168)
Summary:
Make BOLT resilient to changes in the LLVM's X86 target library
by not hardwiring the list of default CIE instructions, but detecting it
at run time.
(cherry picked from FBD4200982)
Summary:
In order to improve gdb experience with BOLT we have to make
sure the output file has a single .eh_frame section. Otherwise
gdb will use either old or new section for unwinding purposes.
This diff relocates the original .eh_frame section next to
the new one generated by LLVM. Later we merge two sections
into one and make sure only the newly created section has
.eh_frame name.
(cherry picked from FBD4203943)
Summary:
We used to patch an existing .eh_frame_hdr and append contents
for split functions at the end. However, this approach does not
work in relocation mode since function addresses change and split
functions will not necessarily be at the end.
Instead of patching and appending we generate the new .eh_frame_hdr
based on contents of old and new .eh_frame sections.
(cherry picked from FBD4180756)
Summary:
In a prev diff I disabled inclusion of FDEs for cold fragments that
we fail to write. The side effect of it was that we failed to
write FDE for the next function with a cold fragment since it
had the same assigned address that we had put in FailedAddresses.
The correct fix is to assign zero address to failed cold fragments
and ignore them when we write .eh_frame_hdr.
(cherry picked from FBD4156740)
Summary:
CFI instructions may live in CIEs or FDEs. CIEs hold common
instructions used across many FDEs. When replaying CFIs to the output
binary, llvm-bolt needs to replay both instructions from CIE and the
corresponding FDE for the function. However, some instructions need not
to be replayed because MCStreamer/MCDwarf and friends will write them
by default in the output CIE. This patch fix the code that tried to
recognize one of these default instructions but was failing, resulting
in an extra CFI instruction in each FDE we outputted. With this patch,
the output binary should be a bit smaller.
(cherry picked from FBD4194753)
Summary:
Modify the MC layer (MCDwarf.h|cpp) to understand CFI
instructions dealing with DWARF expressions. Add code to emit DWARF
expressions in MCDwarf. Change llvm-bolt to pass these CFI instructions
to streamer instead of bailing on them. Change -dump-eh-frame option in
llvm-bolt to dump the EH frame of the rewritten binary in addition to
the one in the original binary, allowing us to proper test this patch.
(cherry picked from FBD4194452)
Summary:
AVX-512 disassembler support in LLVM is not quite ready yet.
Before we feel more comfortable about it we disable processing
of all functions that use any EVEX-encoded instructions.
(cherry picked from FBD4028706)
Summary:
When we fail to write functions that are too big, we have to
effectively cancel their effect on exception handling by ignoring
their FDE entries in .eh_frame while writing .eh_frame_hdr.
This can happen to functions that we split too. In such cases
the cold part has its own FDE and we have to ignore that one too.
This doesn't happen very often - I've only seen one case on
hhvm binary, however it is a potential issue. The fix is to
add the cold part address to the list of failed-to-write
addresses.
(cherry picked from FBD3987984)
Summary:
Modified function discovery process to tolerate more functions and
symbols coming from assembly. The processing order now matches
the memory order of the functions (input symbol table is unsorted).
Added basic support for functions with multiple entries. When
a function references its internal address other than with
a branch instruction, that address could potentially escape.
We mark such addresses as entry points and make sure they
are treated as roots by unreachable code elimination.
Without relocations we have to mark multiple-entry functions
as non-simple.
(cherry picked from FBD3950243)
Summary:
Added support for jump tables in code compiled with "-fpic".
Code pattern generated for position-independent jump tables
is quite different, as is the format of the tables.
More details in comments.
Coverage increased slightly for a test, mostly due to the code
coming from external lib that was compiled with "-fpic".
(cherry picked from FBD3940771)
Summary:
Allow UCE when blocks have EH info. Since UCE may remove blocks
that are referenced from debugging info data structures, we don't
actually delete them. We just mark them with an "invalid" index
and store them in a different vector to be cleaned up later once
the BinaryFunction is destroyed. The debugging code just skips
any BBs that have an invalid index.
Eliminating blocks may also expose useless jmp instructions, i.e.
a jmp around a dead block could just be a fallthrough. I've added
a new routine to cleanup these jmps. Although, @maks is working on
changing fixBranches() so that it can be used instead.
(cherry picked from FBD3793259)
Summary:
Add level for "-jump-tables=<n>" option:
1 - all jump tables are output in the same section (default).
2 - basic splitting, if the table is used it is output to hot section
otherwise to cold one.
3 - aggressively split compound jump tables and collect profile for
all entries.
Option "-print-jump-tables" outputs all jump tables for debugging
and/or analyzing purposes. Use with "-jump-tables=3" to get profile
values for every entry in a jump table.
(cherry picked from FBD3912119)
Summary:
Insert ud2 instructions after indirect tailcalls to prevent the CPU from
decoding instructions following the callsite.
A simple counter in the peephole pass shows 3260 tail call traps inserted.
(cherry picked from FBD3859737)
Summary:
Get rid of all uses of getIndex/getLayoutIndex/getOffset outside of BinaryFunction.
Also made some other offset related methods private.
(cherry picked from FBD3861968)
Summary:
Add -print-sorted-by and -print-sorted-by-order command line options.
The first option takes a list of dyno stats keys used to sort functions
that are printed at the end of all optimization passes. Only the top
100 functions are printed. The -print-sorted-by-order option can be
either ascending or descending (descending is the default).
(cherry picked from FBD3898818)
Summary:
While working on PLT dyno stats I've noticed that we were missing
BinaryFunctions for some symbols that were not PLT. Upon closer inspection
turned out that those symbols were marked as zero-sized functions in
symbol table, but they had duplicates with non-zero size. Since the
zero-size symbols were preceding other duplicates, we were not creating
BinaryFunction for them and they were not added as duplicates.
The 2 most prominent functions that were missing for a test were free() and
malloc(). There's not much to optimize in these functions, but they were
contributing quite significantly to dyno stats.
As a result dyno stats for this test needed an adjustment.
Also several assembly functions (e.g. _init()) had zero size, and now we
set the size to the max size and start processing those. It's good for
coverage but will not affect the performance.
(cherry picked from FBD3874622)
Summary:
Option "-jump-tables=1" enables experimental support for jump tables.
The option hasn't been tested with optimizations other than block
re-ordering.
Only non-PIC jump tables are supported at the moment.
(cherry picked from FBD3867849)
Summary:
This is just a bit of refactoring to make sure that BinaryFunction goes
through methods to get at the state in BinaryBasicBlock. I did this so
that changing the way Index/LayoutIndex/Valid works will be easier.
(cherry picked from FBD3860899)
Summary:
Add "-reorder-blocks=cluster-shuffle" for performance experiments.
Use "-bolt-seed=<N>" to set a randomization seed.
(cherry picked from FBD3851035)
Summary:
Switch table can contain __builtin_unreachable(). As a result,
a compiler may place an entry into a jump table that contains
an address immediately past the last instruction in the function.
Sometimes it may coincide with a start of the next function in
the binary. Thus when we check for switch tables in such cases
we have to check more than a single entry until we see either
an address inside containing function or some address outside
different from the address past the last instruction.
Additonally, don't stop disassembly after discovering that the
function was not simple. We need to detect all outside
references whenever possible.
(cherry picked from FBD3850825)
Summary:
Replace jumps to other unconditional jumps with the final
destination, e.g.
B0: ...
jmp B1 (or jcc B1)
B1: jmp B2
->
B0: ...
jmp B2 (or jcc B1)
This peephole removes 8928 double jumps from a test binary.
Note: after filtering out double jumps found in EH code and infinite
loops, the number of double jumps patched is 49 (24 for a clang
compiled test). The 24 in the clang build are all from external
libraries which have probably been compiled with gcc. This peephole
is still useful for cleaning up after ICP though.
(cherry picked from FBD3815420)
Summary:
I've added dyno stats printing per pass so we can see the results
of each optimization pass on the stats. I've also factored out the
post pass function printing code since it was pretty much the same
after each pass.
(cherry picked from FBD3843587)
Summary:
For now we make SCTC a special pass that runs at the end of all
optimizations and transformations right after fixupBranches().
Since it's the last pass, it has to do its own UCE.
(cherry picked from FBD3838051)
Summary:
Add "-dyno-stats" option that prints instruction stats based on
the execution profile similar to below:
BOLT-INFO: program-wide dynostats after optimizations:
executed forward branches : 109706407 (+8.1%)
taken forward branches : 13769074 (-55.5%)
executed backward branches : 24517582 (-25.0%)
taken backward branches : 15330256 (-27.2%)
executed unconditional branches : 6009826 (-35.5%)
function calls : 17192114 (+0.0%)
executed instructions : 837733057 (-0.4%)
total branches : 140233815 (-2.3%)
taken branches : 35109156 (-42.8%)
Also fixed pseudo instruction discrepancies and added assertions
for BinaryBasicBlock::getNumPseudos() to make sure the number is
synchronized with real number of pseudo instructions.
(cherry picked from FBD3826995)
Summary:
The CFG represents "the ultimate source of truth". Transformations
on functions and blocks have to update the CFG and fixBranches() would
make sure the correct branch instructions are inserted at the end of
basic blocks (or removed when necessary).
We do require a conditional branch at the end of the basic block if
the block has 2 successors as CFG currently lacks the conditional
code support (it will probably stay that way). We only use this
branch instruction for its conditional code, the destination is
determined by CFG - first successor representing true/taken branch,
while the second successor - false/fall-through branch.
When we reverse the branch condition, the CFG is updated accordingly.
The previous version used to insert jumps after some terminating
instructions sometimes resulting in a larger code than needed. As a
result with the new version 1 extra function becomes overwritten for
HHVM binary.
With this diff we also convert conditional branches with one successor
(result of code from __builtin_unreachable()) into unconditional
jumps.
(cherry picked from FBD3802062)
Summary:
This will make it easier to run experiments with the same baseline
BOLT binary but different command line options.
(cherry picked from FBD3831978)
Summary:
A number of fixes/enhancements to inline-small-functions
- Fixed size estimateHotSize to use computeCodeSize instead of the original layout offsets.
- Added -print-inline option to dump CFGs for functions that have been modified by inlining.
- Added flag to force consideration of functions without any profiling info (mostly for testing)
- Updated debug line info for inlined functions.
- Ignore the number of pseudo instructions when checking for candidates of suitable size.
Misc changes
- Moved most print flags to BinaryPasses.cpp
(cherry picked from FBD3812658)
Summary:
A previous diff accidentally disabled tail call conversion.
Additionally some test cases relied on output of "-v=2". Fix those.
(cherry picked from FBD3823760)
Summary:
I've added a verbosity level to help keep the BOLT spewage to a minimum.
The default level is pretty terse now, level 1 is closer to the original,
I've saved level 2 for the noisiest of messages. Error messages should
never be suppressed by the verbosity level only warnings and info messages.
The rational behind stream usage is as follows:
outs() for info and debugging controlled by command line flags.
errs() for errors and warnings.
dbgs() for output within DEBUG().
With the exception of a few of the level 2 messages I don't have any strong feelings about the others.
(cherry picked from FBD3814259)
Summary:
While creating remember_state/restore_state CFI sequences, we
were always placing remember_state instruction into the first
basic block. However, when we have hot-cold splitting, the cold
part has and independent FDE entry in .eh_frame, and thus the
restore_state instruction was missing its counter part.
The fix is to adjust the basic block that is used for placing
remember_state instruction whenever we see the hot-cold split
boundary.
(cherry picked from FBD3767102)
Summary:
Analyze indirect branches and convert them into indirect
tail calls when possible. We analyze the memory contents
when the address could be calculated statically and also
detect epilogue code.
(cherry picked from FBD3754395)
Summary:
We were applying padding to the calculated address but were never
writing it to a file triggering an assertion for cases when
.gcc_except_table size wasn't multiple of 4.
(cherry picked from FBD3744638)
Summary:
We only need ClusterEdges in reordering algorithm optimized for
branches and the computation is quite resource-hungry, thus it
makes sense to only do it when needed.
Some refactoring too.
(cherry picked from FBD3721107)
Summary:
Operands in the initial instruction stream should all have immediate operands
for instructions that can be shortened. But if a BOLT optimization pass adds
one of these instructions with a symbolic operand, the shortening operation
will assert. This diff adds checks to make sure that the operands are
immediate.
I've also disabled shortening pass by default since it won't really be needed
until ICP is submitted. It will still run at CFG creation time.
(cherry picked from FBD3610646)
Summary:
Add the following info the graphviz CFG dump:
- Edges are labeled with the jmp instruction that leads to that edge.
- Edges include the count and misprediction count.
- Nodes have (offset, BB index, BB layout index)
- Nodes optionally have tooltips which contain the code of the basic block.
(enabled with -dot-tooltip-code)
- Added dashed edges to landing pads.
(cherry picked from FBD3646568)
Summary:
Avoid referring to BinaryFunction's by name.
Functions could be found by MCSymbol using
BinaryContext::getFunctionForSymbol().
(cherry picked from FBD3707685)
Summary:
Eliminated BinaryFunction::getName(). The function was confusing since
the name is ambigous. Instead we have BinaryFunction::getPrintName()
used for printing and whenever unique string identifier is needed
one can use getSymbol()->getName(). In the next diff I'll have
a map from MCSymbol to BinaryFunction in BinaryContext to facilitate
function lookup from instruction operand expressions.
There's one bug fixed where the function was called only under assert()
in ICF::foldFunction().
For output we update all symbols associated with the function. At the
moment it has no effect on the generated binary but in the future we
would like to have all symbols in the symbol table updated.
(cherry picked from FBD3704790)
Summary:
This adds functionality for a more aggressive inlining pass, that can
inline tail calls and functions with more than one basic block.
(cherry picked from FBD3677856)
Summary:
Add three new MCOperand types: Annotation, LandingPad and GnuArgsSize.
Annotation is used for associating random data with MCInsts. Clients can
construct their own annotation types (subclassed from MCAnnotation) and
associate them with instructions. Annotations are looked up by string keys.
Annotations can be added, removed and queried using an instance of the
MCInstrAnalysis class.
The LandingPad operand is a MCSymbol, uint64_t pair used to encode exception
handling information for call instructions.
GnuArgsSize is used to annotate calls with the DW_CFA_GNU_args_size attribute.
(cherry picked from FBD3597877)
Summary:
BOLT attempts to convert jumps that serve as tail calls to dedicated tail call
instructions, but this is impossible when the jump is conditional because there is
no corresponding tail call instruction. This was causing the creation of a duplicate
fall-through edge for basic blocks terminated with a conditional jump serving as
a tail call when there is profile data available for the non-taken branch. In this
case, the first fall-through edge had a count taken from the profile data, while
the second has a count computed (incorrectly) by
BinaryFunction::inferFallThroughCounts.
(cherry picked from FBD3560504)
Summary:
LLVM was missing assembler print string for indirect tail
calls which are synthetic instructions created by us.
(cherry picked from FBD3640197)
Summary:
This diff adds a number of methods to BinaryFunction that can be used to edit the CFG after it is created.
The basic public functions are:
- createBasicBlock - create a new block that is not inserted into the CFG.
- insertBasicBlocks - insert a range of blocks (made with createBasicBlock) into the CFG.
- updateLayout - update the CFG layout (either by inserting new blocks at a certain point or recomputing the entire layout).
- fixFallthroughBranch - add a direct jump to the fallthrough successor for a given block.
There are a number of private helper functions used to implement the above.
This was split off the ICP diff to simplify it a bit.
(cherry picked from FBD3611313)
Summary:
This algorithm is similar to our main clustering algorithm but uses
a different heuristic for selecting edges to become fall-throughs.
The weight of an edge is calculated as the win in branches if we choose
to layout this edge as a fall-through. For example, the edges A -> B with
execution count 100 and A -> C with execution count 500 (where B and C
are the only successors of A) have weights -400 and +400 respectively.
(cherry picked from FBD3606591)
Summary:
Added an ICF pass to BOLT, that can recognize identical functions
and replace references to these functions with references to just one
representative.
(cherry picked from FBD3460297)
Summary:
I've factored out the instruction printing and size computation routines to
methods on BinaryContext. I've also added some more debug print functions.
This was split off the ICP diff to simplify it a bit.
(cherry picked from FBD3610690)
Summary:
Instructions that load data from the a read-only data section and their
target address can be computed statically (e.g. RIP-relative addressing)
are modified to corresponding instructions that use immediate operands.
We apply the transformation only when the resulting instruction will have
smaller or equal size.
(cherry picked from FBD3397112)
Summary:
Loop detection for the CFG data structure. Added a GraphTraits
specialization for BOLT's CFG that allows us to use LLVM's loop
detection interface.
(cherry picked from FBD3604837)
Summary:
Shorten when a mov instruction has a 64-bit immediate that can be repesented as
a sign extended 32-bit number, use the smaller mov instruction (MOV64ri -> MOV64ri32).
Add peephole optimization pass that does instruction shortening.
(cherry picked from FBD3603099)
Summary:
Generate short versions of branch instructions by default and rely on
relaxation to produce longer versions when needed.
Also produce short versions of arithmetic instructions if immediate
fits into one byte. This was only triggered once on HHVM binary.
(cherry picked from FBD3591466)
Summary:
patchELFPHDRTable was asserting that it could not find an entry
for .eh_frame_hdr in SectionMapInfo when no functions were modified
by BOLT.
This just changes code to skip modifying GNU_EH_FRAME program headers
hen SectionMapInfo is empty. The existing header is copied and written
instead.
(cherry picked from FBD3557481)
Summary:
If a profile data was collected on a stripped binary but an input
to BOLT is unstripped, we would use a different mangling scheme for
local functions and ignore their profiles. To solve the issue this
diff adds alternative name for all local functions such that one
of the names would match the name in the profile.
If the input binary was stripped, we reject it, unless "-allow-stripped"
option was passed. It's more complicated to do a matching in this case
since we have less information than at the time of profile collection.
It's also not that simple to tell if the profile was gathered on a
stripped binary (in which case we would have no issue matching data).
(cherry picked from FBD3548012)
Summary:
Store the basic block index inside the BinaryBasicBlock instead of a map in BinaryFunction.
This cut another 15-20 sec. from the processing time for hhvm.
(cherry picked from FBD3533606)
Summary:
Use unordered_map instead of map in ReorderAlgorithm and BinaryFunction::BasicBlockIndices.
Cuts about 30sec off the processing time for the hhvm binary. (~8.5 min to ~8min)
(cherry picked from FBD3530910)
Summary:
This fixes the initialization of basic block execution counts, where
we should skip edges to the first basic block but we were not
skipping the corresponding profile info.
Also, I removed a check that was done twice.
(cherry picked from FBD3519265)
Summary:
I noticed the BinaryFunction::viewGraph() method that hadn't been implemented
and decided I could use a simple DOT dumper for CFGs while working on the indirect
call optimization.
I've implemented the bare minimum for the dumper. It's just nodes+BB labels with
dges. We can add more detailed information as needed/desired.
(cherry picked from FBD3509326)
Summary:
Added perf2bolt functionality for extracting branch records
with histories of previous branches. The length of the histories
is user defined, and the default is 0 (previous functionality). Also,
DataReader can parse perf2bolt output with histories.
Note: creating profile data with long histories can increase their
size significantly (2x for history of length 1, 3x for length 2 etc).
(cherry picked from FBD3473983)
Summary:
When a conditional jump is followed by one or more no-ops, the
destination of fall-through branch was recorded as the first no-op in
FuncBranchInfo. However the fall-through basic block after the jump
starts after the no-ops, so the profile data could not match the CFG
and was ignored.
(cherry picked from FBD3496084)
Summary:
The various reorder and clustering algorithms have been refactored
into separate classes, so that it is easier to add new algorithms and/or
change the logic of algorithm selection.
(cherry picked from FBD3473656)
Summary:
With ICF optimization in the linker we were getting mismatches of
function names in .fdata and BinaryFunction name. This diff adds
support for multiple function names for BinaryFunction and
does a match against all possible names for the profile.
(cherry picked from FBD3466215)
Summary:
Verify profile data for a function and reject if there are branches
that don't correspond to any branches in the function CFG. Note that
we have to ignore branches resulting from recursive calls.
Fix printing instruction offsets in disassembled state.
Allow function to have non-zero execution count even if we don't
have branch information.
(cherry picked from FBD3451596)
Summary:
Print total number of functions/objects that have profile
and add new options:
-print - print the list of objects with count to stderr
=none - do not print objects/functions
=exec - print functions sorted by execution count
=branches - print functions sorted by total branch count
-q - do not print merged data to stdout
(cherry picked from FBD3442288)
Summary: This will help optimization passes that need to modify the CFG after it is constructed. Otherwise, the BinaryBasicBlock pointers stored in the layout, successors and predecessors would need to be modified every time a new basic block is created.
(cherry picked from FBD3403372)
Summary:
Turn on -fix-debuginfo-large-functions by default.
In the process of testing I've discovered that we output cold code
for functions that were too large to be emitted. Fixed that.
(cherry picked from FBD3372697)
Summary:
Assembly functions could have no corresponding DW_AT_subprogram
entries, yet they are represented in module ranges (and .debug_aranges)
and will have line number information. Make sure we update those.
Eliminated unnecessary data structures and optimized some passes.
For .debug_loc unused location entries are no longer processed
resulting in smaller output files.
Overall it's a small processing time improvement and memory imporement.
(cherry picked from FBD3362540)
Summary: The inference algorithm for counts of fall through edges takes possible jumps to landing pad blocks into account. Also, the landing pad block execution counts are updated using profile data.
(cherry picked from FBD3350727)
Summary:
Clang uses different attribute for high_pc which
was incompatible with the way we were updating
ranges. This diff fixes it.
(cherry picked from FBD3345537)
Summary:
* Fix several cases for handling debug info:
- properly update CU DW_AT_ranges for function with folded body
due to ICF optimization
- convert ranges to DW_AT_ranges from hi/low PC for all DIEs
- add support for [a, a) range
- update CU ranges even when there are no functions registered
* Overwrite .debug_ranges section instead of appending.
* Convert assertions in debug info handling part into warnings.
(cherry picked from FBD3339383)
Summary:
Some compile unit DIEs might be missing DW_AT_ranges because they were
compiled without "-ffunction-sections" option. This diff adds the
attribute to all compile units.
If the section is not present, we need to create it. Will do it in a
separate diff.
(cherry picked from FBD3314984)
Summary:
Overwrite contents of .debug_line section since we don't reference
the original contents anymore. This saves ~100MB of HHVM binary.
(cherry picked from FBD3314917)
Summary:
A simple optimization to prevent branch misprediction for tail calls.
Convert the sequence:
j<cc> L1
...
L1: jmp foo # tail call
into:
j<cc> foo
but only if 'j<cc> foo' turns out to be a forward branch.
(cherry picked from FBD3234207)
Summary:
While emitting debug lines for a function we don't overwrite, we
don't have a code section context that is needed by default
writing routine. Hence we have to emit end_sequence after the
last address, not at the end of section.
(cherry picked from FBD3291533)
Summary:
Added an optimization pass of inlining calls to small functions (with only one
basic block). Inlining is done in a very simple way, inserting instructions to
simulate the changes to the stack pointer that call/ret would make before/after the
inlined function executes. Also, the heuristic prefers to inline calls that happen
in the hottest blocks (by looking at their execution count). Calls in cold blocks are
ignored.
(cherry picked from FBD3233516)
Summary:
Many functions (around 600) in the HHVM binary are simply
a single unconditional jump instruction to another function. These can
be trivially optimized by modifying the call sites to directly call the
branch target instead (because it also happens with more than one jump
in sequence, we do it iteratively).
This diff also adds a very simple analysis/optimization pass system in
which this pass is the first one to be implemented. A follow-up to this
could be to move the current optimizations to other passes.
(cherry picked from FBD3211138)
Summary:
Many functions (around 600) in the HHVM binary are simply
a single unconditional jump instruction to another function. These can
be trivially optimized by modifying the call sites to directly call the
branch target instead (because it also happens with more than one jump
in sequence, we do it iteratively).
This diff also adds a very simple analysis/optimization pass system in
which this pass is the first one to be implemented. A follow-up to this
could be to move the current optimizations to other passes.
(cherry picked from FBD3211138)
Summary:
Fix the error message by not printing it :)
Explanation: a previous diff accidentally removed this error message from within
the DEBUG macro, and it's expected that we'll have a bunch of them since a lot
of the DIEs we try to update are empty or meaningless. For instance (and mainly), there
is a huge number of lexical block DIEs with no attributes in .debug_info.
In the first phase of collecting debugging info, we store the offsets of all
these DIEs, only later to realize that we cannot update their address
ranges because they have none.
A better fix would be to check this earlier and not store offsets of DIEs
we cannot update to begin with.
(cherry picked from FBD3236923)
Summary:
A lot of the space in the merged .fdata is taken by branches
to and from [heap], which is jitted code. On different machines,
or during different runs, jitted addresses are all different.
We don't use these addresses, but we need branch info to get
accurate function call counts.
This diff treats all [heap] addresses the same, resulting in a
simplified merged file. The size of the compressed file decreased
from 70MB to 8MB.
(cherry picked from FBD3233943)
Summary:
In a test binary some functions are placed in a segment
preceding the segment containing .text section. As a result,
we were miscalculating maximum function size as the calculation
was based on addresses only.
This diff fixes the calculation by checking if symbol after function
belongs to the same section. If it does not, then we set the maximum
function size based on the size of the containing section and not
on the address distance to the next symbol.
(cherry picked from FBD3229205)
Summary:
Added option "-break-funcs=func1,func2,...." to coredump in any
given function by introducing ud2 sequence at the beginning of the
function. Useful for debugging and validating stack traces.
Also renamed options containing "_" to use "-" instead.
Also run hhvm test with "-update-debug-sections".
(cherry picked from FBD3210248)
Summary:
Make sure we can install all tools needed for processing
BOLT .fdata files such as perf2bolt, merge-fdata, etc.
(cherry picked from FBD3223477)
Summary:
merge-fdata tool takes multiple .fdata files and outputs to stdout
combined fdata. Takes about 2 seconds per each additional .fdata
file with hhvm production data.
(cherry picked from FBD3216430)
Summary:
Splitting option now has different meanings/values. Since landing pads
are mostly always cold/frozen, we should split them before anything
else (we still check the execution count is 0). That's value '1'.
Everything else goes on top of that and has increased value (2 - large
functions, 3 - everything).
Sorting was non-deterministic and somewhat broken for functions
with EH ranges. Fixed that and added '-split-all-cold' option to
outline all 0-count blocks.
Fixed compilation of test cases. After my last commit the binaries
were linked to wrong source files (i.e. debug info). Had to rebuild
the binaries from updated sources.
(cherry picked from FBD3209369)
Summary:
GNU_args_size is a special kind of CFI that tells runtime to adjust
%rsp when control is passed to a landing pad. It is used for annotating
call instructions that pass (extra) parameters on the stack and there's
a corresponding landing pad.
It is also special in a way that its value is not handled by
DW_CFA_remember_state/DW_CFA_restore_state instruction sequence
that we utilize to restore the state after block re-ordering.
This diff adds association of call instructions with GNU_args_size value
when it's used. If the function does not use GNU_args_size, there is
no overhead. Otherwise, we regenerate GNU_args_size instruction during
code emission, i.e. after all optimizations and block-reordering.
(cherry picked from FBD3201322)
Summary:
Simple functions which we fail to rewrite after optimizations were
having wrong debugging information because the latter would reflect the optimized
version of the function.
There are only 48 functions (at this time) in this situation in the HHVM binary.
The simple fix is to add another full pass. Another more complicated path, which will
be more efficient, is to reset only the BinaryContext and emit again, but then we need
to recreate all symbols in the new MCContext and update the pointers. I started
taking this path but it started getting too complicated for only those 48 functions
(needed to create a new map of global symbols, recreate landing pads - which needed
to have the internal intermediate labels in the functions kept to be updated too, etc).
Because the overhead is quite large (another full emission pass - around 4m30s here)
and the impact is small I put this behind a new
command-line flag which is off by default: -fix-debuginfo-large-functions.
(cherry picked from FBD3166576)
Summary:
Update address ranges of inlined functions and try/catch blocks.
This was missing and lead gdb to show weird information in a core dump we inspected
because of the several nestings of inline in the call stack.
This is very similar to Lexical Blocks, so the change is to basically generalize that
code to do the same for DW_AT_try_block, DW_AT_catch_block and DW_AT_inlined_subroutine.
(cherry picked from FBD3169417)
Summary:
readelf was showing some errors because we weren't updating DIEs that were not shallow
in the DIE tree, or DIEs of functions with addresses we don't recognize (mostly functions with
address 0, which could have been removed by the Linker Script but still have debugging information
there). These DIEs need to be updated because their abbreviations are patched.
(cherry picked from FBD3159335)
Summary:
We were updating only one DIE per function, but because the Linker Script may map
multiple functions to the same address this would cause us to generate invalid debug info
(as some DIEs weren't updated but their abbreviations were changed).
(cherry picked from FBD3157263)
Summary:
Non-simple functions aren't emitted, and thus didn't have line number information
emitted. This diff emits it for those functions by extending LLVM's generation of the line number program to allow for absolute addresses (it is wholly symbolic), then iterating over the relevant line tables from the input and appending entries with absolute addresses to the line tables to be emited.
This still leaves the simple but not overwritten functions unhandled (there were 48 in HHVM in
my last run). However, I think that to fix them we'd need another pass, since by the time we
realize a simple function wont't fit, debug line info was already written to the output.
(cherry picked from FBD3148468)
Summary:
Summary: Update DWARF location lists in .debug_loc and pointers to
them in .debug_info so that gdb can print variables which change
location during their lifetime.
The following changes were made:
- Refactored BasicBlockOffsetRanges to allow ranges to be tied to binary information (so that we can reuse it for location lists)
- Implemented range compression optimization in BasicBlockOffsetRanges (needed otherwise too much data was being generated).
- Added representation for location lists (LocationList.h, BinaryContext.h)
- Implemented .debug_loc serializer that keeps the updated offsets (DebugLocWriter.{h,cpp})
- After disassembly, traverse entries in .debug_loc and save them in context (BinaryContext.cpp)
- After optimizations, serialize .debug_loc and update pointers in .debug_info (RewriteInstance.cpp)
(cherry picked from FBD3130682)
Summary:
Add a parameter value to "-split-functions=" option to allow splitting
only when the function is too large to fit:
0 - never split
1 - split if too large to fit
2 - always split
We may use this option when the profile data is not very precise.
In that case excessive splitting may increase iTLB misses.
(cherry picked from FBD3137700)
Summary:
This fixes a problem in which bolt was generating a malformed .debug_info
section on the bzip2 binary. The bug was the following:
- A simple and a non-simple function shared an abbreviation
- The abbreviation was patched to contain DW_AT_ranges because of the simple function
- The non-simple function's data was not updated, but then it didn't match the
layout expected by the abbreviation anymore
And because we were already creating an address ranges list in .debug_ranges even
for non-simple functions, it doesn't make sense not to use it anyway.
(cherry picked from FBD3129219)
Summary:
Updates DWARF lexical blocks address ranges in the output binary after optimizations.
This is similar to updating function address ranges except that the ranges representation needs
to be more general, since address ranges can begin or end in the middle of a basic block.
The following changes were made:
- Added a data structure for iterating over the basic blocks that intersect an address range: BasicBlockTable.h
- Added some more bookkeeping in BinaryBasicBlock. Basically, I needed to keep track of the block's size in the input binary as well as its address in the output binary. This information is mostly set by BinaryFunction after disassembly.
- Added a representation for address ranges relative to basic blocks (BasicBlockOffsetRanges.h). Will also serve for location lists.
- Added a representation for Lexical Blocks (LexicalBlock.h)
- Small refactorings in DebugArangesWriter:
-- Renamed to DebugRangesSectionsWriter since it also writes .debug_ranges
-- Refactored it not to depend on BinaryFunction but instead on anything that can be assined an aoffset in .debug_ranges (added an interface for that)
- Iterate over the DIE tree during initialization to find lexical blocks in .debug_info (BinaryContext.cpp)
- Added patches to .debug_abbrev and .debug_info in RewriteInstance to update lexical blocks attributes (in fact, this part is very similar to what was done to function address ranges and I just refactored/reused that code)
- Added small test case (lexical_blocks_address_ranges_debug.test)
(cherry picked from FBD3113181)
Summary:
Before this diff LLVM used to iterate over all sections to find the
one with an address we want to remap. Since we have extremely
large number of section this process is highly inefficient.
Instead we add a new interface to remap a section with a given ID
(which effectively is an index into an array of sections), and
pass the ID instead of the address.
This cuts down the processing time of hhvm binary by 10 seconds,
and brings the total processing time to a little under 2 minutes.
(cherry picked from FBD3110015)
Summary:
Populate function execution count while parsing fdata. Before
we used a quadratic algorithm to populate the execution count
(had to iterate over *all* branches for every single function).
Ignore non-symbol to non-symbol branches while parsing fdata.
These changes combined drop HHVM processing time from
4 minutes 53 seconds down to 2 minutes 9 seconds on my devserver.
Test case had to be modified since it contained irrelevant
branches from PLT to libc.
(cherry picked from FBD3106263)
Summary:
[WIP] Update DWARF info for function address ranges.
This diff currently does not work for unknown reasons,
but I'm describing here what's the current state.
According to both llvm-dwarf and readelf our output seems correct,
but GDB does not interpret it as expected. All details go below in
hope I missed something.
I couldn't actually track the whole change that introduced support for
what we need in gdb yet, but I think I can get to it
(2007-12-04: Support
lexical bocks and function bodies that occupy non-contiguous address ranges). I have reasons to believe gdb at least at some
nges).
The set of introduced changes was basically this:
- After disassembly, iterate over the DIEs in .debug_info and find the
ones that correspond to each BinaryFunction.
- Refactor DebugArangesWriter to also write addresses of functions to
.debug_ranges and track the offsets of function address ranges there
- Add some infrastructure to facilitate patching the binary in
simple ways (BinaryPatcher.h)
- In RewriteInstance, after writing .debug_ranges already with
function address ranges, for each function do:
-- Find the abbreviation corresponding to the function
-- Patch .debug_abbrev to replace DW_AT_low_pc with DW_AT_ranges and
DW_AT_high_pc with DW_AT_producer (I'll explain this hack below).
Also patch the corresponding forms to DW_FORM_sec_offset and
DW_FORM_string (null-terminated in-place string).
-- Patch debug_info with the .debug_ranges offset in place of
the first 4 bytes of DW_AT_low_pc (DW_AT_ranges only occupies 4
bytes whereas low_pc occupies 8), and write an arbitrary string
in-place in the other 12 bytes that were the 4 MSB of low_pc
and the 8 bytes of high_pc before the patch. This depends on
low_pc and high_pc being put consecutively by the compiler, but
it serves to validate the idea. I tried another way of doing it
that does not rely on this but it didn't work either and I believe
the reason for either not working is the same (and still unknown,
but unrelated to them. I might be wrong though, and if I find yet
another way of doing it I may try it). The other way was to
use a form of DW_FORM_data8 for the section offset. This is
disallowed by the specification, but I doubt gdb validates this,
as it's just easier to store it as 64-bit anyway as this is even
necessary to support 64-bit DWARF (which is not what gcc generates
by default apparently).
I still need to make changes to the diff to make it production-ready,
but first I want to figure out why it doesn't work as expected.
By looking at the output of llvm-dwarfdump or readelf, all of
.debug_ranges, .debug_abbrev and .debug_info seem to have been
correctly updated. However, gdb seems to have serious problems with
what we write.
(In fact, readelf --debug-dump=Ranges shows some funny warning messages
of the form ("Warning: There is a hole [0x100 - 0x120] in .debug_ranges"),
but I played around with this and it seems it's just because no
compile unit was using these ranges. Changing .debug_info apparently
changes these warnings, so they seem to be unrelated to the section
itself. Also looking at the hex dump of the section doesn't help,
as everything seems fine. llvm-dwarfdump doesn't say anything.
So I think .debug_ranges is fine.)
The result is that gdb not only doesn't show the function name as we
wanted, but it also stops showing line number information.
Apparently it's not reading/interpreting the address ranges at all,
and so the functions now have no associated address ranges, only the
symbol value which allows one to put a breakpoint in the function,
but not to show source code.
As this left me without more ideas of what to try to feed gdb with,
I believe the most promising next trial is to try to debug gdb itself,
unless someone spots anything I missed.
I found where the interesting part of the code lies for this
case (gdb/dwarf2read.c and some other related files, but mainly that one).
It seems in some parts gdb uses DW_AT_ranges for only getting
its lowest and highest addresses and setting that as low_pc and
high_pc (see dwarf2_get_pc_bounds in gdb's code and where it's called).
I really hope this is not actually the case for
function address ranges. I'll investigate this further. Otherwise
I don't think any changes we make will make it work as initially
intended, as we'll simply need gdb to support it and in that case it
doesn't.
(cherry picked from FBD3073641)
Summary:
We used to output .debug_line information for every instruction, but because of the way
gdb (and probably lldb as of llvm::DWARFDebugLine::LineTable::findAddress) queries the
line table it's not necessary to output information for two instructions if they follow
each other and map to the same source line. By not repeating this information we generate
a bit less .debug_line data.
(cherry picked from FBD3056402)
Summary:
The line number information generated from a null pointer
was actually valid, which caused new instructions without the line number
information set to have a valid and wrong line number reference. This diff
fixes this by making the null pointer be assigned to an invalid line number
row.
(cherry picked from FBD3048453)
Summary:
Write the .debug_aranges section after optimizations to the output binary.
Each function generates at least one range and at most two (one extra for its cold part).
The writing is done manually because LLVM's implementation is tied to the output of
.debug_info (see EmitGenDwarfInfo and EmitGenDwarfARanges in lib/MC/MCDwarf.cpp),
which we don't want to trigger right now.
(cherry picked from FBD3043108)
Summary:
At the moment we rely solely on the symbol table information to discover
function boundaries. However, similar information is contained in
.eh_frame. Verify that the information from these two sources is
consistent, and if it's not, then skip processing the functions with
conflicting information.
(cherry picked from FBD3043800)
Summary:
After we add new line number information we have to update stmt_list
offsets in .debug_info. For this I had to add a primitive relocations
support for non-allocatable sections we are copying from input file.
Also enabled functionality to process relocations in non-allocatable
sections that LLVM is generating, such as .debug_line. I thought
we already had it, but apparently it didn't work, at least not
for ELF binaries.
(cherry picked from FBD3037903)
Summary:
Skip DW_CFA_expression and DW_CFA_val_expression instructions
properly, according to DWARF spec.
If CFI range does not match function range skip that function.
(cherry picked from FBD3040502)
Summary:
Writes .debug_line section by setting the state
in MCContext that LLVM needs to produce and output the
line tables. This basically consists of setting the
current location and compile unit offset. This makes LLVM
output .debug_line in the temporary file, but not yet in
the generated ELF file.
Also computes the line table offsets for each compile unit
and saves them into BinaryContext. Added an option to
print these offsets.
(cherry picked from FBD3004554)
Summary:
The is a set of changes that allow modification of non-allocatable
sections in ELF binary. Primarily for the purpose of updating debug
info.
Extend LLVM interface to allow processing relocations in non-allocatable
sections. This allows to produce .debug* sections with resolved
relocations against generated code.
Extend BOLT rewriting framework to allow appending contents to
non-allocatable sections in the binary.
Re-worked ELF binary rewriting to support the above and to allow future
extensions (e.g. new section names).
(cherry picked from FBD3023403)
Summary:
Reads information in the DWARF .debug_line section using LLVM and
tie every MCInst to one line of a line table from the input binary. Subsequent
diffs will update this information to match the final binary layout and
output updated line tables.
(cherry picked from FBD2989813)
Summary:
Force the splitting of the function into hot/cold even when
the function fits into original slot.
This reduces BOLT optimization time by 50% without affecting
hhvm performance.
(cherry picked from FBD2973773)
Summary:
If we see an unknown CFI instruction, skip processing the function
containing it instead of aborting execution.
(cherry picked from FBD2964557)
Summary:
Added an option to reuse existing program header entry.
This option allows for bfd tools like strip and objcopy
to operate on the optimized binary without destroying it.
Also, all new sections are now properly marked in ELF.
(cherry picked from FBD2943339)
Summary:
We used to require pre-allocated space in the input binary so that
we can write extra sections in there (.eh_frame, .eh_frame_hdr,
.gcc_except_table, etc.). With this diff there's no further
need for pre-allocated storage as we create a new segment and
can use as much space as needed.
There are certain limitations on where the new segment could
be allocated, and as a result the size of the file may increase.
There's currently a limitation if the binary size is close to 4GB
we cannot allocate new segment prior to that and as a result
we require debug info to be stripped to reduce the file size.
The fix is in progress.
(cherry picked from FBD2916029)
Summary:
We use intermediate .o file for debugging purposes, but there's no
reason to generate it by default. Only do it if "-keep-tmp" is
specified.
(cherry picked from FBD2912098)
Summary:
Preserve original layout for basic blocks that have 0 execution
count. Since we don't optimize for size, it's better to rely on
the original input order.
(cherry picked from FBD2875335)
Summary:
We should never outline the first basic block.
Also add an option to accept a file with the list of
functions to optimize.
(cherry picked from FBD2868184)
Summary:
We could split functions with exceptions even without creating
a new exception handling table. This limits us to only move
basic blocks that never throw, and are not a start of a
landing pad.
(cherry picked from FBD2862937)
Summary:
Some basic blocks were created empty because they only contained
alignment nop's. Ignore such nop's before basic block gets created.
Fixed intermittent aborts related to CFI update.
(cherry picked from FBD2844465)
Summary:
* Update CFI state for larger range of functions to increase coverage.
* Issue more warnings indicating reasons for skipping functions.
* Print top called functions in the binary.
(cherry picked from FBD2839734)
Summary:
Modified processing of "-reorder-blocks=" option and added an option
to reverse original basic blocks order for testing purposes.
(cherry picked from FBD2829862)
Summary:
Fixes some issues discovered after hhvm switched to gcc 4.9.
Add support for DW_CFA_GNU_args_size instruction.
Allow CFI instruction after the last instruction in a function.
Reverse conditions of assert for DW_CFA_set_loc.
(cherry picked from FBD28110096)
Summary:
Binary code could be weird. It could include calls to address 0 and
reference data at 0 (e.g. with lea on x86). LLVM JIT fatals
while resolving relocations against symbols at address 0x0. For now
we will stop emitting such code, i.e. we'll skip functions.
(cherry picked from FBD28109837)
Summary:
In a test binary, we found 8 cases where code in a function A would jump to the
middle of another function B. In this case, we cannot reorder function B because
this would change instruction offsets and break the program. This is pretty rare
but can happen in code written in assembly.
(cherry picked from FBD2719850)
Summary:
We found out that the insertion of extra nops to preserve alignment of
some loop bodies do not pay off the increased function size, since this extra
size may inhibit us from rewriting a reordered version of this function.
(cherry picked from FBD2718466)
Summary:
Our CFI parser in the LLVM library was giving up on parsing all CFI
instructions when finding a single instruction with expression operands. Yet,
all gcc-4.9 binaries seem to have at least one CFI instruction with expression
operands (DW_CFA_def_cfa_expression). This patch fixes this and makes DebugInfo
continue to parse other instructions, even though it does not completely parse
DWARF expressions yet. However, this seems to be enough to allow llvm-flo to
process gcc-4.9 binaries because the FDEs with DWARF expressions are linked to
the PLT region, and not to functions that we process.
If we ever try to read a function whose CFI depends on DWARF expression, which
is unlikely, llvm-flo will assert.
(cherry picked from FBD2693088)
Summary:
This patch builds upon the previous patch to create a two-pass process
to function splitting. We first perform the full rewriting pipeline to discover
which functions need splitting. Afterwards, we restart the pipeline with those
functions annotated to be split.
(cherry picked from FBD2691709)
Summary:
Previously, llvm-flo.cpp contained a long function doing lots of
different tasks. This patch refactors this logic into a separate class with
different member functions, exposing the relationship between each step of
the rewritting process and making it easier to coordinate/change it.
(cherry picked from FBD2691674)
Summary:
After basic block reordering, it may be possible that the reordered
function is now larger than the original because of the following reasons:
- jump offsets may change, forcing some jump instructions to use 4-byte
immediate operand instead of the 1-byte, shorter version.
- fall-throughs change, forcing us to emit an extra jump instruction to jump
to the original fall-through at the end of a basic block.
Since we currently do not change function addresses, we need to rewrite the
function back in the binary in the original location. If it doesn't fit, we were
dropping the function.
This patch adds a flag -split-functions that tells llvm-flo to split hot
functions into hot and cold separate regions. The hot region is written back
in the original function location, while the cold region is written in a
separate, far-away region reserved to flo via a linker script.
This patch also adds the logic to create and extra FDE to supply unwinding
information to the cold part of the function. Owing to this, we now need to
rewrite .eh_frame_hdr to another location and patch the EH_FRAME ELF segment
to point to this new .eh_frame_hdr.
(cherry picked from FBD2677996)
Summary:
This is an attempt at determining the hotness of functions we are
rewriting and help detect if we are discarding hot functions. This patch
introduces logic to estimate the number of instructions executed in each
function by using the profile data for branches. It sums the products of
BB frequency and size. Since we can only do this for functions we have
successfully disassembled, created the CFG and annotated with profiling
data, all complex functions that were not disassembled are left out from
this analysis.
(cherry picked from FBD2654985)
Summary:
Previously, we were marking functions with indirect calls as too
complex to be disassembled, but this was unnecessarily conservative. This patch
removes this restriction.
(cherry picked from FBD2669627)
Summary:
Teach llvm-flo to drop on function with LSDA information until we know
how to update them after block reordering.
(cherry picked from FBD2640806)
Summary:
This patch adds logic to detect when the binary has extra space
reserved for us via the __flo_storage symbol. If this symbol is present,
it means we have extra space in the binary to write extraneous information.
When we write a new .eh_frame, we cannot discard the old .eh_frame because
it may still contain relevant information for functions we do not reorder.
Thus, we write the new .eh_frame into __flo_storage and patch the current
.eh_frame_hdr to point to the new .eh_frame only for the functions we touched,
generating a binary that works with a bi-.eh_frame model.
(cherry picked from FBD2639326)
Summary:
This patch is an intermediary step towards updating the CFI in the
optimized binary. It adds the logic necessary to output our CFI annotations to
a new .eh_frame in the temporary object file we create to hold rewritten
functions. The next step will be to fully integrate this new .eh_frame into the
optimized binary.
(cherry picked from FBD2633728)
Summary:
This patch introduces logic to check how the CFI instructions define a
table to help during stack unwinding at exception run time and attempts to fix
any problem in this table that may have been introduced by reordering the basic
blocks. If it fails to fix this problem, the function is marked as not simple
and not eligible for rewriting.
(cherry picked from FBD2633696)
Summary:
Regenerate exception handling information after optimizations.
Use '-print-eh-ranges' to see CFG with updated ranges.
(cherry picked from FBD2660982)
Summary:
There were two issues: we were trying to process non-simple functions,
i.e. function that we don't fully understand, and then we failed to stop
iterating if EH closing label was after the last instruction in a
function.
(cherry picked from FBD2664460)
Summary:
Read .gcc_except_table and add information to CFG. Calls have extra operands
indicating there's a possible handler for exceptions and an action. Landing
pad information is recorded in BinaryFunction.
Also convert JMP instructions that are calls into tail calls pseudo
instructions so that they don't miss call instruction analysis.
(cherry picked from FBD2652775)
Summary: Reverting this commit until we better investigate why
it is necessary to change local symbol names with a prefix.
(cherry picked from FBD28109521)
Summary: After discussion with Maksim, we decided to drop the lines
that add the PG prefix if the symbol is already local, since they
wouldn't be impacted by the way LLVM handles these symbols.
(cherry picked from FBD28109400)
Summary:
This bug would cause llvm-flo to fail to disambiguate two local symbols
with the same file name, causing two different addresses to compete in the
symbol table for the resolution of a given name, causing unpredicted behavior in
the linker.
(cherry picked from FBD2646626)
Summary:
In order to represent CFI information in our BinaryFunction class, this
patch adds a map of Offsets to CFI instructions. In this way, we make it easy to
check exactly where DWARF CFI information is annotated in the disassembled
function.
(cherry picked from FBD2619216)
Summary:
We need to parse the whole contents of .gcc_except_table even if we are
not printing exceptions. Otherwise we are missing type index table and
miscalculate the size of the current table.
(cherry picked from FBD2632965)
Summary: In order to reorder binaries with C++ exceptions, we first need to
read DWARF CFI (call frame info) from binaries in a table in the .eh_frame
ELF section. This table contains unwinding information we need to be aware of
when reordering basic blocks, so as to avoid corrupting it. This patch also
cleans up some code from Exceptions.cpp due to a refactoring where we moved
some functions to the LLVM's libSupport.
(cherry picked from FBD2614464)
Summary:
Print actions for exception ranges from .gcc_except_table.
Types are printed as names if the name is available from symbol table.
(cherry picked from FBD2612631)
Summary:
Previously, we inferred all non-taken branch frequencies with the
information we had for taken branches. This patch teaches perf2flo and llvm-flo
how to read and incorporate non-taken branch frequencies directly from the
traces available in LBR data and by disassembling the binary. It still leaves
the inference engine untouched in case we need it to fill out other
fall-throughs.
(cherry picked from FBD2589212)
Summary:
Pettis' paper on block layout (PLDI'90) suggests we should order
clusters (or chains, using the paper terminology) using a specific criterion.
This patch implements two distinct ideas for cluster layout that can be
activated using different command-line flags. The first one reflects Pettis'
ideas on minimizing branch mispredictions and the second one is targeted at
reducing I-cache misses, described in the Ispike paper (CGO'04).
(cherry picked from FBD2588693)
Summary:
Fixes a bug which caused the block reordering heuristic to put in the
same cluster hot basic blocks and cold basic blocks, increasing I-cache misses.
(cherry picked from FBD2588203)
Summary:
When the ignore-nops patch landed, it exposed a bug in fixBranches()
where it ignored empty BBs. However, we cannot ignore empty BBs when it is
reordered and its fall-through changes. We must update it with a jump to the
original fall-through. This patch fixes this.
(cherry picked from FBD2568244)
Summary:
It is important to remove dead blocks to free up space in functions
and allow us to reorder blocks or align branch targets with more
freedom. This patch implements a simple algorithm to delete all basic
blocks that are not reachable from the entry point. Note that C++
exceptions may create "unreachable" blocks, so this option must be
used with care.
(cherry picked from FBD2562637)
Summary:
SPEC CPU2006 perlbench triggered a bug in our heuristic block
reordering algorithm where a hot edge that targets the entry point (as in a
recursive tail call) would make us try to allocate the call site before the
function entry point. Since we don't update function addresses yet, moving the
entry point will corrupt the program. This patch fixes this.
(cherry picked from FBD2562528)
Summary:
If we have two consecutive JMP instructions and no branches to the
second one, the second one is dead code, but llvm-flo does not handle these
cases properly and put two JMPs in the same BB. This patch fixes this, putting
the extraneous JMP in a separate block, making it easy for us to detect it is
dead code and remove it later in a separate step.
(cherry picked from FBD2562465)
Summary:
Nop instructions are primarily used for alignment purposes on the input.
We remove all nops when we build CFG and derive alignment of basic blocks
based on existing alignment and a presence of nops before it. This
will not always work as some basic blocks will be naturally aligned
without necessity for nops. However, it's better than random alignment.
We would also add heuristics for BB alignment based on execution profile.
(cherry picked from FBD2561740)
Summary:
Adds logic in BinaryFunction to be able to fix branches (invert
its condition, delete or add a branch), making the new function work with the
new layout proposed by the layout pass. All the architecture-specific content
was designed to live in the LLVM Target library, in the MCInstrAnalysis pass.
For now, we only introduce such logic to the X86 backend.
(cherry picked from FBD2551479)
Summary:
Tests with SPEC CPU2006 400.perlbench exposed a bug in the block reordering
heuristic that happened when two blocks are both successor and predecessor of
each other. This patch fixes this.
(cherry picked from FBD2555835)
Summary:
SPEC CPU2006 perlbench exposed a bug in BinaryFunction::optimizeLayout()
where it would try to optimize the layout even though the function had zero
basic blocks. This patch simply checks if the function has zero basic blocks and
bails out.
(cherry picked from FBD2556831)
Summary:
In a recent commit, we changed local symbols to be specially tagged
with the number 2 (local sym) instead of 1 (sym). This patch modifies the reader
to don't choke when seeing a 2 in the symbol id field.
(cherry picked from FBD2552776)
Summary:
This patch implements a dynamic programming approach to solve reorder
basic blocks with profiling information in an optimal way. Since this is
analogous to TSP, it is NP-hard and the algorithm is exponential in time and
memory consumption. Therefore, we only use the optimal algorithm to decide the
layout of small functions (with less than 11 basic blocks).
(cherry picked from FBD2544124)
Summary:
This patch introduces a first approach to reorder basic blocks based on
profiling data that gives us the execution frequency for each edge. Our strategy
is to layout basic blocks in a order that maximizes the weight (hotness) of
branches that will be deleted. We can delete branches when src comes right
before dst in the new layout order. This can be reduced to the TSP problem. This
patch uses a greedy heuristic to solve the problem: we start with a graph with
no edges and progressively add edges by choosing the hottest edges first,
building a layout order that attempts to put BBs with hot edges together.
(cherry picked from FBD2544076)
Summary:
The LBR only has information about taken branches and does not record
information when a branch is not taken. In our CFG, we call these edges
"fall-through" edges. This patch teaches llvm-flo how to infer fall-through
edge frequencies.
(cherry picked from FBD2536633)
Summary:
Changes DataReader to organize branch perf data per function name and
sets up logistics to bring this data to BinaryFunction::buildCFG(). To do this,
we expand BinaryContext with a const reference to DataReader. This patch also
adds the "-dump-functions" flag to force llvm-flo to dump the current state of
BinaryFunctions once they are disassembled and their CFG built, allowing us to
test whether the builder is sane with LLVM LIT tests.
(cherry picked from FBD2534675)
Summary:
This patch introduces DataReader, a module responsible for
parsing llvm flo data files into in-memory data structures.
(cherry picked from FBD2515754)
laith sakka