Commit f029bee7 authored by Tom Stellard's avatar Tom Stellard
Browse files

Merging r288418: 

------------------------------------------------------------------------
r288418 | tnorthover | 2016-12-01 13:31:59 -0800 (Thu, 01 Dec 2016) | 13 lines

AArch64: fix 128-bit cmpxchg at -O0 (again, again).

This time the issue is fortunately just a simple mistake rather than a horrible
design spectre. I thought SUBS/SBCS provided sufficient NZCV flags for
comparing two 64-bit values, but they don't.

The fix is slightly clunkier in AArch64 because we can't use conditional
execution to emit a pair of CMPs. Traditionally an "icmp ne i128" would map to
an EOR/EOR/ORR/CBNZ, but that uses more registers so it's easier to go with a
CSET/CINC/CBNZ combination. Slightly less efficient, but this is -O0 anyway.

Thanks to Anton Korobeynikov for pointing out the issue.

------------------------------------------------------------------------

llvm-svn: 288846
parent feb79af8

llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp

+14 −6
Original line number Diff line number Diff line
@@ -718,13 +718,21 @@ bool AArch64ExpandPseudo::expandCMP_SWAP_128( 
      .addReg(DestLo.getReg(), getKillRegState(DestLo.isDead()))

      .addOperand(DesiredLo)

      .addImm(0);

  BuildMI(LoadCmpBB, DL, TII->get(AArch64::SBCSXr), AArch64::XZR)

  BuildMI(LoadCmpBB, DL, TII->get(AArch64::CSINCWr), StatusReg)

    .addReg(AArch64::WZR)

    .addReg(AArch64::WZR)

    .addImm(AArch64CC::EQ);

  BuildMI(LoadCmpBB, DL, TII->get(AArch64::SUBSXrs), AArch64::XZR)

      .addReg(DestHi.getReg(), getKillRegState(DestHi.isDead()))

      .addOperand(DesiredHi);

  BuildMI(LoadCmpBB, DL, TII->get(AArch64::Bcc))

      .addImm(AArch64CC::NE)

      .addMBB(DoneBB)

      .addReg(AArch64::NZCV, RegState::Implicit | RegState::Kill);

      .addOperand(DesiredHi)

      .addImm(0);

  BuildMI(LoadCmpBB, DL, TII->get(AArch64::CSINCWr), StatusReg)

      .addReg(StatusReg, RegState::Kill)

      .addReg(StatusReg, RegState::Kill)

      .addImm(AArch64CC::EQ);

  BuildMI(LoadCmpBB, DL, TII->get(AArch64::CBNZW))

      .addReg(StatusReg, RegState::Kill)

      .addMBB(DoneBB);

  LoadCmpBB->addSuccessor(DoneBB);

  LoadCmpBB->addSuccessor(StoreBB);

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

+8 −4
Original line number Diff line number Diff line
@@ -65,8 +65,10 @@ define { i128, i1 } @test_cmpxchg_128(i128* %addr, i128 %desired, i128 %new) nou 
; CHECK: [[RETRY:.LBB[0-9]+_[0-9]+]]:

; CHECK:     ldaxp [[OLD_LO:x[0-9]+]], [[OLD_HI:x[0-9]+]], [x0]

; CHECK:     cmp [[OLD_LO]], x2

; CHECK:     sbcs xzr, [[OLD_HI]], x3

; CHECK:     b.ne [[DONE:.LBB[0-9]+_[0-9]+]]

; CHECK:     cset [[CMP_TMP:w[0-9]+]], ne

; CHECK:     cmp [[OLD_HI]], x3

; CHECK:     cinc [[CMP:w[0-9]+]], [[CMP_TMP]], ne

; CHECK:     cbnz [[CMP]], [[DONE:.LBB[0-9]+_[0-9]+]]

; CHECK:     stlxp [[STATUS:w[0-9]+]], x4, x5, [x0]

; CHECK:     cbnz [[STATUS]], [[RETRY]]

; CHECK: [[DONE]]:
@@ -88,8 +90,10 @@ define {i128, i1} @test_cmpxchg_128_unsplit(i128* %addr) { 
; CHECK: [[RETRY:.LBB[0-9]+_[0-9]+]]:

; CHECK:     ldaxp [[OLD_LO:x[0-9]+]], [[OLD_HI:x[0-9]+]], [x0]

; CHECK:     cmp [[OLD_LO]], [[DESIRED_LO]]

; CHECK:     sbcs xzr, [[OLD_HI]], [[DESIRED_HI]]

; CHECK:     b.ne [[DONE:.LBB[0-9]+_[0-9]+]]

; CHECK:     cset [[CMP_TMP:w[0-9]+]], ne

; CHECK:     cmp [[OLD_HI]], [[DESIRED_HI]]

; CHECK:     cinc [[CMP:w[0-9]+]], [[CMP_TMP]], ne

; CHECK:     cbnz [[CMP]], [[DONE:.LBB[0-9]+_[0-9]+]]

; CHECK:     stlxp [[STATUS:w[0-9]+]], [[NEW_LO]], [[NEW_HI]], [x0]

; CHECK:     cbnz [[STATUS]], [[RETRY]]

; CHECK: [[DONE]]: