Skip to content

Commit 616289e

Browse files
jrtc27jrtc27
committed
[LegalizeTypes][RISCV] Correctly sign-extend comparison for ATOMIC_CMP_XCHG
Summary: Currently, the comparison argument used for ATOMIC_CMP_XCHG is legalised with GetPromotedInteger, which leaves the upper bits of the value undefind. Since this is used for comparing in an LR/SC loop with a full-width comparison, we must sign extend it. We introduce a new getExtendForAtomicCmpSwapArg to complement getExtendForAtomicOps, since many targets have compare-and-swap instructions (or pseudos) that correctly handle an any-extend input, and the existing function determines the extension of the result, whereas we are concerned with the input. This is related to https://reviews.llvm.org/D58829, which solved the issue for ATOMIC_CMP_SWAP_WITH_SUCCESS, but not the simpler ATOMIC_CMP_SWAP. Reviewers: asb, lenary, efriedma Reviewed By: asb Subscribers: arichardson, hiraditya, rbar, johnrusso, simoncook, sabuasal, niosHD, kito-cheng, shiva0217, MaskRay, zzheng, edward-jones, rogfer01, MartinMosbeck, brucehoult, the_o, rkruppe, jfb, PkmX, jocewei, psnobl, benna, Jim, s.egerton, pzheng, sameer.abuasal, apazos, luismarques, evandro, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D74453
1 parent e3033c0 commit 616289e

File tree

4 files changed

+43
-1
lines changed

4 files changed

+43
-1
lines changed

‎llvm/include/llvm/CodeGen/TargetLowering.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,6 +1962,18 @@ class TargetLoweringBase {
19621962
return ISD::ZERO_EXTEND;
19631963
}
19641964

1965+
/// Returns how the platform's atomic compare and swap expects its comparison
1966+
/// value to be extended (ZERO_EXTEND, SIGN_EXTEND, or ANY_EXTEND). This is
1967+
/// separate from getExtendForAtomicOps, which is concerned with the
1968+
/// sign-extension of the instruction's output, whereas here we are concerned
1969+
/// with the sign-extension of the input. For targets with compare-and-swap
1970+
/// instructions (or sub-word comparisons in their LL/SC loop expansions),
1971+
/// the input can be ANY_EXTEND, but the output will still have a specific
1972+
/// extension.
1973+
virtual ISD::NodeType getExtendForAtomicCmpSwapArg() const {
1974+
return ISD::ANY_EXTEND;
1975+
}
1976+
19651977
/// @}
19661978

19671979
/// Returns true if we should normalize

‎llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,8 +278,24 @@ SDValue DAGTypeLegalizer::PromoteIntRes_AtomicCmpSwap(AtomicSDNode *N,
278278
return Res.getValue(1);
279279
}
280280

