[llvm] r304267 - ARM: Fix cmpxchg O0 expansion
Matthias Braun via llvm-commits
llvm-commits at lists.llvm.org
Tue May 30 18:21:35 PDT 2017
Author: matze
Date: Tue May 30 20:21:35 2017
New Revision: 304267
URL: http://llvm.org/viewvc/llvm-project?rev=304267&view=rev
Log:
ARM: Fix cmpxchg O0 expansion
This is the equivalent of r304048 for ARM:
- Rewrite livein calculation to use the computeLiveIns() helper
function. This is slightly less efficient but easier to reason about
and doesn't unnecessarily add pristine and reserved registers[1]
- Zero the status register at the beginning of the loop to make sure it
has a defined value.
- Remove kill flags of values that need to stay alive throughout the loop.
[1] An upcoming commit of mine will tighten the MachineVerifier to catch
these.
Modified:
llvm/trunk/lib/Target/ARM/ARMExpandPseudoInsts.cpp
llvm/trunk/test/CodeGen/ARM/cmpxchg-O0.ll
Modified: llvm/trunk/lib/Target/ARM/ARMExpandPseudoInsts.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMExpandPseudoInsts.cpp?rev=304267&r1=304266&r2=304267&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMExpandPseudoInsts.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMExpandPseudoInsts.cpp Tue May 30 20:21:35 2017
@@ -757,14 +757,9 @@ void ARMExpandPseudo::ExpandMOV32BitImm(
MI.eraseFromParent();
}
-static void addPostLoopLiveIns(MachineBasicBlock *MBB, LivePhysRegs &LiveRegs) {
- for (auto I = LiveRegs.begin(); I != LiveRegs.end(); ++I)
- MBB->addLiveIn(*I);
-}
-
/// Expand a CMP_SWAP pseudo-inst to an ldrex/strex loop as simply as
-/// possible. This only gets used at -O0 so we don't care about efficiency of the
-/// generated code.
+/// possible. This only gets used at -O0 so we don't care about efficiency of
+/// the generated code.
bool ARMExpandPseudo::ExpandCMP_SWAP(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI,
unsigned LdrexOp, unsigned StrexOp,
@@ -773,16 +768,15 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(Mac
bool IsThumb = STI->isThumb();
MachineInstr &MI = *MBBI;
DebugLoc DL = MI.getDebugLoc();
- MachineOperand &Dest = MI.getOperand(0);
+ const MachineOperand &Dest = MI.getOperand(0);
unsigned StatusReg = MI.getOperand(1).getReg();
- MachineOperand &Addr = MI.getOperand(2);
- MachineOperand &Desired = MI.getOperand(3);
- MachineOperand &New = MI.getOperand(4);
-
- LivePhysRegs LiveRegs(TII->getRegisterInfo());
- LiveRegs.addLiveOuts(MBB);
- for (auto I = std::prev(MBB.end()); I != MBBI; --I)
- LiveRegs.stepBackward(*I);
+ bool StatusDead = MI.getOperand(1).isDead();
+ // Duplicating undef operands into 2 instructions does not guarantee the same
+ // value on both; However undef should be replaced by xzr anyway.
+ assert(!MI.getOperand(2).isUndef() && "cannot handle undef");
+ unsigned AddrReg = MI.getOperand(2).getReg();
+ unsigned DesiredReg = MI.getOperand(3).getReg();
+ unsigned NewReg = MI.getOperand(4).getReg();
MachineFunction *MF = MBB.getParent();
auto LoadCmpBB = MF->CreateMachineBasicBlock(MBB.getBasicBlock());
@@ -795,25 +789,35 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(Mac
if (UxtOp) {
MachineInstrBuilder MIB =
- BuildMI(MBB, MBBI, DL, TII->get(UxtOp), Desired.getReg())
- .addReg(Desired.getReg(), RegState::Kill);
+ BuildMI(MBB, MBBI, DL, TII->get(UxtOp), DesiredReg)
+ .addReg(DesiredReg, RegState::Kill);
if (!IsThumb)
MIB.addImm(0);
MIB.add(predOps(ARMCC::AL));
}
// .Lloadcmp:
+ // mov wStatus, #0
// ldrex rDest, [rAddr]
// cmp rDest, rDesired
// bne .Ldone
- LoadCmpBB->addLiveIn(Addr.getReg());
- LoadCmpBB->addLiveIn(Dest.getReg());
- LoadCmpBB->addLiveIn(Desired.getReg());
- addPostLoopLiveIns(LoadCmpBB, LiveRegs);
+ if (!StatusDead) {
+ if (IsThumb) {
+ BuildMI(LoadCmpBB, DL, TII->get(ARM::tMOVi8), StatusReg)
+ .addDef(ARM::CPSR, RegState::Dead)
+ .addImm(0)
+ .add(predOps(ARMCC::AL));
+ } else {
+ BuildMI(LoadCmpBB, DL, TII->get(ARM::MOVi), StatusReg)
+ .addImm(0)
+ .add(predOps(ARMCC::AL))
+ .add(condCodeOp());
+ }
+ }
MachineInstrBuilder MIB;
MIB = BuildMI(LoadCmpBB, DL, TII->get(LdrexOp), Dest.getReg());
- MIB.addReg(Addr.getReg());
+ MIB.addReg(AddrReg);
if (LdrexOp == ARM::t2LDREX)
MIB.addImm(0); // a 32-bit Thumb ldrex (only) allows an offset.
MIB.add(predOps(ARMCC::AL));
@@ -821,7 +825,7 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(Mac
unsigned CMPrr = IsThumb ? ARM::tCMPhir : ARM::CMPrr;
BuildMI(LoadCmpBB, DL, TII->get(CMPrr))
.addReg(Dest.getReg(), getKillRegState(Dest.isDead()))
- .add(Desired)
+ .addReg(DesiredReg)
.add(predOps(ARMCC::AL));
unsigned Bcc = IsThumb ? ARM::tBcc : ARM::Bcc;
BuildMI(LoadCmpBB, DL, TII->get(Bcc))
@@ -835,21 +839,16 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(Mac
// strex rStatus, rNew, [rAddr]
// cmp rStatus, #0
// bne .Lloadcmp
- StoreBB->addLiveIn(Addr.getReg());
- StoreBB->addLiveIn(New.getReg());
- addPostLoopLiveIns(StoreBB, LiveRegs);
-
-
- MIB = BuildMI(StoreBB, DL, TII->get(StrexOp), StatusReg);
- MIB.add(New);
- MIB.add(Addr);
+ MIB = BuildMI(StoreBB, DL, TII->get(StrexOp), StatusReg)
+ .addReg(NewReg)
+ .addReg(AddrReg);
if (StrexOp == ARM::t2STREX)
MIB.addImm(0); // a 32-bit Thumb strex (only) allows an offset.
MIB.add(predOps(ARMCC::AL));
unsigned CMPri = IsThumb ? ARM::t2CMPri : ARM::CMPri;
BuildMI(StoreBB, DL, TII->get(CMPri))
- .addReg(StatusReg, RegState::Kill)
+ .addReg(StatusReg, getKillRegState(StatusDead))
.addImm(0)
.add(predOps(ARMCC::AL));
BuildMI(StoreBB, DL, TII->get(Bcc))
@@ -861,12 +860,24 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(Mac
DoneBB->splice(DoneBB->end(), &MBB, MI, MBB.end());
DoneBB->transferSuccessors(&MBB);
- addPostLoopLiveIns(DoneBB, LiveRegs);
MBB.addSuccessor(LoadCmpBB);
NextMBBI = MBB.end();
MI.eraseFromParent();
+
+ // Recompute livein lists.
+ const MachineRegisterInfo &MRI = MBB.getParent()->getRegInfo();
+ LivePhysRegs LiveRegs;
+ computeLiveIns(LiveRegs, MRI, *DoneBB);
+ computeLiveIns(LiveRegs, MRI, *StoreBB);
+ computeLiveIns(LiveRegs, MRI, *LoadCmpBB);
+ // Do an extra pass around the loop to get loop carried registers right.
+ StoreBB->clearLiveIns();
+ computeLiveIns(LiveRegs, MRI, *StoreBB);
+ LoadCmpBB->clearLiveIns();
+ computeLiveIns(LiveRegs, MRI, *LoadCmpBB);
+
return true;
}
@@ -894,19 +905,19 @@ bool ARMExpandPseudo::ExpandCMP_SWAP_64(
DebugLoc DL = MI.getDebugLoc();
MachineOperand &Dest = MI.getOperand(0);
unsigned StatusReg = MI.getOperand(1).getReg();
- MachineOperand &Addr = MI.getOperand(2);
- MachineOperand &Desired = MI.getOperand(3);
- MachineOperand &New = MI.getOperand(4);
+ bool StatusDead = MI.getOperand(1).isDead();
+ // Duplicating undef operands into 2 instructions does not guarantee the same
+ // value on both; However undef should be replaced by xzr anyway.
+ assert(!MI.getOperand(2).isUndef() && "cannot handle undef");
+ unsigned AddrReg = MI.getOperand(2).getReg();
+ unsigned DesiredReg = MI.getOperand(3).getReg();
+ MachineOperand New = MI.getOperand(4);
+ New.setIsKill(false);
unsigned DestLo = TRI->getSubReg(Dest.getReg(), ARM::gsub_0);
unsigned DestHi = TRI->getSubReg(Dest.getReg(), ARM::gsub_1);
- unsigned DesiredLo = TRI->getSubReg(Desired.getReg(), ARM::gsub_0);
- unsigned DesiredHi = TRI->getSubReg(Desired.getReg(), ARM::gsub_1);
-
- LivePhysRegs LiveRegs(TII->getRegisterInfo());
- LiveRegs.addLiveOuts(MBB);
- for (auto I = std::prev(MBB.end()); I != MBBI; --I)
- LiveRegs.stepBackward(*I);
+ unsigned DesiredLo = TRI->getSubReg(DesiredReg, ARM::gsub_0);
+ unsigned DesiredHi = TRI->getSubReg(DesiredReg, ARM::gsub_1);
MachineFunction *MF = MBB.getParent();
auto LoadCmpBB = MF->CreateMachineBasicBlock(MBB.getBasicBlock());
@@ -922,26 +933,21 @@ bool ARMExpandPseudo::ExpandCMP_SWAP_64(
// cmp rDestLo, rDesiredLo
// sbcs rStatus<dead>, rDestHi, rDesiredHi
// bne .Ldone
- LoadCmpBB->addLiveIn(Addr.getReg());
- LoadCmpBB->addLiveIn(Dest.getReg());
- LoadCmpBB->addLiveIn(Desired.getReg());
- addPostLoopLiveIns(LoadCmpBB, LiveRegs);
-
unsigned LDREXD = IsThumb ? ARM::t2LDREXD : ARM::LDREXD;
MachineInstrBuilder MIB;
MIB = BuildMI(LoadCmpBB, DL, TII->get(LDREXD));
addExclusiveRegPair(MIB, Dest, RegState::Define, IsThumb, TRI);
- MIB.addReg(Addr.getReg()).add(predOps(ARMCC::AL));
+ MIB.addReg(AddrReg).add(predOps(ARMCC::AL));
unsigned CMPrr = IsThumb ? ARM::tCMPhir : ARM::CMPrr;
BuildMI(LoadCmpBB, DL, TII->get(CMPrr))
.addReg(DestLo, getKillRegState(Dest.isDead()))
- .addReg(DesiredLo, getKillRegState(Desired.isDead()))
+ .addReg(DesiredLo)
.add(predOps(ARMCC::AL));
BuildMI(LoadCmpBB, DL, TII->get(CMPrr))
.addReg(DestHi, getKillRegState(Dest.isDead()))
- .addReg(DesiredHi, getKillRegState(Desired.isDead()))
+ .addReg(DesiredHi)
.addImm(ARMCC::EQ).addReg(ARM::CPSR, RegState::Kill);
unsigned Bcc = IsThumb ? ARM::tBcc : ARM::Bcc;
@@ -956,18 +962,14 @@ bool ARMExpandPseudo::ExpandCMP_SWAP_64(
// strexd rStatus, rNewLo, rNewHi, [rAddr]
// cmp rStatus, #0
// bne .Lloadcmp
- StoreBB->addLiveIn(Addr.getReg());
- StoreBB->addLiveIn(New.getReg());
- addPostLoopLiveIns(StoreBB, LiveRegs);
-
unsigned STREXD = IsThumb ? ARM::t2STREXD : ARM::STREXD;
MIB = BuildMI(StoreBB, DL, TII->get(STREXD), StatusReg);
addExclusiveRegPair(MIB, New, 0, IsThumb, TRI);
- MIB.add(Addr).add(predOps(ARMCC::AL));
+ MIB.addReg(AddrReg).add(predOps(ARMCC::AL));
unsigned CMPri = IsThumb ? ARM::t2CMPri : ARM::CMPri;
BuildMI(StoreBB, DL, TII->get(CMPri))
- .addReg(StatusReg, RegState::Kill)
+ .addReg(StatusReg, getKillRegState(StatusDead))
.addImm(0)
.add(predOps(ARMCC::AL));
BuildMI(StoreBB, DL, TII->get(Bcc))
@@ -979,12 +981,24 @@ bool ARMExpandPseudo::ExpandCMP_SWAP_64(
DoneBB->splice(DoneBB->end(), &MBB, MI, MBB.end());
DoneBB->transferSuccessors(&MBB);
- addPostLoopLiveIns(DoneBB, LiveRegs);
MBB.addSuccessor(LoadCmpBB);
NextMBBI = MBB.end();
MI.eraseFromParent();
+
+ // Recompute livein lists.
+ const MachineRegisterInfo &MRI = MBB.getParent()->getRegInfo();
+ LivePhysRegs LiveRegs;
+ computeLiveIns(LiveRegs, MRI, *DoneBB);
+ computeLiveIns(LiveRegs, MRI, *StoreBB);
+ computeLiveIns(LiveRegs, MRI, *LoadCmpBB);
+ // Do an extra pass around the loop to get loop carried registers right.
+ StoreBB->clearLiveIns();
+ computeLiveIns(LiveRegs, MRI, *StoreBB);
+ LoadCmpBB->clearLiveIns();
+ computeLiveIns(LiveRegs, MRI, *LoadCmpBB);
+
return true;
}
Modified: llvm/trunk/test/CodeGen/ARM/cmpxchg-O0.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/cmpxchg-O0.ll?rev=304267&r1=304266&r2=304267&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/cmpxchg-O0.ll (original)
+++ llvm/trunk/test/CodeGen/ARM/cmpxchg-O0.ll Tue May 30 20:21:35 2017
@@ -10,10 +10,11 @@ define { i8, i1 } @test_cmpxchg_8(i8* %a
; CHECK: dmb ish
; CHECK: uxtb [[DESIRED:r[0-9]+]], [[DESIRED]]
; CHECK: [[RETRY:.LBB[0-9]+_[0-9]+]]:
+; CHECK: mov{{s?}} [[STATUS:r[0-9]+]], #0
; CHECK: ldrexb [[OLD:r[0-9]+]], [r0]
; CHECK: cmp [[OLD]], [[DESIRED]]
; CHECK: bne [[DONE:.LBB[0-9]+_[0-9]+]]
-; CHECK: strexb [[STATUS:r[0-9]+]], r2, [r0]
+; CHECK: strexb [[STATUS]], r2, [r0]
; CHECK: cmp{{(\.w)?}} [[STATUS]], #0
; CHECK: bne [[RETRY]]
; CHECK: [[DONE]]:
@@ -29,10 +30,11 @@ define { i16, i1 } @test_cmpxchg_16(i16*
; CHECK: dmb ish
; CHECK: uxth [[DESIRED:r[0-9]+]], [[DESIRED]]
; CHECK: [[RETRY:.LBB[0-9]+_[0-9]+]]:
+; CHECK: mov{{s?}} [[STATUS:r[0-9]+]], #0
; CHECK: ldrexh [[OLD:r[0-9]+]], [r0]
; CHECK: cmp [[OLD]], [[DESIRED]]
; CHECK: bne [[DONE:.LBB[0-9]+_[0-9]+]]
-; CHECK: strexh [[STATUS:r[0-9]+]], r2, [r0]
+; CHECK: strexh [[STATUS]], r2, [r0]
; CHECK: cmp{{(\.w)?}} [[STATUS]], #0
; CHECK: bne [[RETRY]]
; CHECK: [[DONE]]:
@@ -48,10 +50,11 @@ define { i32, i1 } @test_cmpxchg_32(i32*
; CHECK: dmb ish
; CHECK-NOT: uxt
; CHECK: [[RETRY:.LBB[0-9]+_[0-9]+]]:
+; CHECK: mov{{s?}} [[STATUS:r[0-9]+]], #0
; CHECK: ldrex [[OLD:r[0-9]+]], [r0]
; CHECK: cmp [[OLD]], [[DESIRED]]
; CHECK: bne [[DONE:.LBB[0-9]+_[0-9]+]]
-; CHECK: strex [[STATUS:r[0-9]+]], r2, [r0]
+; CHECK: strex [[STATUS]], r2, [r0]
; CHECK: cmp{{(\.w)?}} [[STATUS]], #0
; CHECK: bne [[RETRY]]
; CHECK: [[DONE]]:
More information about the llvm-commits
mailing list