Following discussions in #110443, and the following earlier discussions
in https://lists.llvm.org/pipermail/llvm-dev/2017-October/117907.html,
https://reviews.llvm.org/D38482, https://reviews.llvm.org/D38489, this
PR attempts to overhaul the `TargetMachine` and `LLVMTargetMachine`
interface classes. More specifically:
1. Makes `TargetMachine` the only class implemented under
`TargetMachine.h` in the `Target` library.
2. `TargetMachine` contains target-specific interface functions that
relate to IR/CodeGen/MC constructs, whereas before (at least on paper)
it was supposed to have only IR/MC constructs. Any Target that doesn't
want to use the independent code generator simply does not implement
them, and returns either `false` or `nullptr`.
3. Renames `LLVMTargetMachine` to `CodeGenCommonTMImpl`. This renaming
aims to make the purpose of `LLVMTargetMachine` clearer. Its interface
was moved under the CodeGen library, to further emphasis its usage in
Targets that use CodeGen directly.
4. Makes `TargetMachine` the only interface used across LLVM and its
projects. With these changes, `CodeGenCommonTMImpl` is simply a set of
shared function implementations of `TargetMachine`, and CodeGen users
don't need to static cast to `LLVMTargetMachine` every time they need a
CodeGen-specific feature of the `TargetMachine`.
5. More importantly, does not change any requirements regarding library
linking.
cc @arsenm @aeubanks
Note that PointerUnion::{is,get,dyn_cast} have been soft deprecated in
PointerUnion.h:
// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa<T>, cast<T> and the llvm::dyn_cast<T>
The early simplication pipeline is used in non-LTO and (Thin/Full)LTO
pre-link
stage. There are some passes that we want them in non-LTO mode, but not
at LTO
pre-link stage. The control is missing currently. This PR adds the
support. To
demonstrate the use, we only enable the internalization pass in non-LTO
mode for
AMDGPU because having it run in pre-link stage causes some issues.
Some targets (e.g. PPC and Hexagon) already did this. I think it's best
to do this consistently so that frontend authors don't run into
inconsistent results when they emit `naked` functions. For example, in
Zig, we had to change our emit code to also set `frame-pointer=none` to
get reliable results across targets.
Note: I don't have commit access.
Rename the function to reflect its correct behavior and to be consistent
with `Module::getOrInsertFunction`. This is also in preparation of
adding a new `Intrinsic::getDeclaration` that will have behavior similar
to `Module::getFunction` (i.e, just lookup, no creation).
The motivation example likes below
```
$ cat t1.c
struct S {
int var[3];
};
int foo1 (struct S *a, struct S *b)
{
return a - b;
}
```
For cpu v1/v2/v3, the compilation will fail with the following error:
```
$ clang --target=bpf -O2 -c t1.c -mcpu=v3
t1.c:4:5: error: unsupported signed division, please convert to unsigned div/mod.
4 | int foo1 (struct S *a, struct S *b)
| ^
1 error generated.
```
The reason is that sdiv/smod is only supported at -mcpu=v4. At cpu
v1/v2/v3, only udiv/umod is supported.
But the above example (for func foo1()) is reasonable common and user
has to workaround the compilation failure by using udiv with
conditionals.
For x86, for the above t1.c, compile and dump the asm code like below:
```
$ clang -O2 -c t1.c && llvm-objdump -d t1.o
0000000000000000 <foo1>:
0: 48 29 f7 subq %rsi, %rdi
3: 48 c1 ef 02 shrq $0x2, %rdi
7: 69 c7 ab aa aa aa imull $0xaaaaaaab, %edi, %eax # imm = 0xAAAAAAAB
d: c3 retq
```
Basically sdiv can be replaced with sub, shr and imul. Latest gcc-bpf is
also able to generate code similar to x86 with -mcpu=v1. See
https://godbolt.org/z/feP9ETbjj
Generally multiplication is cheaper than divide. LLVM SelectionDAG
already supports to generate alternative codes (mul etc.) for div/mod
operations if the divisor is constant and if the cost of division is not
cheap. For BPF backend, let us use multiplication instead of divide
whenever possible. The following is the list of div/mod operations which
can be converted to mul.
- 32bit signed/unsigned div/mod with constant divisor
- 64bit signed/unsigned div with constant divisor and with 'exact' flag
in IR where 'exact' means if replacing div with mod, it is guaranteed
that modulo result will be 0. The above foo1() case belongs here.
bpf_fastcall induced spill/fill pairs should be generated for
sub-register as well as for sub-registers. At the moment this is not the
case, e.g.:
$ cat t.c
extern int foo(void) __attribute__((bpf_fastcall));
int bar(int a) {
foo();
return a;
}
$ clang --target=bpf -mcpu=v3 -O2 -S t.c -o -
...
call foo
w0 = w1
exit
Modify BPFMIPeephole.cpp:collectBPFFastCalls() to check sub-registers
liveness and thus produce correct code for example above:
*(u64 *)(r10 - 8) = r1
call foo
r1 = *(u64 *)(r10 - 8)
w0 = w1
exit
This follows in the spirit of 7d82c99403f615f6236334e698720bf979959704,
and extends the costing API for compares and selects to provide
information about the operands passed in an analogous manner. This
allows us to model the cost of materializing the vector constant, as
some select-of-constants are significantly more expensive than others
when you account for the cost of materializing the constants involved.
This is a stepping stone towards fixing
https://github.com/llvm/llvm-project/issues/109466. A separate SLP patch
will be required to utilize the new API.
It is almost always simpler to use {} instead of std::nullopt to
initialize an empty ArrayRef. This patch changes all occurrences I could
find in LLVM itself. In future the ArrayRef(std::nullopt_t) constructor
could be deprecated or removed.
All ATOMIC_NOFETCH insns have "$dst = $val" constraints. So let us
enforce "$dst = $val" having the same register type in ATOMIC_NOFETCH as
well.
Currently, things work since ATOMIC_NOFETCH does not have source code
pattern matching. I am experimenting to introduce memory ordering to
BPFInstrInfo.td file and pattern matching will be needed. Eventually,
for atomic_fetch_*() insns locked insns could be generated if memory
ordering is memory_order_relaxed.
[1] https://lore.kernel.org/bpf/7b941f53-2a05-48ec-9032-8f106face3a3@linux.dev/
Before llvm20, (void)__sync_fetch_and_add(...) always generates locked
xadd insns. In linux kernel upstream discussion [1], it is found that
for arm64 architecture, the original semantics of
(void)__sync_fetch_and_add(...), i.e., __atomic_fetch_add(...), is
preferred in order for jit to emit proper native barrier insns.
In llvm commits [2] and [3], (void)__sync_fetch_and_add(...) will
generate the following insns:
- for cpu v1/v2: locked xadd insns to keep backward compatibility
- for cpu v3/v4: __atomic_fetch_add() insns
To ensure proper barrier semantics for (void)__sync_fetch_and_add(...),
cpu v3/v4 is recommended.
This patch enables cpu=v3 as the default cpu version. For users wanting
to use cpu v1, -mcpu=v1 needs to be explicitly added to clang/llc
command line.
[1]
https://lore.kernel.org/bpf/ZqqiQQWRnz7H93Hc@google.com/T/#mb68d67bc8f39e35a0c3db52468b9de59b79f021f
[2] https://github.com/llvm/llvm-project/pull/101428
[3] https://github.com/llvm/llvm-project/pull/106494
This patch contains two pars:
- first to revert the patch https://github.com/llvm/llvm-project/pull/101428.
- second to remove `atomic_fetch_and_*()` to `atomic_<op>()`
conversion (when return value is not used), but preserve
`__sync_fetch_and_add()` to locked insn with cpu v1/v2.
The renamable flag is useful during MachineCopyPropagation but renamable
flag will be dropped after lowerCopy in some case.
This patch introduces extra arguments to pass the renamable flag to
copyPhysReg.
This commit introduces attribute bpf_fastcall to declare BPF functions
that do not clobber some of the caller saved registers (R0-R5).
The idea is to generate the code complying with generic BPF ABI,
but allow compatible Linux Kernel to remove unnecessary spills and
fills of non-scratched registers (given some compiler assistance).
For such functions do register allocation as-if caller saved registers
are not clobbered, but later wrap the calls with spill and fill
patterns that are simple to recognize in kernel.
For example for the following C code:
#define __bpf_fastcall __attribute__((bpf_fastcall))
void bar(void) __bpf_fastcall;
void buz(long i, long j, long k);
void foo(long i, long j, long k) {
bar();
buz(i, j, k);
}
First allocate registers as if:
foo:
call bar # note: no spills for i,j,k (r1,r2,r3)
call buz
exit
And later insert spills fills on the peephole phase:
foo:
*(u64 *)(r10 - 8) = r1; # Such call pattern is
*(u64 *)(r10 - 16) = r2; # correct when used with
*(u64 *)(r10 - 24) = r3; # old kernels.
call bar
r3 = *(u64 *)(r10 - 24); # But also allows new
r2 = *(u64 *)(r10 - 16); # kernels to recognize the
r1 = *(u64 *)(r10 - 8); # pattern and remove spills/fills.
call buz
exit
The offsets for generated spills/fills are picked as minimal stack
offsets for the function. Allocated stack slots are not used for any
other purposes, in order to simplify in-kernel analysis.
This commit introduces attribute bpf_fastcall to declare BPF functions
that do not clobber some of the caller saved registers (R0-R5).
The idea is to generate the code complying with generic BPF ABI, but
allow compatible Linux Kernel to remove unnecessary spills and fills of
non-scratched registers (given some compiler assistance).
For such functions do register allocation as-if caller saved registers
are not clobbered, but later wrap the calls with spill and fill patterns
that are simple to recognize in kernel.
For example for the following C code:
#define __bpf_fastcall __attribute__((bpf_fastcall))
void bar(void) __bpf_fastcall;
void buz(long i, long j, long k);
void foo(long i, long j, long k) {
bar();
buz(i, j, k);
}
First allocate registers as if:
foo:
call bar # note: no spills for i,j,k (r1,r2,r3)
call buz
exit
And later insert spills fills on the peephole phase:
foo:
*(u64 *)(r10 - 8) = r1; # Such call pattern is
*(u64 *)(r10 - 16) = r2; # correct when used with
*(u64 *)(r10 - 24) = r3; # old kernels.
call bar
r3 = *(u64 *)(r10 - 24); # But also allows new
r2 = *(u64 *)(r10 - 16); # kernels to recognize the
r1 = *(u64 *)(r10 - 8); # pattern and remove spills/fills.
call buz
exit
The offsets for generated spills/fills are picked as minimal stack
offsets for the function. Allocated stack slots are not used for any
other purposes, in order to simplify in-kernel analysis.
Corresponding functionality had been merged in Linux Kernel as
[this](https://lore.kernel.org/bpf/172179364482.1919.9590705031832457529.git-patchwork-notify@kernel.org/)
patch set (the patch assumed that `no_caller_saved_regsiters` attribute
would be used by LLVM, naming does not matter for the Kernel).
Peilen Ye reported an issue ([1]) where for __sync_fetch_and_add(...)
without return value like
__sync_fetch_and_add(&foo, 1);
llvm BPF backend generates locked insn e.g.
lock *(u32 *)(r1 + 0) += r2
If __sync_fetch_and_add(...) returns a value like
res = __sync_fetch_and_add(&foo, 1);
llvm BPF backend generates like
r2 = atomic_fetch_add((u32 *)(r1 + 0), r2)
The above generation of 'lock *(u32 *)(r1 + 0) += r2' caused a problem
in jit since proper barrier is not inserted.
The above discrepancy is due to commit [2] where it tries to maintain
backward compatability since before commit [2],
__sync_fetch_and_add(...) generates lock insn in BPF backend.
Based on discussion in [1], now it is time to fix the above discrepancy
so we can have proper barrier support in jit. This patch made sure that
__sync_fetch_and_add(...) always generates atomic_fetch_add(...) insns.
Now 'lock *(u32 *)(r1 + 0) += r2' can only be generated by inline asm. I
also removed the whole BPFMIChecking.cpp file whose original purpose is
to detect and issue errors if XADD{W,D,W32} may return a value used
subsequently. Since insns XADD{W,D,W32} are all inline asm only now,
such error detection is not needed.
[1]
https://lore.kernel.org/bpf/ZqqiQQWRnz7H93Hc@google.com/T/#mb68d67bc8f39e35a0c3db52468b9de59b79f021f
[2]
286daafd65
Co-authored-by: Yonghong Song <yonghong.song@linux.dev>
When BPF object files are linked with bpftool, every symbol must be
accompanied by BTF info. Ensure that extern functions referenced by
global variable initializers are included in BTF.
The primary motivation is "static" initialization of PROG maps:
```c
extern int elsewhere(struct xdp_md *);
struct {
__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
__uint(max_entries, 1);
__type(key, int);
__type(value, int);
__array(values, int (struct xdp_md *));
} prog_map SEC(".maps") = { .values = { elsewhere } };
```
BPF backend needs debug info to produce BTF. Debug info is not
normally generated for external variables and functions. Previously, it
was solved differently for variables (collecting variable declarations
in ExternalDeclarations vector) and functions (logic invoked during
codegen in CGExpr.cpp).
This patch generalises ExternalDefclarations to include both function
and variable declarations. This change ensures that function references
are not missed no matter the context. Previously external functions
referenced in constant expressions lacked debug info.
Timers are an out-of-line function call and a global variable access,
here twice per emitted instruction. At this granularity, not only the
time results become skewed, but the timers also add a performance
overhead when profiling is disabled. Also outside of the innermost loop,
timers add a measurable overhead. As this is quite expensive for a
mostly unused profiling facility, remove the timers.
Fixes#39650.
Currently, an AsmPrinterHandler has several methods that allow to
dynamically hook in unwind or debug info emission, e.g. at begin/end of
every function or instruction. The class hierarchy and the actually
overridden functions are as follows:
(SymSz=setSymbolSize, mFE=markFunctionEnd, BBS=BasicBlockSection,
FL=Funclet; b=beginX, e=endX)
SymSz Mod Fn mFE BBS FL Inst
AsmPrinterHandler - - - - - - -
` PseudoProbeHandler - - - - - - -
` WinCFGuard - e e - - - -
` EHStreamer - - - - - - -
` DwarfCFIException - e be - be - -
` ARMException - - be e - - -
` AIXException - - e - - - -
` WinException - e be e - be -
` WasmException - e be - - - -
` DebugHandlerBase - b be - be - be
` BTFDebug - e - - - - b
` CodeViewDebug - be - - - - b
` DWARFDebug yes be - - - - b
Doing virtual function calls per instruction is costly and useless when
the called function does nothing.
This commit performs the following clean-up/improvements:
- PseudoProbeHandler is no longer an AsmPrinterHandler -- it used
nothing of its functionality to hook in at the possible points. This
avoids virtual function calls when a pseudo probe printer is present.
- DebugHandlerBase is no longer an AsmPrinterHandler, but a separate
base class. DebugHandlerBase is the only remaining "hook" for begin/end
instruction and setSymbolSize (only used by DWARFDebug). begin/end for
function and basic block sections are never overriden and therefore are
no longer virtual. (Originally I intended there to be only one debug
handler, but BPF as the only target supports two at the same time: DWARF
and BTF.)
- AsmPrinterHandler no longer has begin/end instruction and
setSymbolSize hooks -- these were only used by DebugHandlerBase. This
avoid iterating over handlers in every instruction.
AsmPrinterHandler Mod Fn mFE BBS FL
` WinCFGuard e e - - -
` EHStreamer - - - - -
` DwarfCFIException e be - be -
` ARMException - be e - -
` AIXException - e - - -
` WinException e be e - be
` WasmException e be - - -
SymSz Mod Fn BBS Inst
DebugHandlerBase - b be be be
` BTFDebug - e b
` CodeViewDebug - be b
` DWARFDebug yes be b
PseudoProbeHandler (no shared methods)
To continue allowing external users (e.g., Julia) to hook in at every
instruction, a new method addDebugHandler is exposed.
This results in a performance improvement, especially in the -O0 -g0
case with unwind information (e.g., JIT baseline).
This is a helper to avoid writing `getModule()->getDataLayout()`. I
regularly try to use this method only to remember it doesn't exist...
`getModule()->getDataLayout()` is also a common (the most common?)
reason why code has to include the Module.h header.
Alastair Robertson reported a huge compilation time increase without -g
for bpf target when comparing to x86 ([1]). In my setup, with '-O0', for
x86, a large basic block compilation takes 0.19s while bpf target takes
2.46s. The top function which contributes to the compile time is
eliminateFrameIndex().
Such long compilation time without -g is caused by commit
05de2e481811 ("[bpf] error when BPF stack size exceeds 512 bytes")
The compiler tries to get some debug loc by iterating all insns in the
basic block which will be used when compiler warns larger-than-512 stack
size. Even without -g, such iterating also happens which cause
unnecessary compile time increase.
To fix the issue, let us move the related code when the compiler is
about to warn stack limit violation. This fixed the compile time
regression, and on my system, the compile time is reduced from 2.46s to
0.35s.
[1] https://github.com/bpftrace/bpftrace/issues/3257
Co-authored-by: Yonghong Song <yonghong.song@linux.dev>
On MSVC the `this` uses inside `decltype` require a lambda capture. On
clang they result in an unused capture warning instead. Add the capture
and suppress the warning with `(void)this`.
-----
Initializing this map is somewhat expensive (especially for O0), so we
currently only do it if certain flags are used. I would like to make use
of it for crash dumps (#96078), where we don't know in advance whether
it will be needed or not.
This patch changes the initialization to a lazy approach, where a
callback is registered that does the actual initialization. The
callbacks will be run the first time the pass name is requested.
This way there is no compile-time impact if the mapping is not used.
My attempt to fix the Windows build made things worse,
revert entirely for now.
This reverts commit e7137f2fed5cfee822ae3c4c6d39188adb59a16c.
This reverts commit 6eaf204dbb0a6a81cddfd02f625c130f7bb1aae5.
This reverts commit 957dc4366dd2ce9d5d2991c3ad76bbf438e9954e.
Initializing this map is somewhat expensive (especially for O0), so we
currently only do it if certain flags are used. I would like to make use
of it for crash dumps (#96078), where we don't know in advance whether
it will be needed or not.
This patch changes the initialization to a lazy approach, where a
callback is registered that does the actual initialization. The
callbacks will be run the first time the pass name is requested.
This way there is no compile-time impact if the mapping is not used.
- Fix build with `EXPENSIVE_CHECKS`
- Remove unused `PassName::ID` to resolve warning
- Mark `~SelectionDAGISel` virtual so AArch64 backend can work properly
This reverts commit de37c06f01772e02465ccc9f538894c76d89a7a1 to
de37c06f01772e02465ccc9f538894c76d89a7a1
It still breaks EXPENSIVE_CHECKS build. Sorry.
Port selection dag isel to new pass manager.
Only `AMDGPU` and `X86` support new pass version. `-verify-machineinstrs` in new pass manager belongs to verify instrumentation, it is enabled by default.
Previously `report_fatal_error` is used for reporting something goes
wrong in the backend, but this is confusing because `report_fatal_error`
basically means there are something unexpected & crashed in the backend.
So, turn this "crash" into an elegant error reporting. After this patch,
clang can diagnose it:
bpf-crash.c:4:30: error: Invalid usage of the XADD return value
4 | u32 next_event_id() { return __sync_fetch_and_add(&GLOBAL_EVENT_ID,
1); }
| ^
1 error generated.
I'm planning to remove StringRef::equals in favor of
StringRef::operator==.
- StringRef::operator==/!= outnumber StringRef::equals by a factor of
38 under llvm/ in terms of their usage.
- The elimination of StringRef::equals brings StringRef closer to
std::string_view, which has operator== but not equals.
- S == "foo" is more readable than S.equals("foo"), especially for
!Long.Expression.equals("str") vs Long.Expression != "str".
Andrii found an issue where the BTF line info may have empty source
which seems wrong. The program is a Meta internal bpf program. I can
reproduce with latest upstream compiler as well. Let the bpf program
built without this patch and then with the following veristat check
where veristat is a bpf verifier tool to do kernel verification for bpf
programs:
$ veristat -vl2 yhs.bpf.o --log-size=150000000 >& log
$ rg '^;' log | sort | uniq -c | sort -nr | head -n10
4206 ; } else if (action->dry_run) { @ src_mitigations.h:57
3907 ; if (now < start_allow_time) { @ ban.h:17
3674 ; @ src_mitigations.h:0
3223 ; if (action->vip_id != ALL_VIPS_ID && action->vip_id != vip_id) {
@ src_mitigations.h:85
1737 ; pkt_info->is_dry_run_drop = action->dry_run; @
src_mitigations.h:26
1737 ; if (mitigation == ALLOW) { @ src_mitigations.h:28
1737 ; enum match_action mitigation = action->action; @
src_mitigations.h:25
1727 ; void* res = bpf_map_lookup_elem(bpf_map, key); @
filter_helpers.h:498
1691 ; bpf_map_lookup_elem(&rate_limit_config_map, rule_id); @
rate_limit.h:76
1688 ; if (throttle_cfg) { @ rate_limit.h:85
You can see
3674 ; @ src_mitigations.h:0
where we do not have proper line information and line number.
In LLVM Machine IR, some instructions may carry DebugLoc information
to specify where the corresponding source is for this instruction.
The information includes file_name, line_num and col_num.
Each instruction may also attribute to a function in debuginfo.
So there are two ways to find file_name for a particular insn:
(1) find the corresponding function in debuginfo
(MI->getMF()->getFunction().getSubprogram()) and then
find the file_name from DISubprogram.
(2) find the corresponding file_name from DebugLoc.
The option (1) is used in current implementation. This mostly works.
But if one instruction is somehow generated from multiple functions,
the compiler has to pick just one. This may cause a mismatch between
file_name and line_num/col_num.
Besides potential incorrect mismatch of file_name vs. line_num/col_num,
There is another issue where some DebugLoc has line number 0. For
example,
I dumped the dwarf line table for the above bpf program:
Address Line Column File ISA Discriminator OpIndex Flags
------------------ ------ ------ ------ --- ------------- -------
-------------
0x0000000000000000 96 0 17 0 0 0 is_stmt
0x0000000000000010 100 12 17 0 0 0 is_stmt prologue_end
0x0000000000000020 0 12 17 0 0 0
0x0000000000000058 37 7 17 0 0 0 is_stmt
0x0000000000000060 0 0 17 0 0 0
0x0000000000000088 37 7 17 0 0 0
0x0000000000000090 42 75 17 0 0 0 is_stmt
0x00000000000000a8 42 52 17 0 0 0
0x00000000000000c0 120 9 17 0 0 0 is_stmt
0x00000000000000c8 0 9 17 0 0 0
0x00000000000000d0 106 21 17 0 0 0 is_stmt
0x00000000000000d8 106 3 17 0 0 0
0x00000000000000e0 110 25 17 0 0 0 is_stmt
0x00000000000000f8 110 36 17 0 0 0
0x0000000000000100 0 36 17 0 0 0
...
These DebugLoc with line number 0 needs to be skipped since we cannot
map them to the correct source code. Note that selftest
offset-reloc-basic.ll
has this issue as well which is adjusted by this patch.
With the above two fixes, empty lines for source annotation are removed.
$ veristat -vl2 yhs.bpf.o --log-size=150000000 >& log
$ rg '^;' log.latest | sort | uniq -c | sort -nr | head -n10
4206 ; } else if (action->dry_run) { @ src_mitigations.h:57
3907 ; if (now < start_allow_time) { @ ban.h:17
3223 ; if (action->vip_id != ALL_VIPS_ID && action->vip_id != vip_id) {
@ src_mitigations.h:85
1737 ; pkt_info->is_dry_run_drop = action->dry_run; @
src_mitigations.h:26
1737 ; if (mitigation == ALLOW) { @ src_mitigations.h:28
1737 ; enum match_action mitigation = action->action; @
src_mitigations.h:25
1727 ; void* res = bpf_map_lookup_elem(bpf_map, key); @
filter_helpers.h:498
1691 ; bpf_map_lookup_elem(&rate_limit_config_map, rule_id); @
rate_limit.h:76
1688 ; if (throttle_cfg) { @ rate_limit.h:85
1670 ; if (rl_cfg) { @ rate_limit.h:77
You can see that we do not have empty line any more.
3223 ; if (action->vip_id != ALL_VIPS_ID && action->vip_id != vip_id) {
@ src_mitigations.h:85
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
Related to clean-up opportunities discussed at #90013.
After these cleanups, the `RelaxAll` parameter from
`createMCObjectStreamer` can be removed as well. As
`createMCObjectStreamer` is a more user-facing API and used by two files
in mlir/, we postpone the cleanup to the future.
