From 41dbbb7c22da536b7f55da50293faa9aab51ef6d Mon Sep 17 00:00:00 2001 From: Tom Stellard Date: Tue, 6 Dec 2016 20:09:33 +0000 Subject: Merging r288433: ------------------------------------------------------------------------ r288433 | oranevskyy | 2016-12-01 14:58:35 -0800 (Thu, 01 Dec 2016) | 24 lines [ARM] Fix for 64-bit CAS expansion on ARM32 with -O0 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 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. Reviewers: t.p.northover Subscribers: aemerson, rengolin, llvm-commits, asl Differential Revision: https://reviews.llvm.org/D27315 ------------------------------------------------------------------------ llvm-svn: 288847 --- llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp | 11 ++++------- llvm/test/CodeGen/ARM/cmpxchg-O0.ll | 8 ++++---- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp b/llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp index 56f5728..a7b2996 100644 --- a/llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp +++ b/llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp @@ -932,13 +932,10 @@ bool ARMExpandPseudo::ExpandCMP_SWAP_64(MachineBasicBlock &MBB, .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(DestHi, getKillRegState(Dest.isDead())) + .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)) diff --git a/llvm/test/CodeGen/ARM/cmpxchg-O0.ll b/llvm/test/CodeGen/ARM/cmpxchg-O0.ll index ec3005d..f8ad2bb 100644 --- a/llvm/test/CodeGen/ARM/cmpxchg-O0.ll +++ b/llvm/test/CodeGen/ARM/cmpxchg-O0.ll @@ -69,9 +69,9 @@ define { i64, i1 } @test_cmpxchg_64(i64* %addr, i64 %desired, i64 %new) nounwind ; 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]]: @@ -87,9 +87,9 @@ define { i64, i1 } @test_nontrivial_args(i64* %addr, i64 %desired, i64 %new) { ; 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]]: -- cgit v1.1