On 2/27/19 12:50 PM, Jiong Wang wrote: > Support sub-register code-gen for XADD is like supporting any other Load > and Store patterns. > > No new instruction is introduced. > > lock *(u32 *)(r1 + 0) += w2 > > has exactly the same underlying insn as: > > lock *(u32 *)(r1 + 0) += r2 > > BPF_W width modifier has guaranteed they behave the same at runtime. This > patch merely teaches BPF back-end that BPF_W width modifier could work > GPR32 register class and that's all needed for sub-register code-gen > support for XADD. > > test/CodeGen/BPF/xadd.ll updated to include sub-register code-gen tests. > > A new testcase test/CodeGen/BPF/xadd_legal.ll is added to make sure the > legal case could pass on all code-gen modes. It could also test dead Def > check on GPR32. If there is no proper handling like what has been done > inside BPFMIChecking.cpp:hasLivingDefs, then this testcase will fail. > > Signed-off-by: Jiong Wang <jiong.wang@xxxxxxxxxxxxx> > --- > lib/Target/BPF/BPFInstrInfo.td | 28 ++++++++++++++++++++++++---- > lib/Target/BPF/BPFMIChecking.cpp | 4 +++- > test/CodeGen/BPF/xadd.ll | 2 ++ > test/CodeGen/BPF/xadd_legal.ll | 26 ++++++++++++++++++++++++++ > 4 files changed, 55 insertions(+), 5 deletions(-) > create mode 100644 test/CodeGen/BPF/xadd_legal.ll > > diff --git a/lib/Target/BPF/BPFInstrInfo.td b/lib/Target/BPF/BPFInstrInfo.td > index 36724b8..c44702a 100644 > --- a/lib/Target/BPF/BPFInstrInfo.td > +++ b/lib/Target/BPF/BPFInstrInfo.td > @@ -614,11 +614,31 @@ class XADD<BPFWidthModifer SizeOp, string OpcodeStr, PatFrag OpNode> > let BPFClass = BPF_STX; > } > > +class XADD32<BPFWidthModifer SizeOp, string OpcodeStr, PatFrag OpNode> > + : TYPE_LD_ST<BPF_XADD.Value, SizeOp.Value, > + (outs GPR32:$dst), > + (ins MEMri:$addr, GPR32:$val), > + "lock *("#OpcodeStr#" *)($addr) += $val", > + [(set GPR32:$dst, (OpNode ADDRri:$addr, GPR32:$val))]> { > + bits<4> dst; > + bits<20> addr; > + > + let Inst{51-48} = addr{19-16}; // base reg > + let Inst{55-52} = dst; > + let Inst{47-32} = addr{15-0}; // offset > + let BPFClass = BPF_STX; > +} > + > let Constraints = "$dst = $val" in { > -def XADD32 : XADD<BPF_W, "u32", atomic_load_add_32>; > -def XADD64 : XADD<BPF_DW, "u64", atomic_load_add_64>; > -// undefined def XADD16 : XADD<1, "xadd16", atomic_load_add_16>; > -// undefined def XADD8 : XADD<2, "xadd8", atomic_load_add_8>; > + let Predicates = [BPFNoALU32] in { > + def XADDW : XADD<BPF_W, "u32", atomic_load_add_32>; > + } > + > + let Predicates = [BPFHasALU32], DecoderNamespace = "BPFALU32" in { > + def XADDW32 : XADD32<BPF_W, "u32", atomic_load_add_32>; > + } > + > + def XADDD : XADD<BPF_DW, "u64", atomic_load_add_64>; The XADD64 is changed to XADDD. I guess I am okay with that since it is consistent with LDD/STD. > } > > // bswap16, bswap32, bswap64 > diff --git a/lib/Target/BPF/BPFMIChecking.cpp b/lib/Target/BPF/BPFMIChecking.cpp > index 7a4ba96..938e539 100644 > --- a/lib/Target/BPF/BPFMIChecking.cpp > +++ b/lib/Target/BPF/BPFMIChecking.cpp > @@ -165,7 +165,9 @@ static bool hasLivingDefs(const MachineInstr &MI, > void BPFMIPreEmitChecking::checkingIllegalXADD(void) { > for (MachineBasicBlock &MBB : *MF) { > for (MachineInstr &MI : MBB) { > - if (MI.getOpcode() != BPF::XADD32 && MI.getOpcode() != BPF::XADD64) > + if (MI.getOpcode() != BPF::XADDW && > + MI.getOpcode() != BPF::XADDD && > + MI.getOpcode() != BPF::XADDW32) > continue; > > LLVM_DEBUG(MI.dump()); > diff --git a/test/CodeGen/BPF/xadd.ll b/test/CodeGen/BPF/xadd.ll > index c5f2620..49f0c10 100644 > --- a/test/CodeGen/BPF/xadd.ll > +++ b/test/CodeGen/BPF/xadd.ll > @@ -1,5 +1,7 @@ > ; RUN: not llc -march=bpfel < %s 2>&1 | FileCheck %s > ; RUN: not llc -march=bpfeb < %s 2>&1 | FileCheck %s > +; RUN: not llc -march=bpfel -mattr=+alu32 < %s 2>&1 | FileCheck %s > +; RUN: not llc -march=bpfeb -mattr=+alu32 < %s 2>&1 | FileCheck %s > > ; This file is generated with the source command and source > ; $ clang -target bpf -O2 -g -S -emit-llvm t.c > diff --git a/test/CodeGen/BPF/xadd_legal.ll b/test/CodeGen/BPF/xadd_legal.ll > new file mode 100644 > index 0000000..0d30084 > --- /dev/null > +++ b/test/CodeGen/BPF/xadd_legal.ll > @@ -0,0 +1,26 @@ > +; RUN: llc -march=bpfel < %s 2>&1 | FileCheck --check-prefix=CHECK-64 %s > +; RUN: llc -march=bpfeb < %s 2>&1 | FileCheck --check-prefix=CHECK-64 %s > +; RUN: llc -march=bpfel -mattr=+alu32 < %s 2>&1 | FileCheck --check-prefix=CHECK-32 %s > +; RUN: llc -march=bpfeb -mattr=+alu32 < %s 2>&1 | FileCheck --check-prefix=CHECK-32 %s > + > +; This file is generated with the source command and source > +; $ clang -target bpf -O2 -S -emit-llvm t.c > +; $ cat t.c > +; int test(int *ptr, unsigned long long a) { > +; __sync_fetch_and_add(ptr, a); > +; return *ptr; > +; } > +; > +; NOTE: passing unsigned long long as the second operand of __sync_fetch_and_add > +; could effectively create sub-register reference coming from indexing a full > +; register which could then exerceise hasLivingDefs inside BPFMIChecker.cpp. > + > +define dso_local i32 @test(i32* nocapture %ptr, i64 %a) { > +entry: > + %conv = trunc i64 %a to i32 > + %0 = atomicrmw add i32* %ptr, i32 %conv seq_cst > +; CHECK-64: lock *(u32 *)(r1 + 0) += r2 > +; CHECK-32: lock *(u32 *)(r1 + 0) += w2 > + %1 = load i32, i32* %ptr, align 4 > + ret i32 %1 > +} >