[llvm-branch-commits] [llvm-branch] r310628 - Merging r310534:
Hans Wennborg via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Aug 10 10:12:27 PDT 2017
Author: hans
Date: Thu Aug 10 10:12:27 2017
New Revision: 310628
URL: http://llvm.org/viewvc/llvm-project?rev=310628&view=rev
Log:
Merging r310534:
------------------------------------------------------------------------
r310534 | matze | 2017-08-09 15:22:05 -0700 (Wed, 09 Aug 2017) | 20 lines
ARM: Fix CMP_SWAP expansion
Clean up after my misguided attempt in r304267 to "fix" CMP_SWAP
returning an uninitialized status value.
- I was always using tMOVi8 to zero the status register which cannot
encode higher register numbers and llvm would silently miscompile)
- Nobody was ever looking at that status value outside the expansion.
ARMDAGToDAGISel::SelectCMP_SWAP() the only place creating CMP_SWAP
instructions was not mapping anything to it. (The cmpxchg status value
from llvm IR is lowered to a manual comparison after the CMP_SWAP)
So this:
- Renames the register from "status" to "temp" it make it obvious that
it isn't used outside the expansion.
- Remove the zeroing status/temp register.
- Keep the live-in list improvements from r304267
Fixes http://llvm.org/PR34056
------------------------------------------------------------------------
Modified:
llvm/branches/release_50/ (props changed)
llvm/branches/release_50/lib/Target/ARM/ARMExpandPseudoInsts.cpp
llvm/branches/release_50/lib/Target/ARM/ARMInstrInfo.td
llvm/branches/release_50/test/CodeGen/ARM/cmpxchg-O0.ll
Propchange: llvm/branches/release_50/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Aug 10 10:12:27 2017
@@ -1,3 +1,3 @@
/llvm/branches/Apple/Pertwee:110850,110961
/llvm/branches/type-system-rewrite:133420-134817
-/llvm/trunk:155241,308483-308484,308503,308808,308813,308891,308906,308950,308963,308978,308986,309113,309302,309321,309323,309325,309330,309343,309353,309355,309422,309481,309483,309495,309555,309561,309594,309651,309744,309758,309849,309928,309930,310071,310190,310240-310242,310250,310253,310267
+/llvm/trunk:155241,308483-308484,308503,308808,308813,308891,308906,308950,308963,308978,308986,309113,309302,309321,309323,309325,309330,309343,309353,309355,309422,309481,309483,309495,309555,309561,309594,309651,309744,309758,309849,309928,309930,310071,310190,310240-310242,310250,310253,310267,310534
Modified: llvm/branches/release_50/lib/Target/ARM/ARMExpandPseudoInsts.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_50/lib/Target/ARM/ARMExpandPseudoInsts.cpp?rev=310628&r1=310627&r2=310628&view=diff
==============================================================================
--- llvm/branches/release_50/lib/Target/ARM/ARMExpandPseudoInsts.cpp (original)
+++ llvm/branches/release_50/lib/Target/ARM/ARMExpandPseudoInsts.cpp Thu Aug 10 10:12:27 2017
@@ -769,8 +769,7 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(Mac
MachineInstr &MI = *MBBI;
DebugLoc DL = MI.getDebugLoc();
const MachineOperand &Dest = MI.getOperand(0);
- unsigned StatusReg = MI.getOperand(1).getReg();
- bool StatusDead = MI.getOperand(1).isDead();
+ unsigned TempReg = MI.getOperand(1).getReg();
// 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");
@@ -797,23 +796,9 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(Mac
}
// .Lloadcmp:
- // mov wStatus, #0
// ldrex rDest, [rAddr]
// cmp rDest, rDesired
// bne .Ldone
- 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());
@@ -836,10 +821,10 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(Mac
LoadCmpBB->addSuccessor(StoreBB);
// .Lstore:
- // strex rStatus, rNew, [rAddr]
- // cmp rStatus, #0
+ // strex rTempReg, rNew, [rAddr]
+ // cmp rTempReg, #0
// bne .Lloadcmp
- MIB = BuildMI(StoreBB, DL, TII->get(StrexOp), StatusReg)
+ MIB = BuildMI(StoreBB, DL, TII->get(StrexOp), TempReg)
.addReg(NewReg)
.addReg(AddrReg);
if (StrexOp == ARM::t2STREX)
@@ -848,7 +833,7 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(Mac
unsigned CMPri = IsThumb ? ARM::t2CMPri : ARM::CMPri;
BuildMI(StoreBB, DL, TII->get(CMPri))
- .addReg(StatusReg, getKillRegState(StatusDead))
+ .addReg(TempReg, RegState::Kill)
.addImm(0)
.add(predOps(ARMCC::AL));
BuildMI(StoreBB, DL, TII->get(Bcc))
@@ -904,8 +889,7 @@ bool ARMExpandPseudo::ExpandCMP_SWAP_64(
MachineInstr &MI = *MBBI;
DebugLoc DL = MI.getDebugLoc();
MachineOperand &Dest = MI.getOperand(0);
- unsigned StatusReg = MI.getOperand(1).getReg();
- bool StatusDead = MI.getOperand(1).isDead();
+ unsigned TempReg = MI.getOperand(1).getReg();
// 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");
@@ -931,7 +915,7 @@ bool ARMExpandPseudo::ExpandCMP_SWAP_64(
// .Lloadcmp:
// ldrexd rDestLo, rDestHi, [rAddr]
// cmp rDestLo, rDesiredLo
- // sbcs rStatus<dead>, rDestHi, rDesiredHi
+ // sbcs rTempReg<dead>, rDestHi, rDesiredHi
// bne .Ldone
unsigned LDREXD = IsThumb ? ARM::t2LDREXD : ARM::LDREXD;
MachineInstrBuilder MIB;
@@ -959,17 +943,17 @@ bool ARMExpandPseudo::ExpandCMP_SWAP_64(
LoadCmpBB->addSuccessor(StoreBB);
// .Lstore:
- // strexd rStatus, rNewLo, rNewHi, [rAddr]
- // cmp rStatus, #0
+ // strexd rTempReg, rNewLo, rNewHi, [rAddr]
+ // cmp rTempReg, #0
// bne .Lloadcmp
unsigned STREXD = IsThumb ? ARM::t2STREXD : ARM::STREXD;
- MIB = BuildMI(StoreBB, DL, TII->get(STREXD), StatusReg);
+ MIB = BuildMI(StoreBB, DL, TII->get(STREXD), TempReg);
addExclusiveRegPair(MIB, New, 0, IsThumb, TRI);
MIB.addReg(AddrReg).add(predOps(ARMCC::AL));
unsigned CMPri = IsThumb ? ARM::t2CMPri : ARM::CMPri;
BuildMI(StoreBB, DL, TII->get(CMPri))
- .addReg(StatusReg, getKillRegState(StatusDead))
+ .addReg(TempReg, RegState::Kill)
.addImm(0)
.add(predOps(ARMCC::AL));
BuildMI(StoreBB, DL, TII->get(Bcc))
Modified: llvm/branches/release_50/lib/Target/ARM/ARMInstrInfo.td
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_50/lib/Target/ARM/ARMInstrInfo.td?rev=310628&r1=310627&r2=310628&view=diff
==============================================================================
--- llvm/branches/release_50/lib/Target/ARM/ARMInstrInfo.td (original)
+++ llvm/branches/release_50/lib/Target/ARM/ARMInstrInfo.td Thu Aug 10 10:12:27 2017
@@ -6053,21 +6053,21 @@ def SPACE : PseudoInst<(outs GPR:$Rd), (
// significantly more naive than the standard expansion: we conservatively
// assume seq_cst, strong cmpxchg and omit clrex on failure.
-let Constraints = "@earlyclobber $Rd, at earlyclobber $status",
+let Constraints = "@earlyclobber $Rd, at earlyclobber $temp",
mayLoad = 1, mayStore = 1 in {
-def CMP_SWAP_8 : PseudoInst<(outs GPR:$Rd, GPR:$status),
+def CMP_SWAP_8 : PseudoInst<(outs GPR:$Rd, GPR:$temp),
(ins GPR:$addr, GPR:$desired, GPR:$new),
NoItinerary, []>, Sched<[]>;
-def CMP_SWAP_16 : PseudoInst<(outs GPR:$Rd, GPR:$status),
+def CMP_SWAP_16 : PseudoInst<(outs GPR:$Rd, GPR:$temp),
(ins GPR:$addr, GPR:$desired, GPR:$new),
NoItinerary, []>, Sched<[]>;
-def CMP_SWAP_32 : PseudoInst<(outs GPR:$Rd, GPR:$status),
+def CMP_SWAP_32 : PseudoInst<(outs GPR:$Rd, GPR:$temp),
(ins GPR:$addr, GPR:$desired, GPR:$new),
NoItinerary, []>, Sched<[]>;
-def CMP_SWAP_64 : PseudoInst<(outs GPRPair:$Rd, GPR:$status),
+def CMP_SWAP_64 : PseudoInst<(outs GPRPair:$Rd, GPR:$temp),
(ins GPR:$addr, GPRPair:$desired, GPRPair:$new),
NoItinerary, []>, Sched<[]>;
}
Modified: llvm/branches/release_50/test/CodeGen/ARM/cmpxchg-O0.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_50/test/CodeGen/ARM/cmpxchg-O0.ll?rev=310628&r1=310627&r2=310628&view=diff
==============================================================================
--- llvm/branches/release_50/test/CodeGen/ARM/cmpxchg-O0.ll (original)
+++ llvm/branches/release_50/test/CodeGen/ARM/cmpxchg-O0.ll Thu Aug 10 10:12:27 2017
@@ -10,11 +10,10 @@ 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]], r2, [r0]
+; CHECK: strexb [[STATUS:r[0-9]+]], r2, [r0]
; CHECK: cmp{{(\.w)?}} [[STATUS]], #0
; CHECK: bne [[RETRY]]
; CHECK: [[DONE]]:
@@ -30,11 +29,10 @@ 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]], r2, [r0]
+; CHECK: strexh [[STATUS:r[0-9]+]], r2, [r0]
; CHECK: cmp{{(\.w)?}} [[STATUS]], #0
; CHECK: bne [[RETRY]]
; CHECK: [[DONE]]:
@@ -50,11 +48,10 @@ 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]], r2, [r0]
+; CHECK: strex [[STATUS:r[0-9]+]], r2, [r0]
; CHECK: cmp{{(\.w)?}} [[STATUS]], #0
; CHECK: bne [[RETRY]]
; CHECK: [[DONE]]:
More information about the llvm-branch-commits
mailing list