diff options
| author | Tim Northover <tnorthover@apple.com> | 2016-12-01 21:31:59 +0000 |
|---|---|---|
| committer | Tim Northover <tnorthover@apple.com> | 2016-12-01 21:31:59 +0000 |
| commit | 5bb87b6769aaabae141d76ddbf705f2aca566b6c (patch) | |
| tree | c1741863629f754018a9d35247cfb8c9dd4523f9 /llvm | |
| parent | 873947141b376ad1691ddc66881020de4ce91f7e (diff) | |
| download | bcm5719-llvm-5bb87b6769aaabae141d76ddbf705f2aca566b6c.tar.gz bcm5719-llvm-5bb87b6769aaabae141d76ddbf705f2aca566b6c.zip | |
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: 288418
Diffstat (limited to 'llvm')
| -rw-r--r-- | llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp | 20 | ||||
| -rw-r--r-- | llvm/test/CodeGen/AArch64/cmpxchg-O0.ll | 12 |
2 files changed, 22 insertions, 10 deletions
diff --git a/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp b/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp index a708a171cfc..94b208a8e7d 100644 --- a/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp +++ b/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp @@ -712,13 +712,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) + .addUse(AArch64::WZR) + .addUse(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) + .addUse(StatusReg, RegState::Kill) + .addUse(StatusReg, RegState::Kill) + .addImm(AArch64CC::EQ); + BuildMI(LoadCmpBB, DL, TII->get(AArch64::CBNZW)) + .addUse(StatusReg, RegState::Kill) + .addMBB(DoneBB); LoadCmpBB->addSuccessor(DoneBB); LoadCmpBB->addSuccessor(StoreBB); diff --git a/llvm/test/CodeGen/AArch64/cmpxchg-O0.ll b/llvm/test/CodeGen/AArch64/cmpxchg-O0.ll index b6918f6ec1c..8432b15ea52 100644 --- a/llvm/test/CodeGen/AArch64/cmpxchg-O0.ll +++ b/llvm/test/CodeGen/AArch64/cmpxchg-O0.ll @@ -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]]: |

