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,@earlyclobber $status", +let Constraints = "@earlyclobber $Rd,@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]]: _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
