SystemZ: Don't promote atomic store in IR #90899
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is the mirror to the recent atomic load change. The same bitcast-back-to-integer case is a small code quality regression for the same reason. This would disappear with a bitcastable legal 128-bit type.
|
@llvm/pr-subscribers-llvm-selectiondag @llvm/pr-subscribers-backend-systemz Author: Matt Arsenault (arsenm) ChangesThis is the mirror to the recent atomic load change. The same bitcast-back-to-integer case is a small code quality regression for the same reason. This would disappear with a bitcastable legal 128-bit type. Full diff: https://github.com/llvm/llvm-project/pull/90899.diff 3 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
index c04ca5ddc409c0..8bd4839c17a628 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
@@ -1197,7 +1197,6 @@ SDValue DAGTypeLegalizer::SoftenFloatOp_STORE(SDNode *N, unsigned OpNo) {
}
SDValue DAGTypeLegalizer::SoftenFloatOp_ATOMIC_STORE(SDNode *N, unsigned OpNo) {
- assert(ISD::isUNINDEXEDStore(N) && "Indexed store during type legalization!");
assert(OpNo == 1 && "Can only soften the stored value!");
AtomicSDNode *ST = cast<AtomicSDNode>(N);
SDValue Val = ST->getVal();
diff --git a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
index 75073accc8a548..5612e4ced4f607 100644
--- a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
@@ -295,6 +295,7 @@ SystemZTargetLowering::SystemZTargetLowering(const TargetMachine &TM,
setOperationAction(ISD::ATOMIC_LOAD, MVT::i128, Custom);
setOperationAction(ISD::ATOMIC_STORE, MVT::i128, Custom);
setOperationAction(ISD::ATOMIC_LOAD, MVT::f128, Custom);
+ setOperationAction(ISD::ATOMIC_STORE, MVT::f128, Custom);
// Mark sign/zero extending atomic loads as legal, which will make
// DAGCombiner fold extensions into atomic loads if possible.
@@ -941,9 +942,6 @@ SystemZTargetLowering::shouldCastAtomicLoadInIR(LoadInst *LI) const {
TargetLowering::AtomicExpansionKind
SystemZTargetLowering::shouldCastAtomicStoreInIR(StoreInst *SI) const {
- // Lower fp128 the same way as i128.
- if (SI->getValueOperand()->getType()->isFP128Ty())
- return AtomicExpansionKind::CastToInteger;
return AtomicExpansionKind::None;
}
@@ -6269,6 +6267,26 @@ static SDValue expandBitCastI128ToF128(SelectionDAG &DAG, SDValue Src,
return SDValue(Pair, 0);
}
+static std::pair<SDValue, SDValue>
+expandBitCastF128ToI128Parts(SelectionDAG &DAG, SDValue Src, const SDLoc &SL) {
+ SDValue LoFP =
+ DAG.getTargetExtractSubreg(SystemZ::subreg_l64, SL, MVT::f64, Src);
+ SDValue HiFP =
+ DAG.getTargetExtractSubreg(SystemZ::subreg_h64, SL, MVT::f64, Src);
+ SDValue Lo = DAG.getNode(ISD::BITCAST, SL, MVT::i64, LoFP);
+ SDValue Hi = DAG.getNode(ISD::BITCAST, SL, MVT::i64, HiFP);
+
+ return {Hi, Lo};
+}
+
+static SDValue expandBitCastF128ToI128(SelectionDAG &DAG, SDValue Src,
+ const SDLoc &SL) {
+
+ auto [Hi, Lo] = expandBitCastF128ToI128Parts(DAG, Src, SL);
+ SDNode *Pair = DAG.getMachineNode(SystemZ::PAIR128, SL, MVT::Untyped, Hi, Lo);
+ return SDValue(Pair, 0);
+}
+
// Lower operations with invalid operand or result types (currently used
// only for 128-bit integer types).
void
@@ -6307,8 +6325,17 @@ SystemZTargetLowering::LowerOperationWrapper(SDNode *N,
case ISD::ATOMIC_STORE: {
SDLoc DL(N);
SDVTList Tys = DAG.getVTList(MVT::Other);
- SDValue Ops[] = {N->getOperand(0), lowerI128ToGR128(DAG, N->getOperand(1)),
- N->getOperand(2)};
+ SDValue Val = N->getOperand(1);
+ EVT VT = Val.getValueType();
+
+ if (VT == MVT::i128 || isTypeLegal(MVT::i128)) {
+ Val = DAG.getBitcast(MVT::i128, Val);
+ Val = lowerI128ToGR128(DAG, Val);
+ } else {
+ Val = expandBitCastF128ToI128(DAG, Val, DL);
+ }
+
+ SDValue Ops[] = {N->getOperand(0), Val, N->getOperand(2)};
MachineMemOperand *MMO = cast<AtomicSDNode>(N)->getMemOperand();
SDValue Res = DAG.getMemIntrinsicNode(SystemZISD::ATOMIC_STORE_128,
DL, Tys, Ops, MVT::i128, MMO);
@@ -6351,15 +6378,12 @@ SystemZTargetLowering::LowerOperationWrapper(SDNode *N,
Hi = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL, MVT::i64, VecBC,
DAG.getConstant(0, DL, MVT::i32));
} else {
+ // FIXME: Assert should be moved into expandBitCastF128ToI128Parts
assert(getRepRegClassFor(MVT::f128) == &SystemZ::FP128BitRegClass &&
"Unrecognized register class for f128.");
- SDValue LoFP = DAG.getTargetExtractSubreg(SystemZ::subreg_l64,
- DL, MVT::f64, Src);
- SDValue HiFP = DAG.getTargetExtractSubreg(SystemZ::subreg_h64,
- DL, MVT::f64, Src);
- Lo = DAG.getNode(ISD::BITCAST, DL, MVT::i64, LoFP);
- Hi = DAG.getNode(ISD::BITCAST, DL, MVT::i64, HiFP);
+ std::tie(Hi, Lo) = expandBitCastF128ToI128Parts(DAG, Src, DL);
}
+
Results.push_back(DAG.getNode(ISD::BUILD_PAIR, DL, MVT::i128, Lo, Hi));
}
break;
diff --git a/llvm/test/CodeGen/SystemZ/atomic-store-08.ll b/llvm/test/CodeGen/SystemZ/atomic-store-08.ll
index 545ee120e01c50..4b330aa1f81af6 100644
--- a/llvm/test/CodeGen/SystemZ/atomic-store-08.ll
+++ b/llvm/test/CodeGen/SystemZ/atomic-store-08.ll
@@ -1,5 +1,4 @@
-; Test long double atomic stores. The atomic store is converted to i128 by
-; the AtomicExpand pass.
+; Test long double atomic stores. The atomic store is converted to i128
;
; RUN: llc < %s -mtriple=s390x-linux-gnu | FileCheck -check-prefixes=CHECK,BASE %s
; RUN: llc < %s -mtriple=s390x-linux-gnu -mcpu=z13 | FileCheck -check-prefixes=CHECK,Z13 %s
@@ -7,15 +6,28 @@
; TODO: Is it worth testing softfp with vector?
; RUN: llc < %s -mtriple=s390x-linux-gnu -mattr=+soft-float | FileCheck -check-prefixes=SOFTFP %s
+; TODO: Test soft float
+; RUN: llc < %s -mtriple=s390x-linux-gnu | FileCheck -check-prefixes=CHECK,BASE %s
+; RUN: llc < %s -mtriple=s390x-linux-gnu -mcpu=z13 | FileCheck -check-prefixes=CHECK,Z13 %s
+; FIXME: With legal 128-bit operation to bitcast, the base code would
+; be the same as z13
define void @f1(ptr %dst, ptr %src) {
; CHECK-LABEL: f1:
; CHECK: # %bb.0:
-; CHECK-NEXT: lg %r1, 8(%r3)
-; CHECK-NEXT: lg %r0, 0(%r3)
-; CHECK-NEXT: stpq %r0, 0(%r2)
-; CHECK-NEXT: bcr 1{{[45]}}, %r0
-; CHECK-NEXT: br %r14
+; Z13-NEXT: lg %r1, 8(%r3)
+; Z13-NEXT: lg %r0, 0(%r3)
+; Z13-NEXT: stpq %r0, 0(%r2)
+; Z13-NEXT: bcr 1{{[45]}}, %r0
+; Z13-NEXT: br %r14
+
+; BASE-NEXT: ld %f0, 0(%r3)
+; BASE-NEXT: ld %f2, 8(%r3)
+; BASE-NEXT: lgdr %r1, %f2
+; BASE-NEXT: lgdr %r0, %f0
+; BASE-NEXT: stpq %r0, 0(%r2)
+; BASE-NEXT: bcr 15, %r0
+; BASE-NEXT: br %r14
; SOFTFP-LABEL: f1:
; SOFTFP: # %bb.0:
|
arsenm
commented
May 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the mirror to the recent atomic load change. The same bitcast-back-to-integer case is a small code quality regression for the same reason. This would disappear with a bitcastable legal 128-bit type.