[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