[ARM] Fix for 64-bit CAS expansion on ARM32 with -O0

[iid_iunknown]

Summary

This patch fixes comparison of 64-bit atomic with its expected value in CMP_SWAP_64 expansion.

Currently, the low words are compared with CMP, while the high words are compared with SBC. SBC expects the carry flag to be set if CMP detects a difference. CMP might leave the carry unset for unequal arguments though if the first one is >= than the second. This might cause the comparison logic to detect false equality.

Example of the broken C++ code:

std::atomic<long long> at(2);

long long ll = 1;
std::atomic_compare_exchange_strong(&at, &ll, 3);

Even though the atomic at and the expected value ll are not equal and atomic_compare_exchange_strong returns false, at is changed to 3.

The patch replaces SBC with CMPEQ.

[t.p.northover]

lib/Target/ARM/ARMExpandPseudoInsts.cpp

937–938

You can probably drop the "MIB =" here. Otherwise everything looks fine though. Thanks for fixing this!

[iid_iunknown]

Addressing the review remarks.

[iid_iunknown]

lib/Target/ARM/ARMExpandPseudoInsts.cpp

937–938

Yes, you are right. I've updated the patch.

Thanks for your review, Tim!

lib/Target/ARM/ARMExpandPseudoInsts.cpp

bool ARMExpandPseudo::ExpandCMP_SWAP_64(MachineBasicBlock &MBB,
   addExclusiveRegPair(MIB, Dest, RegState::Define, IsThumb, TRI);
   MIB.addReg(Addr.getReg());
   AddDefaultPred(MIB);
 
   unsigned CMPrr = IsThumb ? ARM::tCMPhir : ARM::CMPrr;
   AddDefaultPred(BuildMI(LoadCmpBB, DL, TII->get(CMPrr))
                      .addReg(DestLo, getKillRegState(Dest.isDead()))
                      .addReg(DesiredLo, getKillRegState(Desired.isDead())));
 
-  unsigned SBCrr = IsThumb ? ARM::t2SBCrr : ARM::SBCrr;
-  MIB = BuildMI(LoadCmpBB, DL, TII->get(SBCrr))
-            .addReg(StatusReg, RegState::Define | RegState::Dead)
-            .addReg(DestHi, getKillRegState(Dest.isDead()))
-            .addReg(DesiredHi, getKillRegState(Desired.isDead()));
-  AddDefaultPred(MIB);
-  MIB.addReg(ARM::CPSR, RegState::Kill);
+  BuildMI(LoadCmpBB, DL, TII->get(CMPrr))
+      .addReg(DesiredHi, getKillRegState(Desired.isDead()))
+      .addImm(ARMCC::EQ).addReg(ARM::CPSR, RegState::Kill);
 
   unsigned Bcc = IsThumb ? ARM::tBcc : ARM::Bcc;
   BuildMI(LoadCmpBB, DL, TII->get(Bcc))
       .addMBB(DoneBB)
       .addImm(ARMCC::NE)
       .addReg(ARM::CPSR, RegState::Kill);
   LoadCmpBB->addSuccessor(DoneBB);
   LoadCmpBB->addSuccessor(StoreBB);

test/CodeGen/ARM/cmpxchg-O0.ll

 
 define { i64, i1 } @test_cmpxchg_64(i64* %addr, i64 %desired, i64 %new) nounwind {
 ; CHECK-LABEL: test_cmpxchg_64:
 ; CHECK:     dmb ish
 ; CHECK-NOT: uxt
 ; CHECK: [[RETRY:.LBB[0-9]+_[0-9]+]]:
 ; CHECK:     ldrexd [[OLDLO:r[0-9]+]], [[OLDHI:r[0-9]+]], [r0]
 ; CHECK:     cmp [[OLDLO]], r6
-; CHECK:     sbcs{{(\.w)?}} [[STATUS:r[0-9]+]], [[OLDHI]], r7
+; CHECK:     cmpeq [[OLDHI]], r7
 ; CHECK:     bne [[DONE:.LBB[0-9]+_[0-9]+]]
-; CHECK:     strexd [[STATUS]], r4, r5, [r0]
+; CHECK:     strexd [[STATUS:r[0-9]+]], r4, r5, [r0]
 ; CHECK:     cmp{{(\.w)?}} [[STATUS]], #0
 ; CHECK:     bne [[RETRY]]
 ; CHECK: [[DONE]]:
 ; CHECK:     dmb ish
   %res = cmpxchg i64* %addr, i64 %desired, i64 %new seq_cst monotonic
   ret { i64, i1 } %res
 }
 
 define { i64, i1 } @test_nontrivial_args(i64* %addr, i64 %desired, i64 %new) {
 ; CHECK-LABEL: test_nontrivial_args:
 ; CHECK:     dmb ish
 ; CHECK-NOT: uxt
 ; CHECK: [[RETRY:.LBB[0-9]+_[0-9]+]]:
 ; CHECK:     ldrexd [[OLDLO:r[0-9]+]], [[OLDHI:r[0-9]+]], [r0]
 ; CHECK:     cmp [[OLDLO]], {{r[0-9]+}}
-; CHECK:     sbcs{{(\.w)?}} [[STATUS:r[0-9]+]], [[OLDHI]], {{r[0-9]+}}
+; CHECK:     cmpeq [[OLDHI]], {{r[0-9]+}}
 ; CHECK:     bne [[DONE:.LBB[0-9]+_[0-9]+]]
-; CHECK:     strexd [[STATUS]], {{r[0-9]+}}, {{r[0-9]+}}, [r0]
+; CHECK:     strexd [[STATUS:r[0-9]+]], {{r[0-9]+}}, {{r[0-9]+}}, [r0]
 ; CHECK:     cmp{{(\.w)?}} [[STATUS]], #0
 ; CHECK:     bne [[RETRY]]
 ; CHECK: [[DONE]]:
 ; CHECK:     dmb ish
 
   %desired1 = add i64 %desired, 1
   %new1 = add i64 %new, 1
   %res = cmpxchg i64* %addr, i64 %desired1, i64 %new1 seq_cst seq_cst