4ast wrote:

> Since load_acquire and store_release are essentially atomic operations (e.g. 
> __atomic_load_n() and __atomic_store_n()), > is it possible to add 
> acquire/release support in BPF_ATOMIC?

We discussed it with Eduard as well and came to similar conclusion.
BPF_STX | BPF_ATOMIC  and imm = LOAD_ACQ or imm = STORE_REL
would fit better than using ModeModifier for these two new insns.

The verifier is using Mode to express either arena or probe-ing needs.
Like BPF_STX | BPF_ATOMIC is converted to BPF_STX | BPF_PROBE_ATOMIC by the 
verifier
and JITs that don't support atomic insns in arena will complain about such 
insns,
because such opcode will not be found in the big switch() statement.

New load_acq/store_rel insn would need to be supported in arena.
If we use Mode for them it will be hard to convey the change to JITs.
But if we go with imm = LOAD_ACQ or imm = STORE_REL approach
the BPF_ATOMIC -> BPF_PROBE_ATOMIC conversion will cover these insns 
automatically.
We need to tweak bpf_jit_supports_insn(), but internal protocol between the 
verifier and JITs
will stay the same.

BPF_STX | BPF_ATOMIC opcode for load acquire insn may look a bit odd,
but once you consider that this opcode is used to return the value for 
atomic_fetch_or|add... insns
it doesn't look as odd.

https://github.com/llvm/llvm-project/pull/108636
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to