yonghong-song added inline comments.
================ Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:699 + let Inst{51-48} = addr{19-16}; // base reg + let Inst{55-52} = dst; + let Inst{47-32} = addr{15-0}; // offset ---------------- jackmanb wrote: > There is another mismatch between what I implemented in the kernel and what > you've implemented here, which is that I used `dst` as the pointer base and > `src` as the writeback register but you did the opposite. > > IIUC what we have here at the moment is `dst = sync_fetch_and_add(*(src + > off), dst);`. The advantage is that `dst` makes more intuitive sense as a > writeback register. > > If we instead had `src = sync_fetch_and_add(*(dst + off), src)` we have the > unintuitive writeback to `src`, but now a) the 2nd argument (the addend) > makes more intuitive sense, and b) `dst` being the pointer base is consistent > with the rest of `BPF_STX`. could you double check? my encoding is actually dst = sync_fetch_and_add(*(src + off), dst);. The encoding itself is very similar to xadd/stx. ================ 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; ---------------- 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); Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72184/new/ https://reviews.llvm.org/D72184 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits