[llvm] SystemZ: Don't promote atomic store in IR (PR #90899)
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Fri May 3 00:40:31 PDT 2024
https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/90899
>From ee49110f6ad3a7b580788fbd7f6519f893bb12b9 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Fri, 26 Apr 2024 23:11:56 +0200
Subject: [PATCH 1/2] SystemZ: Don't promote atomic store in IR
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.
---
.../SelectionDAG/LegalizeFloatTypes.cpp | 1 -
.../Target/SystemZ/SystemZISelLowering.cpp | 46 ++++++++++++++-----
llvm/test/CodeGen/SystemZ/atomic-store-08.ll | 26 ++++++++---
3 files changed, 54 insertions(+), 19 deletions(-)
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:
>From e7f0a067c45b8c0332e166b435cf2b32b30a9dfc Mon Sep 17 00:00:00 2001
From: Matt Arsenault <arsenm2 at gmail.com>
Date: Fri, 3 May 2024 09:40:23 +0200
Subject: [PATCH 2/2] Remove duplicated run lines
---
llvm/test/CodeGen/SystemZ/atomic-store-08.ll | 3 ---
1 file changed, 3 deletions(-)
diff --git a/llvm/test/CodeGen/SystemZ/atomic-store-08.ll b/llvm/test/CodeGen/SystemZ/atomic-store-08.ll
index 4b330aa1f81af6..422dcb63b531fe 100644
--- a/llvm/test/CodeGen/SystemZ/atomic-store-08.ll
+++ b/llvm/test/CodeGen/SystemZ/atomic-store-08.ll
@@ -6,9 +6,6 @@
; 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
More information about the llvm-commits
mailing list