281-
SDValue Op2 = GetPromotedInteger(N->getOperand(2));
281+
// Op2 is used for the comparison and thus must be extended according to the
282+
// target's atomic operations. Op3 is merely stored and so can be left alone.
283+
SDValue Op2 = N->getOperand(2);
282284
SDValue Op3 = GetPromotedInteger(N->getOperand(3));
285+
switch (TLI.getExtendForAtomicCmpSwapArg()) {
286+
case ISD::SIGN_EXTEND:
287+
Op2 = SExtPromotedInteger(Op2);
288+
break;
289+
case ISD::ZERO_EXTEND:
290+
Op2 = ZExtPromotedInteger(Op2);
291+
break;
292+
case ISD::ANY_EXTEND:
293+
Op2 = GetPromotedInteger(Op2);
294+
break;
295+
default:
296+
llvm_unreachable("Invalid atomic op extension");
297+
}
298+
283299
SDVTList VTs =
284300
DAG.getVTList(Op2.getValueType(), N->getValueType(1), MVT::Other);
285301
SDValue Res = DAG.getAtomicCmpSwap(

‎llvm/lib/Target/RISCV/RISCVISelLowering.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ class RISCVTargetLowering : public TargetLowering {
129129
return ISD::SIGN_EXTEND;
130130
}
131131

132+
ISD::NodeType getExtendForAtomicCmpSwapArg() const override {
133+
return ISD::SIGN_EXTEND;
134+
}
135+
132136
bool shouldExpandShift(SelectionDAG &DAG, SDNode *N) const override {
133137
if (DAG.getMachineFunction().getFunction().hasMinSize())
134138
return false;

‎llvm/test/CodeGen/RISCV/atomic-cmpxchg.ll

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1628,6 +1628,7 @@ define void @cmpxchg_i32_monotonic_monotonic(i32* %ptr, i32 %cmp, i32 %val) noun
16281628
;
16291629
; RV64IA-LABEL: cmpxchg_i32_monotonic_monotonic:
16301630
; RV64IA: # %bb.0:
1631+
; RV64IA-NEXT: sext.w a1, a1
16311632
; RV64IA-NEXT: .LBB20_1: # =>This Inner Loop Header: Depth=1
16321633
; RV64IA-NEXT: lr.w a3, (a0)
16331634
; RV64IA-NEXT: bne a3, a1, .LBB20_3
@@ -1680,6 +1681,7 @@ define void @cmpxchg_i32_acquire_monotonic(i32* %ptr, i32 %cmp, i32 %val) nounwi
16801681
;
16811682
; RV64IA-LABEL: cmpxchg_i32_acquire_monotonic:
16821683
; RV64IA: # %bb.0:
1684+
; RV64IA-NEXT: sext.w a1, a1
16831685
; RV64IA-NEXT: .LBB21_1: # =>This Inner Loop Header: Depth=1
16841686
; RV64IA-NEXT: lr.w.aq a3, (a0)
16851687
; RV64IA-NEXT: bne a3, a1, .LBB21_3
@@ -1732,6 +1734,7 @@ define void @cmpxchg_i32_acquire_acquire(i32* %ptr, i32 %cmp, i32 %val) nounwind
17321734
;
17331735
; RV64IA-LABEL: cmpxchg_i32_acquire_acquire:
17341736
; RV64IA: # %bb.0:
1737+
; RV64IA-NEXT: sext.w a1, a1
17351738
; RV64IA-NEXT: .LBB22_1: # =>This Inner Loop Header: Depth=1
17361739
; RV64IA-NEXT: lr.w.aq a3, (a0)
17371740
; RV64IA-NEXT: bne a3, a1, .LBB22_3
@@ -1784,6 +1787,7 @@ define void @cmpxchg_i32_release_monotonic(i32* %ptr, i32 %cmp, i32 %val) nounwi
17841787
;
17851788
; RV64IA-LABEL: cmpxchg_i32_release_monotonic:
17861789
; RV64IA: # %bb.0:
1790+
; RV64IA-NEXT: sext.w a1, a1
17871791
; RV64IA-NEXT: .LBB23_1: # =>This Inner Loop Header: Depth=1
17881792
; RV64IA-NEXT: lr.w a3, (a0)
17891793
; RV64IA-NEXT: bne a3, a1, .LBB23_3
@@ -1836,6 +1840,7 @@ define void @cmpxchg_i32_release_acquire(i32* %ptr, i32 %cmp, i32 %val) nounwind
18361840
;
18371841
; RV64IA-LABEL: cmpxchg_i32_release_acquire:
18381842
; RV64IA: # %bb.0:
1843+
; RV64IA-NEXT: sext.w a1, a1
18391844
; RV64IA-NEXT: .LBB24_1: # =>This Inner Loop Header: Depth=1
18401845
; RV64IA-NEXT: lr.w a3, (a0)
18411846
; RV64IA-NEXT: bne a3, a1, .LBB24_3
@@ -1888,6 +1893,7 @@ define void @cmpxchg_i32_acq_rel_monotonic(i32* %ptr, i32 %cmp, i32 %val) nounwi
18881893
;
18891894
; RV64IA-LABEL: cmpxchg_i32_acq_rel_monotonic:
18901895
; RV64IA: # %bb.0:
1896+
; RV64IA-NEXT: sext.w a1, a1
18911897
; RV64IA-NEXT: .LBB25_1: # =>This Inner Loop Header: Depth=1
18921898
; RV64IA-NEXT: lr.w.aq a3, (a0)
18931899
; RV64IA-NEXT: bne a3, a1, .LBB25_3
@@ -1940,6 +1946,7 @@ define void @cmpxchg_i32_acq_rel_acquire(i32* %ptr, i32 %cmp, i32 %val) nounwind
19401946
;
19411947
; RV64IA-LABEL: cmpxchg_i32_acq_rel_acquire:
19421948
; RV64IA: # %bb.0:
1949+
; RV64IA-NEXT: sext.w a1, a1
19431950
; RV64IA-NEXT: .LBB26_1: # =>This Inner Loop Header: Depth=1
19441951
; RV64IA-NEXT: lr.w.aq a3, (a0)
19451952
; RV64IA-NEXT: bne a3, a1, .LBB26_3
@@ -1992,6 +1999,7 @@ define void @cmpxchg_i32_seq_cst_monotonic(i32* %ptr, i32 %cmp, i32 %val) nounwi
19921999
;
19932000
; RV64IA-LABEL: cmpxchg_i32_seq_cst_monotonic:
19942001
; RV64IA: # %bb.0:
2002+
; RV64IA-NEXT: sext.w a1, a1
19952003
; RV64IA-NEXT: .LBB27_1: # =>This Inner Loop Header: Depth=1
19962004
; RV64IA-NEXT: lr.w.aqrl a3, (a0)
19972005
; RV64IA-NEXT: bne a3, a1, .LBB27_3
@@ -2044,6 +2052,7 @@ define void @cmpxchg_i32_seq_cst_acquire(i32* %ptr, i32 %cmp, i32 %val) nounwind
20442052
;
20452053
; RV64IA-LABEL: cmpxchg_i32_seq_cst_acquire:
20462054
; RV64IA: # %bb.0:
2055+
; RV64IA-NEXT: sext.w a1, a1
20472056
; RV64IA-NEXT: .LBB28_1: # =>This Inner Loop Header: Depth=1
20482057
; RV64IA-NEXT: lr.w.aqrl a3, (a0)
20492058
; RV64IA-NEXT: bne a3, a1, .LBB28_3
@@ -2096,6 +2105,7 @@ define void @cmpxchg_i32_seq_cst_seq_cst(i32* %ptr, i32 %cmp, i32 %val) nounwind
20962105
;
20972106
; RV64IA-LABEL: cmpxchg_i32_seq_cst_seq_cst:
20982107
; RV64IA: # %bb.0:
2108+
; RV64IA-NEXT: sext.w a1, a1
20992109
; RV64IA-NEXT: .LBB29_1: # =>This Inner Loop Header: Depth=1
21002110
; RV64IA-NEXT: lr.w.aqrl a3, (a0)
21012111
; RV64IA-NEXT: bne a3, a1, .LBB29_3

0 commit comments

Comments
 (0)