[RISCV] Split f64 loads/stores for RV32+Zdinx during isel instead of post-RA. #139840
Conversation
This file contains hidden or 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
…post-RA. This avoids a bunch of complexity around making sure the offset doesn't exceed 4091 so we can add 4 after splitting later. By splitting early, the split loads/stores will get selected independently. There's a bit of follow up work to do, particularly around splitting a constant pool load. Overall I think this is cleaner with less edge cases.
|
@llvm/pr-subscribers-backend-risc-v Author: Craig Topper (topperc) ChangesThis avoids a bunch of complexity around making sure the offset doesn't exceed 4091 so we can add 4 after splitting later. By splitting early, the split loads/stores will get selected independently. There's a bit of follow up work to do, particularly around splitting a constant pool load. Overall I think this is cleaner with less edge cases. Patch is 70.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/139840.diff 12 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
index 9db15ff25f979..811f59370e3a8 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -2531,8 +2531,7 @@ bool RISCVDAGToDAGISel::SelectAddrFrameIndex(SDValue Addr, SDValue &Base,
static bool selectConstantAddr(SelectionDAG *CurDAG, const SDLoc &DL,
const MVT VT, const RISCVSubtarget *Subtarget,
SDValue Addr, SDValue &Base, SDValue &Offset,
- bool IsPrefetch = false,
- bool IsRV32Zdinx = false) {
+ bool IsPrefetch = false) {
if (!isa<ConstantSDNode>(Addr))
return false;
@@ -2546,9 +2545,6 @@ static bool selectConstantAddr(SelectionDAG *CurDAG, const SDLoc &DL,
if (!Subtarget->is64Bit() || isInt<32>(Hi)) {
if (IsPrefetch && (Lo12 & 0b11111) != 0)
return false;
- if (IsRV32Zdinx && !isInt<12>(Lo12 + 4))
- return false;
-
if (Hi) {
int64_t Hi20 = (Hi >> 12) & 0xfffff;
Base = SDValue(
@@ -2572,8 +2568,6 @@ static bool selectConstantAddr(SelectionDAG *CurDAG, const SDLoc &DL,
Lo12 = Seq.back().getImm();
if (IsPrefetch && (Lo12 & 0b11111) != 0)
return false;
- if (IsRV32Zdinx && !isInt<12>(Lo12 + 4))
- return false;
// Drop the last instruction.
Seq.pop_back();
@@ -2665,7 +2659,7 @@ bool RISCVDAGToDAGISel::SelectAddrRegRegScale(SDValue Addr,
}
bool RISCVDAGToDAGISel::SelectAddrRegImm(SDValue Addr, SDValue &Base,
- SDValue &Offset, bool IsRV32Zdinx) {
+ SDValue &Offset) {
if (SelectAddrFrameIndex(Addr, Base, Offset))
return true;
@@ -2673,39 +2667,14 @@ bool RISCVDAGToDAGISel::SelectAddrRegImm(SDValue Addr, SDValue &Base,
MVT VT = Addr.getSimpleValueType();
if (Addr.getOpcode() == RISCVISD::ADD_LO) {
- // If this is non RV32Zdinx we can always fold.
- if (!IsRV32Zdinx) {
- Base = Addr.getOperand(0);
- Offset = Addr.getOperand(1);
- return true;
- }
-
- // For RV32Zdinx we need to have more than 4 byte alignment so we can add 4
- // to the offset when we expand in RISCVExpandPseudoInsts.
- if (auto *GA = dyn_cast<GlobalAddressSDNode>(Addr.getOperand(1))) {
- const DataLayout &DL = CurDAG->getDataLayout();
- Align Alignment = commonAlignment(
- GA->getGlobal()->getPointerAlignment(DL), GA->getOffset());
- if (Alignment > 4) {
- Base = Addr.getOperand(0);
- Offset = Addr.getOperand(1);
- return true;
- }
- }
- if (auto *CP = dyn_cast<ConstantPoolSDNode>(Addr.getOperand(1))) {
- Align Alignment = commonAlignment(CP->getAlign(), CP->getOffset());
- if (Alignment > 4) {
- Base = Addr.getOperand(0);
- Offset = Addr.getOperand(1);
- return true;
- }
- }
+ Base = Addr.getOperand(0);
+ Offset = Addr.getOperand(1);
+ return true;
}
- int64_t RV32ZdinxRange = IsRV32Zdinx ? 4 : 0;
if (CurDAG->isBaseWithConstantOffset(Addr)) {
int64_t CVal = cast<ConstantSDNode>(Addr.getOperand(1))->getSExtValue();
- if (isInt<12>(CVal) && isInt<12>(CVal + RV32ZdinxRange)) {
+ if (isInt<12>(CVal) && isInt<12>(CVal)) {
Base = Addr.getOperand(0);
if (Base.getOpcode() == RISCVISD::ADD_LO) {
SDValue LoOperand = Base.getOperand(1);
@@ -2718,8 +2687,7 @@ bool RISCVDAGToDAGISel::SelectAddrRegImm(SDValue Addr, SDValue &Base,
const DataLayout &DL = CurDAG->getDataLayout();
Align Alignment = commonAlignment(
GA->getGlobal()->getPointerAlignment(DL), GA->getOffset());
- if ((CVal == 0 || Alignment > CVal) &&
- (!IsRV32Zdinx || commonAlignment(Alignment, CVal) > 4)) {
+ if ((CVal == 0 || Alignment > CVal)) {
int64_t CombinedOffset = CVal + GA->getOffset();
Base = Base.getOperand(0);
Offset = CurDAG->getTargetGlobalAddress(
@@ -2740,13 +2708,13 @@ bool RISCVDAGToDAGISel::SelectAddrRegImm(SDValue Addr, SDValue &Base,
// Handle ADD with large immediates.
if (Addr.getOpcode() == ISD::ADD && isa<ConstantSDNode>(Addr.getOperand(1))) {
int64_t CVal = cast<ConstantSDNode>(Addr.getOperand(1))->getSExtValue();
- assert(!(isInt<12>(CVal) && isInt<12>(CVal + RV32ZdinxRange)) &&
+ assert(!(isInt<12>(CVal) && isInt<12>(CVal)) &&
"simm12 not already handled?");
// Handle immediates in the range [-4096,-2049] or [2048, 4094]. We can use
// an ADDI for part of the offset and fold the rest into the load/store.
// This mirrors the AddiPair PatFrag in RISCVInstrInfo.td.
- if (CVal >= -4096 && CVal <= (4094 - RV32ZdinxRange)) {
+ if (CVal >= -4096 && CVal <= 4094) {
int64_t Adj = CVal < 0 ? -2048 : 2047;
Base = SDValue(
CurDAG->getMachineNode(RISCV::ADDI, DL, VT, Addr.getOperand(0),
@@ -2764,7 +2732,7 @@ bool RISCVDAGToDAGISel::SelectAddrRegImm(SDValue Addr, SDValue &Base,
// instructions.
if (isWorthFoldingAdd(Addr) &&
selectConstantAddr(CurDAG, DL, VT, Subtarget, Addr.getOperand(1), Base,
- Offset, /*IsPrefetch=*/false, RV32ZdinxRange)) {
+ Offset, /*IsPrefetch=*/false)) {
// Insert an ADD instruction with the materialized Hi52 bits.
Base = SDValue(
CurDAG->getMachineNode(RISCV::ADD, DL, VT, Addr.getOperand(0), Base),
@@ -2774,7 +2742,7 @@ bool RISCVDAGToDAGISel::SelectAddrRegImm(SDValue Addr, SDValue &Base,
}
if (selectConstantAddr(CurDAG, DL, VT, Subtarget, Addr, Base, Offset,
- /*IsPrefetch=*/false, RV32ZdinxRange))
+ /*IsPrefetch=*/false))
return true;
Base = Addr;
diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h
index cd211d41f30fb..11d62e5edad3f 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h
@@ -46,11 +46,7 @@ class RISCVDAGToDAGISel : public SelectionDAGISel {
std::vector<SDValue> &OutOps) override;
bool SelectAddrFrameIndex(SDValue Addr, SDValue &Base, SDValue &Offset);
- bool SelectAddrRegImm(SDValue Addr, SDValue &Base, SDValue &Offset,
- bool IsRV32Zdinx = false);
- bool SelectAddrRegImmRV32Zdinx(SDValue Addr, SDValue &Base, SDValue &Offset) {
- return SelectAddrRegImm(Addr, Base, Offset, true);
- }
+ bool SelectAddrRegImm(SDValue Addr, SDValue &Base, SDValue &Offset);
bool SelectAddrRegImmLsb00000(SDValue Addr, SDValue &Base, SDValue &Offset);
bool SelectAddrRegRegScale(SDValue Addr, unsigned MaxShiftAmount,
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index c01496c9a7f3a..ce0cc2b43b8de 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -581,6 +581,11 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
if (!Subtarget.is64Bit())
setOperationAction(ISD::BITCAST, MVT::i64, Custom);
+ if (Subtarget.hasStdExtZdinx() && !Subtarget.is64Bit()) {
+ setOperationAction(ISD::LOAD, MVT::f64, Custom);
+ setOperationAction(ISD::STORE, MVT::f64, Custom);
+ }
+
if (Subtarget.hasStdExtZfa()) {
setOperationAction(ISD::ConstantFP, MVT::f64, Custom);
setOperationAction(FPRndMode, MVT::f64, Legal);
@@ -7705,19 +7710,42 @@ SDValue RISCVTargetLowering::LowerOperation(SDValue Op,
}
case ISD::LOAD: {
auto *Load = cast<LoadSDNode>(Op);
- EVT VecTy = Load->getMemoryVT();
+ EVT VT = Load->getValueType(0);
+ if (VT == MVT::f64) {
+ assert(Subtarget.hasStdExtZdinx() && !Subtarget.is64Bit() &&
+ "Unexpected custom legalisation");
+
+ // Replace a double precision load with two i32 loads and a BuildPairF64.
+ SDLoc DL(Op);
+ SDValue BasePtr = Load->getBasePtr();
+ SDValue Chain = Load->getChain();
+
+ SDValue Lo = DAG.getLoad(MVT::i32, DL, Chain, BasePtr,
+ Load->getPointerInfo(), Load->getOriginalAlign(),
+ Load->getMemOperand()->getFlags());
+ BasePtr = DAG.getObjectPtrOffset(DL, BasePtr, TypeSize::getFixed(4));
+ SDValue Hi = DAG.getLoad(
+ MVT::i32, DL, Chain, BasePtr, Load->getPointerInfo().getWithOffset(4),
+ Load->getOriginalAlign(), Load->getMemOperand()->getFlags());
+ Chain = DAG.getNode(ISD::TokenFactor, DL, MVT::Other, Lo.getValue(1),
+ Hi.getValue(1));
+
+ SDValue Pair = DAG.getNode(RISCVISD::BuildPairF64, DL, MVT::f64, Lo, Hi);
+ return DAG.getMergeValues({Pair, Chain}, DL);
+ }
+
// Handle normal vector tuple load.
- if (VecTy.isRISCVVectorTuple()) {
+ if (VT.isRISCVVectorTuple()) {
SDLoc DL(Op);
MVT XLenVT = Subtarget.getXLenVT();
- unsigned NF = VecTy.getRISCVVectorTupleNumFields();
- unsigned Sz = VecTy.getSizeInBits().getKnownMinValue();
+ unsigned NF = VT.getRISCVVectorTupleNumFields();
+ unsigned Sz = VT.getSizeInBits().getKnownMinValue();
unsigned NumElts = Sz / (NF * 8);
int Log2LMUL = Log2_64(NumElts) - 3;
auto Flag = SDNodeFlags();
Flag.setNoUnsignedWrap(true);
- SDValue Ret = DAG.getUNDEF(VecTy);
+ SDValue Ret = DAG.getUNDEF(VT);
SDValue BasePtr = Load->getBasePtr();
SDValue VROffset = DAG.getNode(RISCVISD::READ_VLENB, DL, XLenVT);
VROffset =
@@ -7731,7 +7759,7 @@ SDValue RISCVTargetLowering::LowerOperation(SDValue Op,
MVT::getScalableVectorVT(MVT::i8, NumElts), DL, Load->getChain(),
BasePtr, MachinePointerInfo(Load->getAddressSpace()), Align(8));
OutChains.push_back(LoadVal.getValue(1));
- Ret = DAG.getNode(RISCVISD::TUPLE_INSERT, DL, VecTy, Ret, LoadVal,
+ Ret = DAG.getNode(RISCVISD::TUPLE_INSERT, DL, VT, Ret, LoadVal,
DAG.getVectorIdxConstant(i, DL));
BasePtr = DAG.getNode(ISD::ADD, DL, XLenVT, BasePtr, VROffset, Flag);
}
@@ -7748,13 +7776,34 @@ SDValue RISCVTargetLowering::LowerOperation(SDValue Op,
case ISD::STORE: {
auto *Store = cast<StoreSDNode>(Op);
SDValue StoredVal = Store->getValue();
- EVT VecTy = StoredVal.getValueType();
+ EVT VT = StoredVal.getValueType();
+ if (VT == MVT::f64) {
+ assert(Subtarget.hasStdExtZdinx() && !Subtarget.is64Bit() &&
+ "Unexpected custom legalisation");
+
+ // Replace a double precision store with a SplitF64 and i32 stores.
+ SDValue DL(Op);
+ SDValue BasePtr = Store->getBasePtr();
+ SDValue Chain = Store->getChain();
+ SDValue Split = DAG.getNode(RISCVISD::SplitF64, DL,
+ DAG.getVTList(MVT::i32, MVT::i32), StoredVal);
+
+ SDValue Lo = DAG.getStore(
+ Chain, DL, Split.getValue(0), BasePtr, Store->getPointerInfo(),
+ Store->getOriginalAlign(), Store->getMemOperand()->getFlags());
+ BasePtr = DAG.getObjectPtrOffset(DL, BasePtr, TypeSize::getFixed(4));
+ SDValue Hi = DAG.getStore(Chain, DL, Split.getValue(1), BasePtr,
+ Store->getPointerInfo().getWithOffset(4),
+ Store->getOriginalAlign(),
+ Store->getMemOperand()->getFlags());
+ return DAG.getNode(ISD::TokenFactor, DL, MVT::Other, Lo, Hi);
+ }
// Handle normal vector tuple store.
- if (VecTy.isRISCVVectorTuple()) {
+ if (VT.isRISCVVectorTuple()) {
SDLoc DL(Op);
MVT XLenVT = Subtarget.getXLenVT();
- unsigned NF = VecTy.getRISCVVectorTupleNumFields();
- unsigned Sz = VecTy.getSizeInBits().getKnownMinValue();
+ unsigned NF = VT.getRISCVVectorTupleNumFields();
+ unsigned Sz = VT.getSizeInBits().getKnownMinValue();
unsigned NumElts = Sz / (NF * 8);
int Log2LMUL = Log2_64(NumElts) - 3;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoD.td b/llvm/lib/Target/RISCV/RISCVInstrInfoD.td
index 0c584daf45b14..a3300097f5bc4 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoD.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoD.td
@@ -27,8 +27,6 @@ def : GINodeEquiv<G_MERGE_VALUES, RISCVBuildPairF64>;
def RISCVSplitF64 : RVSDNode<"SplitF64", SDT_RISCVSplitF64>;
def : GINodeEquiv<G_UNMERGE_VALUES, RISCVSplitF64>;
-def AddrRegImmINX : ComplexPattern<iPTR, 2, "SelectAddrRegImmRV32Zdinx">;
-
//===----------------------------------------------------------------------===//
// Operand and SDNode transformation definitions.
//===----------------------------------------------------------------------===//
@@ -529,16 +527,12 @@ defm Select_FPR64IN32X : SelectCC_GPR_rrirr<FPR64IN32X, f64>;
def PseudoFROUND_D_IN32X : PseudoFROUND<FPR64IN32X, f64>;
/// Loads
-let isCall = 0, mayLoad = 1, mayStore = 0, Size = 8, isCodeGenOnly = 1 in
+let hasSideEffects = 0, mayLoad = 1, mayStore = 0, Size = 8, isCodeGenOnly = 1 in
def PseudoRV32ZdinxLD : Pseudo<(outs GPRPair:$dst), (ins GPR:$rs1, simm12:$imm12), []>;
-def : Pat<(f64 (load (AddrRegImmINX (XLenVT GPR:$rs1), simm12:$imm12))),
- (PseudoRV32ZdinxLD GPR:$rs1, simm12:$imm12)>;
/// Stores
-let isCall = 0, mayLoad = 0, mayStore = 1, Size = 8, isCodeGenOnly = 1 in
+let hasSideEffects = 0, mayLoad = 0, mayStore = 1, Size = 8, isCodeGenOnly = 1 in
def PseudoRV32ZdinxSD : Pseudo<(outs), (ins GPRPair:$rs2, GPRNoX0:$rs1, simm12:$imm12), []>;
-def : Pat<(store (f64 GPRPair:$rs2), (AddrRegImmINX (XLenVT GPR:$rs1), simm12:$imm12)),
- (PseudoRV32ZdinxSD GPRPair:$rs2, GPR:$rs1, simm12:$imm12)>;
} // Predicates = [HasStdExtZdinx, IsRV32]
let Predicates = [HasStdExtD, IsRV32] in {
diff --git a/llvm/test/CodeGen/RISCV/double-calling-conv.ll b/llvm/test/CodeGen/RISCV/double-calling-conv.ll
index 798eac64e9fc2..1a01fceca75a5 100644
--- a/llvm/test/CodeGen/RISCV/double-calling-conv.ll
+++ b/llvm/test/CodeGen/RISCV/double-calling-conv.ll
@@ -165,10 +165,10 @@ define double @callee_double_stack(i64 %a, i64 %b, i64 %c, i64 %d, double %e, do
;
; RV32IZFINXZDINX-LABEL: callee_double_stack:
; RV32IZFINXZDINX: # %bb.0:
-; RV32IZFINXZDINX-NEXT: lw a0, 8(sp)
; RV32IZFINXZDINX-NEXT: lw a1, 12(sp)
-; RV32IZFINXZDINX-NEXT: lw a2, 0(sp)
; RV32IZFINXZDINX-NEXT: lw a3, 4(sp)
+; RV32IZFINXZDINX-NEXT: lw a0, 8(sp)
+; RV32IZFINXZDINX-NEXT: lw a2, 0(sp)
; RV32IZFINXZDINX-NEXT: fadd.d a0, a2, a0
; RV32IZFINXZDINX-NEXT: ret
%1 = fadd double %e, %f
diff --git a/llvm/test/CodeGen/RISCV/double-convert.ll b/llvm/test/CodeGen/RISCV/double-convert.ll
index 03ab83ece8ce7..0716650374d0d 100644
--- a/llvm/test/CodeGen/RISCV/double-convert.ll
+++ b/llvm/test/CodeGen/RISCV/double-convert.ll
@@ -734,38 +734,42 @@ define i64 @fcvt_l_d_sat(double %a) nounwind {
; RV32IZFINXZDINX-NEXT: sw ra, 12(sp) # 4-byte Folded Spill
; RV32IZFINXZDINX-NEXT: sw s0, 8(sp) # 4-byte Folded Spill
; RV32IZFINXZDINX-NEXT: sw s1, 4(sp) # 4-byte Folded Spill
+; RV32IZFINXZDINX-NEXT: sw s2, 0(sp) # 4-byte Folded Spill
+; RV32IZFINXZDINX-NEXT: lui a2, %hi(.LCPI12_0)
+; RV32IZFINXZDINX-NEXT: lw a4, %lo(.LCPI12_0)(a2)
+; RV32IZFINXZDINX-NEXT: addi a2, a2, %lo(.LCPI12_0)
+; RV32IZFINXZDINX-NEXT: lw a5, 4(a2)
; RV32IZFINXZDINX-NEXT: mv s1, a1
; RV32IZFINXZDINX-NEXT: mv s0, a0
+; RV32IZFINXZDINX-NEXT: fle.d s2, a4, s0
; RV32IZFINXZDINX-NEXT: call __fixdfdi
-; RV32IZFINXZDINX-NEXT: lui a2, %hi(.LCPI12_0)
-; RV32IZFINXZDINX-NEXT: lw a3, %lo(.LCPI12_0+4)(a2)
-; RV32IZFINXZDINX-NEXT: lw a2, %lo(.LCPI12_0)(a2)
-; RV32IZFINXZDINX-NEXT: fle.d a3, a2, s0
-; RV32IZFINXZDINX-NEXT: lui a4, 524288
+; RV32IZFINXZDINX-NEXT: lui a3, 524288
; RV32IZFINXZDINX-NEXT: lui a2, 524288
-; RV32IZFINXZDINX-NEXT: beqz a3, .LBB12_2
+; RV32IZFINXZDINX-NEXT: beqz s2, .LBB12_2
; RV32IZFINXZDINX-NEXT: # %bb.1: # %start
; RV32IZFINXZDINX-NEXT: mv a2, a1
; RV32IZFINXZDINX-NEXT: .LBB12_2: # %start
; RV32IZFINXZDINX-NEXT: lui a1, %hi(.LCPI12_1)
-; RV32IZFINXZDINX-NEXT: lw a6, %lo(.LCPI12_1)(a1)
-; RV32IZFINXZDINX-NEXT: lw a7, %lo(.LCPI12_1+4)(a1)
-; RV32IZFINXZDINX-NEXT: flt.d a1, a6, s0
+; RV32IZFINXZDINX-NEXT: lw a4, %lo(.LCPI12_1)(a1)
+; RV32IZFINXZDINX-NEXT: addi a1, a1, %lo(.LCPI12_1)
+; RV32IZFINXZDINX-NEXT: lw a5, 4(a1)
+; RV32IZFINXZDINX-NEXT: flt.d a1, a4, s0
; RV32IZFINXZDINX-NEXT: beqz a1, .LBB12_4
; RV32IZFINXZDINX-NEXT: # %bb.3:
-; RV32IZFINXZDINX-NEXT: addi a2, a4, -1
+; RV32IZFINXZDINX-NEXT: addi a2, a3, -1
; RV32IZFINXZDINX-NEXT: .LBB12_4: # %start
-; RV32IZFINXZDINX-NEXT: feq.d a4, s0, s0
+; RV32IZFINXZDINX-NEXT: feq.d a3, s0, s0
+; RV32IZFINXZDINX-NEXT: neg a4, a1
+; RV32IZFINXZDINX-NEXT: neg a1, s2
; RV32IZFINXZDINX-NEXT: neg a3, a3
-; RV32IZFINXZDINX-NEXT: neg a5, a1
-; RV32IZFINXZDINX-NEXT: neg a4, a4
+; RV32IZFINXZDINX-NEXT: and a0, a1, a0
+; RV32IZFINXZDINX-NEXT: and a1, a3, a2
+; RV32IZFINXZDINX-NEXT: or a0, a4, a0
; RV32IZFINXZDINX-NEXT: and a0, a3, a0
-; RV32IZFINXZDINX-NEXT: and a1, a4, a2
-; RV32IZFINXZDINX-NEXT: or a0, a5, a0
-; RV32IZFINXZDINX-NEXT: and a0, a4, a0
; RV32IZFINXZDINX-NEXT: lw ra, 12(sp) # 4-byte Folded Reload
; RV32IZFINXZDINX-NEXT: lw s0, 8(sp) # 4-byte Folded Reload
; RV32IZFINXZDINX-NEXT: lw s1, 4(sp) # 4-byte Folded Reload
+; RV32IZFINXZDINX-NEXT: lw s2, 0(sp) # 4-byte Folded Reload
; RV32IZFINXZDINX-NEXT: addi sp, sp, 16
; RV32IZFINXZDINX-NEXT: ret
;
@@ -986,12 +990,13 @@ define i64 @fcvt_lu_d_sat(double %a) nounwind {
; RV32IZFINXZDINX-NEXT: fcvt.d.w a2, zero
; RV32IZFINXZDINX-NEXT: lui a4, %hi(.LCPI14_0)
; RV32IZFINXZDINX-NEXT: fle.d a2, a2, s0
-; RV32IZFINXZDINX-NEXT: lw a5, %lo(.LCPI14_0+4)(a4)
-; RV32IZFINXZDINX-NEXT: lw a4, %lo(.LCPI14_0)(a4)
+; RV32IZFINXZDINX-NEXT: lw a6, %lo(.LCPI14_0)(a4)
+; RV32IZFINXZDINX-NEXT: addi a3, a4, %lo(.LCPI14_0)
+; RV32IZFINXZDINX-NEXT: lw a7, 4(a3)
; RV32IZFINXZDINX-NEXT: neg a2, a2
; RV32IZFINXZDINX-NEXT: and a0, a2, a0
; RV32IZFINXZDINX-NEXT: and a1, a2, a1
-; RV32IZFINXZDINX-NEXT: flt.d a2, a4, s0
+; RV32IZFINXZDINX-NEXT: flt.d a2, a6, s0
; RV32IZFINXZDINX-NEXT: neg a2, a2
; RV32IZFINXZDINX-NEXT: or a0, a2, a0
; RV32IZFINXZDINX-NEXT: or a1, a2, a1
@@ -1653,17 +1658,19 @@ define signext i16 @fcvt_w_s_sat_i16(double %a) nounwind {
; RV32IZFINXZDINX-LABEL: fcvt_w_s_sat_i16:
; RV32IZFINXZDINX: # %bb.0: # %start
; RV32IZFINXZDINX-NEXT: lui a2, %hi(.LCPI26_0)
-; RV32IZFINXZDINX-NEXT: lw a3, %lo(.LCPI26_0+4)(a2)
-; RV32IZFINXZDINX-NEXT: lw a2, %lo(.LCPI26_0)(a2)
-; RV32IZFINXZDINX-NEXT: lui a4, %hi(.LCPI26_1)
-; RV32IZFINXZDINX-NEXT: lw a5, %lo(.LCPI26_1+4)(a4)
-; RV32IZFINXZDINX-NEXT: lw a4, %lo(.LCPI26_1)(a4)
-; RV32IZFINXZDINX-NEXT: fmax.d a2, a0, a2
-; RV32IZFINXZDINX-NEXT: feq.d a0, a0, a0
-; RV32IZFINXZDINX-NEXT: neg a0, a0
-; RV32IZFINXZDINX-NEXT: fmin.d a2, a2, a4
-; RV32IZFINXZDINX-NEXT: fcvt.w.d a1, a2, rtz
-; RV32IZFINXZDINX-NEXT: and a0, a0, a1
+; RV32IZFINXZDINX-NEXT: lui a3, %hi(.LCPI26_1)
+; RV32IZFINXZDINX-NEXT: lw a4, %lo(.LCPI26_0)(a2)
+; RV32IZFINXZDINX-NEXT: addi a2, a2, %lo(.LCPI26_0)
+; RV32IZFINXZDINX-NEXT: lw a5, 4(a2)
+; RV32IZFINXZDINX-NEXT: lw a2, %lo(.LCPI26_1)(a3)
+; RV32IZFINXZDINX-NEXT: addi a3, a3, %lo(.LCPI26_1)
+; RV32IZFINXZDINX-NEXT: lw a3, 4(a3)
+; RV32IZFINXZDINX-NEXT: feq.d a6, a0, a0
+; RV32IZFINXZDINX-NEXT: fmax.d a0, a0, a4
+; RV32IZFINXZDINX-NEXT: fmin.d a0, a0, a2
+; RV32IZFINXZDINX-NEXT: fcvt.w.d a0, a0, rtz
+; RV32IZFINXZDINX-NEXT: neg a1, a6
+; RV32IZFINXZDINX-NEXT: and a0, a1, a0
; RV32IZFINXZDINX-NEXT: ret
;
; RV64IZFINXZDINX-LABEL: fcvt_w_s_sat_i16:
@@ -1850,11 +1857,12 @@ define zeroext i16 @fcvt_wu_s_sat_i16(double %a) nounwind {
; RV32IZFINXZDINX-LABEL: fcvt_wu_s_sat_i16:
; RV32IZFINXZDINX: # %bb.0: # %start
; RV32IZFINXZDINX-NEXT: lui a2, %hi(.LCPI28_0)
-; RV32IZFINXZDINX-NEXT: lw a3, %lo(.LCPI28_0+4)(a2)
-; RV32IZFINXZDINX-NEXT: lw a2, %lo(.LCPI28_0)(a2)
-; RV32IZFINXZDINX-NEXT: fcvt.d.w a4, zero
-; RV32IZFINXZDINX-NEXT: fmax.d a0, a0, a4
-; RV32IZFINXZDINX-NEXT: fmin...
[truncated]
|
topperc
commented
May 14, 2025
| ; RV32IZFINXZDINX-NEXT: sw s2, 0(sp) # 4-byte Folded Spill | ||
| ; RV32IZFINXZDINX-NEXT: lui a2, %hi(.LCPI12_0) | ||
| ; RV32IZFINXZDINX-NEXT: lw a4, %lo(.LCPI12_0)(a2) | ||
| ; RV32IZFINXZDINX-NEXT: addi a2, a2, %lo(.LCPI12_0) |
There was a problem hiding this comment.
We should be able to check the alignment of the constant pool and fold the +4 like we did before.
There was a problem hiding this comment.
This seems like a useful generic optimization as well. I assume you're going to follow up here?
There was a problem hiding this comment.
Yes. I have a patch I'll post later. It only affected the Zdinx lit tests from this patch. Let me know if you have any suggestions on how to hit this issue outside of Zdinx.
There was a problem hiding this comment.
I guess if this is specific to constant pools (as opposed to global constants), then we'd have to have another lowering which generates a constant and then reads pieces of it. I'd look in e.g. i128 legalization or something like that, but on reflection, you're right that his is probably fairly narrow.
There was a problem hiding this comment.
I think there has been discussion in the scalar efficiency sig about when clang/llvm uses constant pools for e.g. 64-bit integers - Zilsd may be another place to revisit whether those should be split before materialization, or put in a constant pool and loaded with ld. https://lists.riscv.org/g/sig-scalar-efficiency/topic/110801992 is the discussion, which I note does focus more on 48-bit instructions for materialisation rather than constant pools.
preames
requested changes
May 14, 2025
lenary
approved these changes
May 14, 2025
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 avoids a bunch of complexity around making sure the offset doesn't exceed 4091 so we can add 4 after splitting later. By splitting early, the split loads/stores will get selected independently.
There's a bit of follow up work to do, particularly around splitting a constant pool load. Overall I think this is cleaner with less edge cases.