[PATCH] D72184: [BPF] support atomic instructions

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added inline comments.


================
Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:783
+  let Inst{47-32} = addr{15-0}; // offset
+  let Inst{11-8} = new;
+  let Inst{7-4} = BPF_CMPXCHG.Value;
----------------
ast wrote:
> yonghong-song wrote:
> > jackmanb wrote:
> > > If we go down the route of matching operands with x86 as you have done for `XFALU64` and `XCHG`, I think  we should also do it for cmpxchg.
> > > 
> > > IIUC this is `dst = atomic_cmpxchg(*(src + off), r0, new);` 
> > > 
> > > But to do it in a single x86 instruction we need to have only 2 operands + the hard-coded r0. `r0 = atomic_xmpxchg(*(dst + off), r0, src);` would seem most natural to me.
> > We can do this:
> >   r0 = atomic_xmpxchg(*(dst + off), r0, src);
> I'm confused. Isn't that what that is already?
> r0 = atomic_cmpxchg(*(dst + off), r0, src);
> 
> In "class CMPXCHG":
> let Inst{51-48} = addr{19-16}; // this is 'dst_reg' in bpf_insn.
> let Inst{55-52} = new; // this is 'src_reg' in bpf_insn.
> 
> Same as old xadd and new fetch_add insns.
> What am I missing?
I just fixed to return R0/W0 in the early afternoon. So you looked at old comments with new code and that is why it is confusion :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72184/new/

https://reviews.llvm.org/D72184