Skip to content

Commit

Permalink
[BPF] Do not convert atomic_fetch_and_*() to atomic_<op>()'s
Browse files Browse the repository at this point in the history 
In previous commit, atomic_fetch_and_*() operations are converted
to atomic_<op>()'s if there are no return values. This is not
what we want, we would like to preserve atomic_fetch_and_*() insn
so bpf jit can add proper barrier insns.

Preserving atomic_fetch_and_*() are okay for most __sync_fetch_and_*()
functions, but not for __sync_fetch_and_add() since
__sync_fetch_and_add() has been used to generic locked insns
in cpu=v1/v2.

So after preserving atomic_fetch_and_*() even if return value
is not used, XFADDD in BPFInstrInfo.td is adjusted to emit
locked add insn for cpu v1/v2 and to emit atomic_fetch_and_add()
for cpu >= v3.
  • Loading branch information
Yonghong Song committed Aug 29, 2024
1 parent 98d3e3f commit 1aa05fd
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 94 deletions.
2 changes: 1 addition & 1 deletion llvm/lib/Target/BPF/BPF.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ InstructionSelector *createBPFInstructionSelector(const BPFTargetMachine &,
void initializeBPFCheckAndAdjustIRPass(PassRegistry&);
void initializeBPFDAGToDAGISelLegacyPass(PassRegistry &);
void initializeBPFMIPeepholePass(PassRegistry &);
void initializeBPFMIPreEmitCheckingPass(PassRegistry&);
void initializeBPFMIPreEmitCheckingPass(PassRegistry &);
void initializeBPFMIPreEmitPeepholePass(PassRegistry &);
void initializeBPFMISimplifyPatchablePass(PassRegistry &);

Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/Target/BPF/BPFInstrInfo.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,7 @@ class XADD<BPFWidthModifer SizeOp, string OpcodeStr, PatFrag OpNode>
let Constraints = "$dst = $val" in {
let Predicates = [BPFNoALU32] in {
def XADDW : XADD<BPF_W, "u32", atomic_load_add_i32>;
def XADDD : XADD<BPF_DW, "u64", atomic_load_add_i64>;
}
}

Expand Down Expand Up @@ -849,8 +850,6 @@ let Constraints = "$dst = $val" in {
def XORW32 : ATOMIC32_NOFETCH<BPF_OR, "|">;
def XXORW32 : ATOMIC32_NOFETCH<BPF_XOR, "^">;
}

def XADDD : ATOMIC_NOFETCH<BPF_ADD, "+">;
def XANDD : ATOMIC_NOFETCH<BPF_AND, "&">;
def XORD : ATOMIC_NOFETCH<BPF_OR, "|">;
def XXORD : ATOMIC_NOFETCH<BPF_XOR, "^">;
Expand Down Expand Up @@ -901,7 +900,9 @@ let Constraints = "$dst = $val" in {
def XFXORW32 : XFALU32<BPF_W, BPF_XOR, "u32", "xor", atomic_load_xor_i32>;
}

def XFADDD : XFALU64<BPF_DW, BPF_ADD, "u64", "add", atomic_load_add_i64>;
let Predicates = [BPFHasALU32] in {
def XFADDD : XFALU64<BPF_DW, BPF_ADD, "u64", "add", atomic_load_add_i64>;
}
def XFANDD : XFALU64<BPF_DW, BPF_AND, "u64", "and", atomic_load_and_i64>;
def XFORD : XFALU64<BPF_DW, BPF_OR, "u64", "or", atomic_load_or_i64>;
def XFXORD : XFALU64<BPF_DW, BPF_XOR, "u64", "xor", atomic_load_xor_i64>;
Expand Down
84 changes: 7 additions & 77 deletions llvm/lib/Target/BPF/BPFMIChecking.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,14 @@ struct BPFMIPreEmitChecking : public MachineFunctionPass {
// Initialize class variables.
void initialize(MachineFunction &MFParm);

bool processAtomicInsts();
void processAtomicInsts();

public:

// Main entry point for this pass.
bool runOnMachineFunction(MachineFunction &MF) override {
if (!skipFunction(MF.getFunction())) {
initialize(MF);
return processAtomicInsts();
processAtomicInsts();
}
return false;
}
Expand Down Expand Up @@ -107,7 +106,7 @@ void BPFMIPreEmitChecking::initialize(MachineFunction &MFParm) {
// Dead correctly, and it is safe to use such information or our purpose.
static bool hasLiveDefs(const MachineInstr &MI, const TargetRegisterInfo *TRI) {
const MCRegisterClass *GPR64RegClass =
&BPFMCRegisterClasses[BPF::GPRRegClassID];
&BPFMCRegisterClasses[BPF::GPRRegClassID];
std::vector<unsigned> GPR32LiveDefs;
std::vector<unsigned> GPR64DeadDefs;

Expand Down Expand Up @@ -153,12 +152,10 @@ static bool hasLiveDefs(const MachineInstr &MI, const TargetRegisterInfo *TRI) {
return false;
}

bool BPFMIPreEmitChecking::processAtomicInsts() {
void BPFMIPreEmitChecking::processAtomicInsts() {
for (MachineBasicBlock &MBB : *MF) {
for (MachineInstr &MI : MBB) {
if (MI.getOpcode() != BPF::XADDW &&
MI.getOpcode() != BPF::XADDD &&
MI.getOpcode() != BPF::XADDW32)
if (MI.getOpcode() != BPF::XADDW && MI.getOpcode() != BPF::XADDD)
continue;

LLVM_DEBUG(MI.dump());
Expand All @@ -171,81 +168,14 @@ bool BPFMIPreEmitChecking::processAtomicInsts() {
}
}
}

// Check return values of atomic_fetch_and_{add,and,or,xor}.
// If the return is not used, the atomic_fetch_and_<op> instruction
// is replaced with atomic_<op> instruction.
MachineInstr *ToErase = nullptr;
bool Changed = false;
const BPFInstrInfo *TII = MF->getSubtarget<BPFSubtarget>().getInstrInfo();
for (MachineBasicBlock &MBB : *MF) {
for (MachineInstr &MI : MBB) {
if (ToErase) {
ToErase->eraseFromParent();
ToErase = nullptr;
}

if (MI.getOpcode() != BPF::XFADDW32 && MI.getOpcode() != BPF::XFADDD &&
MI.getOpcode() != BPF::XFANDW32 && MI.getOpcode() != BPF::XFANDD &&
MI.getOpcode() != BPF::XFXORW32 && MI.getOpcode() != BPF::XFXORD &&
MI.getOpcode() != BPF::XFORW32 && MI.getOpcode() != BPF::XFORD)
continue;

if (hasLiveDefs(MI, TRI))
continue;

LLVM_DEBUG(dbgs() << "Transforming "; MI.dump());
unsigned newOpcode;
switch (MI.getOpcode()) {
case BPF::XFADDW32:
newOpcode = BPF::XADDW32;
break;
case BPF::XFADDD:
newOpcode = BPF::XADDD;
break;
case BPF::XFANDW32:
newOpcode = BPF::XANDW32;
break;
case BPF::XFANDD:
newOpcode = BPF::XANDD;
break;
case BPF::XFXORW32:
newOpcode = BPF::XXORW32;
break;
case BPF::XFXORD:
newOpcode = BPF::XXORD;
break;
case BPF::XFORW32:
newOpcode = BPF::XORW32;
break;
case BPF::XFORD:
newOpcode = BPF::XORD;
break;
default:
llvm_unreachable("Incorrect Atomic Instruction Opcode");
}

BuildMI(MBB, MI, MI.getDebugLoc(), TII->get(newOpcode))
.add(MI.getOperand(0))
.add(MI.getOperand(1))
.add(MI.getOperand(2))
.add(MI.getOperand(3));

ToErase = &MI;
Changed = true;
}
}

return Changed;
}

} // end default namespace
} // namespace

INITIALIZE_PASS(BPFMIPreEmitChecking, "bpf-mi-pemit-checking",
"BPF PreEmit Checking", false, false)

char BPFMIPreEmitChecking::ID = 0;
FunctionPass* llvm::createBPFMIPreEmitCheckingPass()
{
FunctionPass* llvm::createBPFMIPreEmitCheckingPass() {
return new BPFMIPreEmitChecking();
}
8 changes: 4 additions & 4 deletions llvm/test/CodeGen/BPF/atomics.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
; CHECK-LABEL: test_load_add_32
; CHECK: lock *(u32 *)(r1 + 0) += r2
; CHECK: encoding: [0xc3,0x21
; CHECK-V3: lock *(u32 *)(r1 + 0) += w2
; CHECK-V3: encoding: [0xc3,0x21,0x00,0x00,0x00,0x00,0x00,0x00]
; CHECK-V3: w2 = atomic_fetch_add((u32 *)(r1 + 0), w2)
; CHECK-V3: encoding: [0xc3,0x21,0x00,0x00,0x01,0x00,0x00,0x00]
define void @test_load_add_32(ptr %p, i32 zeroext %v) {
entry:
atomicrmw add ptr %p, i32 %v seq_cst
Expand All @@ -15,8 +15,8 @@ entry:
; CHECK-LABEL: test_load_add_64
; CHECK: lock *(u64 *)(r1 + 0) += r2
; CHECK: encoding: [0xdb,0x21
; CHECK-V3: lock *(u64 *)(r1 + 0) += r2
; CHECK-V3: encoding: [0xdb,0x21,0x00,0x00,0x00,0x00,0x00,0x00]
; CHECK-V3: r2 = atomic_fetch_add((u64 *)(r1 + 0), r2)
; CHECK-V3: encoding: [0xdb,0x21,0x00,0x00,0x01,0x00,0x00,0x00]
define void @test_load_add_64(ptr %p, i64 zeroext %v) {
entry:
atomicrmw add ptr %p, i64 %v seq_cst
Expand Down
16 changes: 8 additions & 8 deletions llvm/test/CodeGen/BPF/atomics_2.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ entry:
}

; CHECK-LABEL: test_atomic_xor_32
; CHECK: lock *(u32 *)(r1 + 0) ^= w2
; CHECK: encoding: [0xc3,0x21,0x00,0x00,0xa0,0x00,0x00,0x00]
; CHECK: w2 = atomic_fetch_xor((u32 *)(r1 + 0), w2)
; CHECK: encoding: [0xc3,0x21,0x00,0x00,0xa1,0x00,0x00,0x00]
; CHECK: w0 = 0
define dso_local i32 @test_atomic_xor_32(ptr nocapture %p, i32 %v) local_unnamed_addr {
entry:
Expand All @@ -224,8 +224,8 @@ entry:
}

; CHECK-LABEL: test_atomic_xor_64
; CHECK: lock *(u64 *)(r1 + 0) ^= r2
; CHECK: encoding: [0xdb,0x21,0x00,0x00,0xa0,0x00,0x00,0x00]
; CHECK: atomic_fetch_xor((u64 *)(r1 + 0), r2)
; CHECK: encoding: [0xdb,0x21,0x00,0x00,0xa1,0x00,0x00,0x00]
; CHECK: w0 = 0
define dso_local i32 @test_atomic_xor_64(ptr nocapture %p, i64 %v) local_unnamed_addr {
entry:
Expand All @@ -234,8 +234,8 @@ entry:
}

; CHECK-LABEL: test_atomic_and_64
; CHECK: lock *(u64 *)(r1 + 0) &= r2
; CHECK: encoding: [0xdb,0x21,0x00,0x00,0x50,0x00,0x00,0x00]
; CHECK: r2 = atomic_fetch_and((u64 *)(r1 + 0), r2)
; CHECK: encoding: [0xdb,0x21,0x00,0x00,0x51,0x00,0x00,0x00]
; CHECK: w0 = 0
define dso_local i32 @test_atomic_and_64(ptr nocapture %p, i64 %v) local_unnamed_addr {
entry:
Expand All @@ -244,8 +244,8 @@ entry:
}

; CHECK-LABEL: test_atomic_or_64
; CHECK: lock *(u64 *)(r1 + 0) |= r2
; CHECK: encoding: [0xdb,0x21,0x00,0x00,0x40,0x00,0x00,0x00]
; CHECK: r2 = atomic_fetch_or((u64 *)(r1 + 0), r2)
; CHECK: encoding: [0xdb,0x21,0x00,0x00,0x41,0x00,0x00,0x00]
; CHECK: w0 = 0
define dso_local i32 @test_atomic_or_64(ptr nocapture %p, i64 %v) local_unnamed_addr {
entry:
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/BPF/xadd_legal.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ entry:
%conv = trunc i64 %a to i32
%0 = atomicrmw add ptr %ptr, i32 %conv seq_cst
; CHECK-64: lock *(u32 *)(r1 + 0) += r2
; CHECK-32: lock *(u32 *)(r1 + 0) += w2
; CHECK-32: w2 = atomic_fetch_add((u32 *)(r1 + 0), w2)
%1 = load i32, ptr %ptr, align 4
ret i32 %1
}

0 comments on commit 1aa05fd

Please sign in to comment.