Skip to content

Commit

Permalink
SystemZ: Don't promote atomic store in IR (#90899)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
arsenm committed May 3, 2024
1 parent 6535e7a commit edbe6eb
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 19 deletions.
1 change: 0 additions & 1 deletion llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
46 changes: 35 additions & 11 deletions llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
23 changes: 16 additions & 7 deletions llvm/test/CodeGen/SystemZ/atomic-store-08.ll
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -8,14 +7,24 @@
; RUN: llc < %s -mtriple=s390x-linux-gnu -mattr=+soft-float | FileCheck -check-prefixes=SOFTFP %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:
Expand Down

0 comments on commit edbe6eb

Please sign in to comment.