Skip to content
This repository was archived by the owner on Feb 5, 2019. It is now read-only.

Commit dc84a08

Browse files
zmodemzmodem
committed
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 ------------------------------------------------------------------------ git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_50@310628 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent aab7f85 commit dc84a08

File tree

3 files changed

+19
-38
lines changed

3 files changed

+19
-38
lines changed

‎lib/Target/ARM/ARMExpandPseudoInsts.cpp

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -769,8 +769,7 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(MachineBasicBlock &MBB,
769769
MachineInstr &MI = *MBBI;
770770
DebugLoc DL = MI.getDebugLoc();
771771
const MachineOperand &Dest = MI.getOperand(0);
772-
unsigned StatusReg = MI.getOperand(1).getReg();
773-
bool StatusDead = MI.getOperand(1).isDead();
772+
unsigned TempReg = MI.getOperand(1).getReg();
774773
// Duplicating undef operands into 2 instructions does not guarantee the same
775774
// value on both; However undef should be replaced by xzr anyway.
776775
assert(!MI.getOperand(2).isUndef() && "cannot handle undef");
@@ -797,23 +796,9 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(MachineBasicBlock &MBB,
797796
}
798797

799798
// .Lloadcmp:
800-
// mov wStatus, #0
801799
// ldrex rDest, [rAddr]
802800
// cmp rDest, rDesired
803801
// bne .Ldone
804-
if (!StatusDead) {
805-
if (IsThumb) {
806-
BuildMI(LoadCmpBB, DL, TII->get(ARM::tMOVi8), StatusReg)
807-
.addDef(ARM::CPSR, RegState::Dead)
808-
.addImm(0)
809-
.add(predOps(ARMCC::AL));
810-
} else {
811-
BuildMI(LoadCmpBB, DL, TII->get(ARM::MOVi), StatusReg)
812-
.addImm(0)
813-
.add(predOps(ARMCC::AL))
814-
.add(condCodeOp());
815-
}
816-
}
817802

818803
MachineInstrBuilder MIB;
819804
MIB = BuildMI(LoadCmpBB, DL, TII->get(LdrexOp), Dest.getReg());
@@ -836,10 +821,10 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(MachineBasicBlock &MBB,
836821
LoadCmpBB->addSuccessor(StoreBB);
837822

838823
// .Lstore:
839-
// strex rStatus, rNew, [rAddr]
840-
// cmp rStatus, #0
824+
// strex rTempReg, rNew, [rAddr]
825+
// cmp rTempReg, #0
841826
// bne .Lloadcmp
842-
MIB = BuildMI(StoreBB, DL, TII->get(StrexOp), StatusReg)
827+
MIB = BuildMI(StoreBB, DL, TII->get(StrexOp), TempReg)
843828
.addReg(NewReg)
844829
.addReg(AddrReg);
845830
if (StrexOp == ARM::t2STREX)
@@ -848,7 +833,7 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(MachineBasicBlock &MBB,
848833

849834
unsigned CMPri = IsThumb ? ARM::t2CMPri : ARM::CMPri;
850835
BuildMI(StoreBB, DL, TII->get(CMPri))
851-
.addReg(StatusReg, getKillRegState(StatusDead))
836+
.addReg(TempReg, RegState::Kill)
852837
.addImm(0)
853838
.add(predOps(ARMCC::AL));
854839
BuildMI(StoreBB, DL, TII->get(Bcc))
@@ -904,8 +889,7 @@ bool ARMExpandPseudo::ExpandCMP_SWAP_64(MachineBasicBlock &MBB,
904889
MachineInstr &MI = *MBBI;
905890
DebugLoc DL = MI.getDebugLoc();
906891
MachineOperand &Dest = MI.getOperand(0);
907-
unsigned StatusReg = MI.getOperand(1).getReg();
908-
bool StatusDead = MI.getOperand(1).isDead();
892+
unsigned TempReg = MI.getOperand(1).getReg();
909893
// Duplicating undef operands into 2 instructions does not guarantee the same
910894
// value on both; However undef should be replaced by xzr anyway.
911895
assert(!MI.getOperand(2).isUndef() && "cannot handle undef");
@@ -931,7 +915,7 @@ bool ARMExpandPseudo::ExpandCMP_SWAP_64(MachineBasicBlock &MBB,
931915
// .Lloadcmp:
932916
// ldrexd rDestLo, rDestHi, [rAddr]
933917
// cmp rDestLo, rDesiredLo
934-
// sbcs rStatus<dead>, rDestHi, rDesiredHi
918+
// sbcs rTempReg<dead>, rDestHi, rDesiredHi
935919
// bne .Ldone
936920
unsigned LDREXD = IsThumb ? ARM::t2LDREXD : ARM::LDREXD;
937921
MachineInstrBuilder MIB;
@@ -959,17 +943,17 @@ bool ARMExpandPseudo::ExpandCMP_SWAP_64(MachineBasicBlock &MBB,
959943
LoadCmpBB->addSuccessor(StoreBB);
960944

961945
// .Lstore:
962-
// strexd rStatus, rNewLo, rNewHi, [rAddr]
963-
// cmp rStatus, #0
946+
// strexd rTempReg, rNewLo, rNewHi, [rAddr]
947+
// cmp rTempReg, #0
964948
// bne .Lloadcmp
965949
unsigned STREXD = IsThumb ? ARM::t2STREXD : ARM::STREXD;
966-
MIB = BuildMI(StoreBB, DL, TII->get(STREXD), StatusReg);
950+
MIB = BuildMI(StoreBB, DL, TII->get(STREXD), TempReg);
967951
addExclusiveRegPair(MIB, New, 0, IsThumb, TRI);
968952
MIB.addReg(AddrReg).add(predOps(ARMCC::AL));
969953

970954
unsigned CMPri = IsThumb ? ARM::t2CMPri : ARM::CMPri;
971955
BuildMI(StoreBB, DL, TII->get(CMPri))
972-
.addReg(StatusReg, getKillRegState(StatusDead))
956+
.addReg(TempReg, RegState::Kill)
973957
.addImm(0)
974958
.add(predOps(ARMCC::AL));
975959
BuildMI(StoreBB, DL, TII->get(Bcc))

‎lib/Target/ARM/ARMInstrInfo.td

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6053,21 +6053,21 @@ def SPACE : PseudoInst<(outs GPR:$Rd), (ins i32imm:$size, GPR:$Rn),
60536053
// significantly more naive than the standard expansion: we conservatively
60546054
// assume seq_cst, strong cmpxchg and omit clrex on failure.
60556055

6056-
let Constraints = "@earlyclobber $Rd,@earlyclobber $status",
6056+
let Constraints = "@earlyclobber $Rd,@earlyclobber $temp",
60576057
mayLoad = 1, mayStore = 1 in {
6058-
def CMP_SWAP_8 : PseudoInst<(outs GPR:$Rd, GPR:$status),
6058+
def CMP_SWAP_8 : PseudoInst<(outs GPR:$Rd, GPR:$temp),
60596059
(ins GPR:$addr, GPR:$desired, GPR:$new),
60606060
NoItinerary, []>, Sched<[]>;
60616061

6062-
def CMP_SWAP_16 : PseudoInst<(outs GPR:$Rd, GPR:$status),
6062+
def CMP_SWAP_16 : PseudoInst<(outs GPR:$Rd, GPR:$temp),
60636063
(ins GPR:$addr, GPR:$desired, GPR:$new),
60646064
NoItinerary, []>, Sched<[]>;
60656065

6066-
def CMP_SWAP_32 : PseudoInst<(outs GPR:$Rd, GPR:$status),
6066+
def CMP_SWAP_32 : PseudoInst<(outs GPR:$Rd, GPR:$temp),
60676067
(ins GPR:$addr, GPR:$desired, GPR:$new),
60686068
NoItinerary, []>, Sched<[]>;
60696069

6070-
def CMP_SWAP_64 : PseudoInst<(outs GPRPair:$Rd, GPR:$status),
6070+
def CMP_SWAP_64 : PseudoInst<(outs GPRPair:$Rd, GPR:$temp),
60716071
(ins GPR:$addr, GPRPair:$desired, GPRPair:$new),
60726072
NoItinerary, []>, Sched<[]>;
60736073
}

‎test/CodeGen/ARM/cmpxchg-O0.ll

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,10 @@ define { i8, i1 } @test_cmpxchg_8(i8* %addr, i8 %desired, i8 %new) nounwind {
1010
; CHECK: dmb ish
1111
; CHECK: uxtb [[DESIRED:r[0-9]+]], [[DESIRED]]
1212
; CHECK: [[RETRY:.LBB[0-9]+_[0-9]+]]:
13-
; CHECK: mov{{s?}} [[STATUS:r[0-9]+]], #0
1413
; CHECK: ldrexb [[OLD:r[0-9]+]], [r0]
1514
; CHECK: cmp [[OLD]], [[DESIRED]]
1615
; CHECK: bne [[DONE:.LBB[0-9]+_[0-9]+]]
17-
; CHECK: strexb [[STATUS]], r2, [r0]
16+
; CHECK: strexb [[STATUS:r[0-9]+]], r2, [r0]
1817
; CHECK: cmp{{(\.w)?}} [[STATUS]], #0
1918
; CHECK: bne [[RETRY]]
2019
; CHECK: [[DONE]]:
@@ -30,11 +29,10 @@ define { i16, i1 } @test_cmpxchg_16(i16* %addr, i16 %desired, i16 %new) nounwind
3029
; CHECK: dmb ish
3130
; CHECK: uxth [[DESIRED:r[0-9]+]], [[DESIRED]]
3231
; CHECK: [[RETRY:.LBB[0-9]+_[0-9]+]]:
33-
; CHECK: mov{{s?}} [[STATUS:r[0-9]+]], #0
3432
; CHECK: ldrexh [[OLD:r[0-9]+]], [r0]
3533
; CHECK: cmp [[OLD]], [[DESIRED]]
3634
; CHECK: bne [[DONE:.LBB[0-9]+_[0-9]+]]
37-
; CHECK: strexh [[STATUS]], r2, [r0]
35+
; CHECK: strexh [[STATUS:r[0-9]+]], r2, [r0]
3836
; CHECK: cmp{{(\.w)?}} [[STATUS]], #0
3937
; CHECK: bne [[RETRY]]
4038
; CHECK: [[DONE]]:
@@ -50,11 +48,10 @@ define { i32, i1 } @test_cmpxchg_32(i32* %addr, i32 %desired, i32 %new) nounwind
5048
; CHECK: dmb ish
5149
; CHECK-NOT: uxt
5250
; CHECK: [[RETRY:.LBB[0-9]+_[0-9]+]]:
53-
; CHECK: mov{{s?}} [[STATUS:r[0-9]+]], #0
5451
; CHECK: ldrex [[OLD:r[0-9]+]], [r0]
5552
; CHECK: cmp [[OLD]], [[DESIRED]]
5653
; CHECK: bne [[DONE:.LBB[0-9]+_[0-9]+]]
57-
; CHECK: strex [[STATUS]], r2, [r0]
54+
; CHECK: strex [[STATUS:r[0-9]+]], r2, [r0]
5855
; CHECK: cmp{{(\.w)?}} [[STATUS]], #0
5956
; CHECK: bne [[RETRY]]
6057
; CHECK: [[DONE]]:

0 commit comments

Comments
 (0)