fix clang-format warnings.

use llvm_unreachable() in default case of the switch statement

• remove -mcpu=v4.
• for new instructions (except xadd), for 32bit mode, only alu32 mode is supported. I chose this way since we have -mcpu=v3 for a while which has alu32 as default. We really want to promote alu32 mode. The new kernel which supports atomic op definitely supports alu32 well...

@niravd Looks like https://reviews.llvm.org/D91833 fixed the issue. I have created a BPF patch https://reviews.llvm.org/D92451 with a test case so if anything happens we can detect the regression. Could you take a look whether https://reviews.llvm.org/D91833 can truely fix the issue?

Looks like https://reviews.llvm.org/D91833 fixed the issue. Abandon this.

As an alternative, we could update ImproveChain (and visitLIFETIME_END) to limit the aliasing
around lifetime_start / end to disallow improving the chain dependence of a mem op node
from a different lifetime node that may alias. That should prohibit access from two aliasable
frame indices from being concurrent while still allowing us to leverage that disjoint allocs should be disjoint.

• remove atomic_fetch_sub which can be implemented with neg + atomic_fetch_add
• add support for xand, xor, xxor (xadd already been supported)
• for any given __sync_fetch_and_{add, and, or, xor}, llvm will generate either atomic_fetch_<op> or x<op> depending on whether the return value is used or not.

The change looks good to me. Thanks!

No problem. I will wait for that dependent patch getting merged and then will do the review.

I hit a build failure with latest llvm-project repo (top commit e42021d5cc25 ("[Clang][-fvisibility-from-dllstorageclass] Set DSO Locality from final visibility")).

Ya, the above llvm crash is expected as bpf backend does not handle AtomicStore.

add some comments in test w.r.t. __sync_lock_test_and_set()

okay, my fault. I must have uploaded an old version of the patch so comment is not in the test. will upload the new version tonight.

In D72184#2407264, @ast wrote:

Looks like the test didn't change. Only commit log... that's fine. I think the diff is ready to land, but let's wait for the kernel patches to be ready as well.

add proper comment in test and clarify in commit message for __sync_lock_test_and_set which actually means an atomic exchange operation.

• remove all char/short atomic operations
• remove barrier instruction

Thanks. Somehow my build is successful. I kind of know what is the possible issue but wonder why I did not hit it.

Can we please keep barriers out of scope? I think there's a lot of design to be done there and I'd rather just get the core atomics working first.

The reason I put it here is to make *cpu=v4* roughly complete. Because if you adds barrier later we may need to add "cpu=v5", and I want to avoid that. But I agree this is a little bit overwhelming on you and let us discuss in bpf office hour how to proceed.

add support for a barrier function. The correspond C code is sync_synchronize().
If we want to have variant like barrier for read/write/rw, etc. we may need to define bpf specific barrier functions as sync_synchronize() does not allow taking additional parameters. I tentatively use one particular encoding but we can certainly change that later.

add fetch_and_{and,or,xor} support
force cmpxchg insn return results in r0 like r0 = cmpxchg(addr, r0, new)

make sense. will add and/or/xor support in the next revision. will also think about how to support hardware-level barrier. Totally agree that we should have adequate atomic op support in cpu=v4.

use the same register for dst and val for fetch-add, fetch-sub and xchg instructions.
for fetch-sub, if it is deemed later using different registers for dst/val is preferred, I can make the change then.

fix clang-format warning

• remove RFC tag.
• addressed comments for encoding, etc.

changed encoding based on internal discussion on encoding
add a test

In D89149#2333147, @niravd wrote:

As an alternative, we could update ImproveChain (and visitLIFETIME_END) to limit the aliasing around lifetime_start / end to disallow improving the chain dependence of a mem op node from a different lifetime node that may alias. That should prohibit access from two aliasable frame indices from being concurrent while still allowing us to leverage that disjoint allocs should be disjoint.

ping @courbet @niravd, any comments on this patch?

LGTM. Thanks!

Thanks for explanation for IsRequired(), really appreciated.

If I understand correctly, "isRequired()" means this pass has to run. In this regard, BPFPreserveDIType also need isRequired().
Could you explain a little bit what if one pass does not have "isRequired()"? How PassManager deals with it?

• added tests, added llc option to disable optimization if needed.
• removed RFC tag.

• got an even simpler fix. Just change the map to hold reloc globals GEPGlobals to be class static, so it will be available for all instances of BPFAbstractMemberAccess. this also resolved the issue for NPM nicely.

BTW, can you share what is the plan for NPM? Any expected time to turn it on as the default?

I think I got how to enable npm in opt. It is -passes option in opt -passes='default<O2>' %s. It looks good. Will stamp it.

@aeubanks I assume this is related to new pass manager and is not on by default? What is the option I could turn npm on and give a try? Looks like Hexagon is another backend which has implemented npm support. We may plan to implement new (early or module) passes so we need to modify both adjustPassManager and registerPassBuilderCallbacks. Understanding how it really functions will help our future work. The code looks correct, esp. when comparing to what Hexagon has done, but I would like to experiment a little bit before accepting the patch. Thanks!

• remove RFC tag as proper tests are added
• added "opt" options so the added optimizations can be disabled.

In D88525#2304656, @ast wrote:

looks like x86 64-bit is ILP, so it's not niche.
Looks like Source gets the least amount of testing (judging by archs that use it).
RegPressure is probably 2nd most used after ILP.
It feels that we should fix it inside ILP instead.

removed RFC tag as bpf selftests are solved.
using setSchedulingPreference() instead of enableMachineScheduler() to limit the scope
Regarding previous reported failures for bpf selftest test_verif_scale3.o:

• current llvm-project master: processed 997315 insns
• enableMachineScheduler() processed 1005967 insns (hacked verifier to increase to permit 2M insns), resulted from more instructions due to enabled machine scheduling phase (and possibly additional regalloc functionality, not verified).
• setSchedulingPreference() processed 998991 insns

rebase since https://reviews.llvm.org/D87153 changed

• • remove RFC tag. The patch is complete and all tests are adjusted and passed.
  • remove the adjustIR phase which tries to coalescing two reloc-globals. My previous implementation did not compare the *last* type in the first reloc-global and the *first* type in the second reloc-global. This will require to traverse the access string (0:1:1:...). This seems too much work for this little optimization. I will delay it into the future as an enhancement.
• adjusted existing clang/llvm CORE tests so they can pass.

add seq_num arg to passthrough builtin

add seq_num arg to passthrough builtin.

minor change: add some more conditions to further restrict transformation.

In D85570#2266620, @ast wrote:

is this based on D87153 ?
the passthrough will be removed later, right?

Abandon this one. The current approach is to add "user barriers" in an IR phase before simplifyCFG/instrcombine to disable unfavorable speculative code motion.

Abandon this one. The current approach is to introduce some kind of user barriers before instcombine to prevent such optimization in IR level.

A few changes:

• AdjustOpt IR pass only called at EP_ModuleOptimizerEarly now
• remove the pass which deals with a condition is used in more than one branch insns. This will be added to BPFCheckAndAdjustIR later.

• introduced __builtin_bpf_passthrough() and use it for user barrier instead of inline asm. I found inline asm has some impact on inlining when running selftests (specifically checked for map_ptr_kern.c).
• introduced an IR pass at the beginning of target dependent optimizations to (1). ensure reloc globals not in PHI node, (2). remove __builtin_bpf_passthrough(). (3). coalescing reloc globals if possible.

This patch can now pass all selftests.

add another adjustopt phase right after clang codegen to prevent certain early CSE optimization for __builtin_preserve_* intrinsics.

now all bpf selftests can pass.