Commit 375efaaa authored by Hans Wennborg's avatar Hans Wennborg
Browse files

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
------------------------------------------------------------------------

llvm-svn: 310628
parent 9364e3bf

llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp

+11 −27
Original line number Diff line number Diff line
@@ -769,8 +769,7 @@ bool ARMExpandPseudo::ExpandCMP_SWAP(MachineBasicBlock &MBB, 
  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(MachineBasicBlock &MBB, 
  }



  // .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(MachineBasicBlock &MBB, 
  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(MachineBasicBlock &MBB, 


  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(MachineBasicBlock &MBB, 
  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(MachineBasicBlock &MBB, 
  // .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(MachineBasicBlock &MBB, 
  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))

llvm/lib/Target/ARM/ARMInstrInfo.td

+5 −5
Original line number Diff line number Diff line
@@ -6053,21 +6053,21 @@ def SPACE : PseudoInst<(outs GPR:$Rd), (ins i32imm:$size, GPR:$Rn), 
// 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<[]>;

}

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

+3 −6
Original line number Diff line number Diff line
@@ -10,11 +10,10 @@ define { i8, i1 } @test_cmpxchg_8(i8* %addr, i8 %desired, i8 %new) nounwind { 
; 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* %addr, i16 %desired, i16 %new) nounwind 
; 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* %addr, i32 %desired, i32 %new) nounwind 
; 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]]: