As part of the "RemoveDIs" project, BasicBlock::iterator now carries a debug-info bit that's needed when getFirstNonPHI and similar feed into instruction insertion positions. Call-sites where that's necessary were updated a year ago; but to ensure some type safety however, we'd like to have all calls to getFirstNonPHI use the iterator-returning version. This patch changes a bunch of call-sites calling getFirstNonPHI to use getFirstNonPHIIt, which returns an iterator. All these call sites are where it's obviously safe to fetch the iterator then dereference it. A follow-up patch will contain less-obviously-safe changes. We'll eventually deprecate and remove the instruction-pointer getFirstNonPHI, but not before adding concise documentation of what considerations are needed (very few). --------- Co-authored-by: Stephen Tozer <Melamoto@gmail.com>
As part of the "RemoveDIs" project, BasicBlock::iterator now carries a debug-info bit that's needed when getFirstNonPHI and similar feed into instruction insertion positions. Call-sites where that's necessary were updated a year ago; but to ensure some type safety however, we'd like to have all calls to moveBefore use iterators. This patch adds a (guaranteed dereferenceable) iterator-taking moveBefore, and changes a bunch of call-sites where it's obviously safe to change to use it by just calling getIterator() on an instruction pointer. A follow-up patch will contain less-obviously-safe changes. We'll eventually deprecate and remove the instruction-pointer insertBefore, but not before adding concise documentation of what considerations are needed (very few).
Identified with misc-include-cleaner.
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).
D137796 made these passes unused. `opt --bpf-ir-peephole` is specified in one test. Add a `registerPipelineParsingCallback` so that we can use change the test to use `opt --passes=bpf-ir-peephole` instead.
This is part of a series of patches that aim at making Function::getInstList() private. Differential Revision: https://reviews.llvm.org/D139971
When investigating an issue with bcc tool inject.py, I found
a verifier failure with latest clang. The portion of code
can be illustrated as below:
struct pid_struct {
u64 curr_call;
u64 conds_met;
u64 stack[2];
};
struct pid_struct *bpf_map_lookup_elem();
int foo() {
struct pid_struct *p = bpf_map_lookup_elem();
if (!p) return 0;
p->curr_call--;
if (p->conds_met < 1 || p->conds_met >= 3)
return 0;
if (p->stack[p->conds_met - 1] == p->curr_call)
p->conds_met--;
...
}
The verifier failure looks like:
...
8: (79) r1 = *(u64 *)(r0 +0)
R0_w=map_value(id=0,off=0,ks=4,vs=32,imm=0) R10=fp0 fp-8=mmmm????
9: (07) r1 += -1
10: (7b) *(u64 *)(r0 +0) = r1
R0_w=map_value(id=0,off=0,ks=4,vs=32,imm=0) R1_w=inv(id=0) R10=fp0 fp-8=mmmm????
11: (79) r2 = *(u64 *)(r0 +8)
R0_w=map_value(id=0,off=0,ks=4,vs=32,imm=0) R1_w=inv(id=0) R10=fp0 fp-8=mmmm????
12: (bf) r3 = r2
13: (07) r3 += -3
14: (b7) r4 = -2
15: (2d) if r4 > r3 goto pc+13
R0=map_value(id=0,off=0,ks=4,vs=32,imm=0) R1=inv(id=0) R2=inv(id=2)
R3=inv(id=0,umin_value=18446744073709551614,var_off=(0xffffffff00000000; 0xffffffff))
R4=inv-2 R10=fp0 fp-8=mmmm????
16: (07) r2 += -1
17: (bf) r3 = r2
18: (67) r3 <<= 3
19: (bf) r4 = r0
20: (0f) r4 += r3
math between map_value pointer and register with unbounded min value is not allowed
Here the compiler optimized "p->conds_met < 1 || p->conds_met >= 3" to
r2 = p->conds_met
r3 = r2
r3 += -3
r4 = -2
if (r3 < r4) return 0
r2 += -1
r3 = r2
...
In the above, r3 is initially equal to r2, but is modified used by the comparison.
But later on r2 is used again. This caused verification failure.
BPF backend has a pass, AdjustOpt, to prevent such transformation, but only
focused on signed integers since typical bpf helper returns signed integers.
To fix this case, let us handle unsigned integers as well.
Differential Revision: https://reviews.llvm.org/D121937
When checking a bcc issue related to bcc tool inject.py, I found a bug in BPFAdjustOpt pass for icmp transformation, caused by typo's. For the following condition: Cond2Op != ICmpInst::ICMP_SLT && Cond1Op != ICmpInst::ICMP_SLE it should be Cond2Op != ICmpInst::ICMP_SLT && Cond2Op != ICmpInst::ICMP_SLE This patch fixed the problem and a test case is added. Differential Revision: https://reviews.llvm.org/D121883
Patch [1] added further InstCombine trunc+icmp => mask+icmp optimization and this caused a couple of bpf selftest failure. Previous llvm BPF backend patch [2] introduced llvm.bpf.compare builtin to handle such situations. This patch further added support ">" and ">=" icmp opcodes. Tested with bpf selftests and all tests are passed including two previously failed ones. Note Patch [1] also added optimization if the to-be-compared constant is negative-power-of-2 (-C) or not-of-power-of-2 (~C). This patch didn't implement these two cases as typical bpf program compares a scalar to a positive length or boundary value, and this scalar later is used as a index into an array buffer or packet buffer. [1] https://reviews.llvm.org/D112634 [2] https://reviews.llvm.org/D112938 Differential Revision: https://reviews.llvm.org/D114215
Commit acabad9ff6bf ("[InstCombine] try to canonicalize icmp with
trunc op into mask and cmp") added a transformation to
convert "(conv)a < power_2_const" to "a & <const>" in certain
cases and bpf kernel verifier has to handle the resulted code
conservatively and this may reject otherwise legitimate program.
This commit tries to prevent such a transformation. A bpf backend
builtin llvm.bpf.compare is added. The ICMP insn, which is subject to
above InstCombine transformation, is converted to the builtin
function. The builtin function is later lowered to original ICMP insn,
certainly after InstCombine pass.
With this change, all affected bpf strobemeta* selftests are
passed now.
Differential Revision: https://reviews.llvm.org/D112938