Skip to content

[RISCV][GlobalIsel] Reduce constant pool usage without FP extension #158346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 15, 2025
Merged

[RISCV][GlobalIsel] Reduce constant pool usage without FP extension #158346

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5b538f8
[RISCV] Reduce constant pool usage without FP extension
sunshaoce Sep 12, 2025
80f8c3b
Address comments
sunshaoce Sep 13, 2025
81c6378
Remove extra ADD
sunshaoce Sep 13, 2025
7c868d7
Convert G_FCONSTANT to G_CONSTANT
sunshaoce Sep 13, 2025
a4eca16
Add test: constant pool test for 1234.5
sunshaoce Sep 13, 2025
a56f79a
Add more cases in `shouldBeInFConstantPool`
sunshaoce Sep 13, 2025
cc73b5e
Merge branch 'main' into pr-fconstant
sunshaoce Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -745,10 +745,19 @@ bool RISCVInstructionSelector::select(MachineInstr &MI) {
if (!materializeImm(GPRReg, Imm.getSExtValue(), MIB))
return false;

unsigned Opcode = Size == 64 ? RISCV::FMV_D_X
: Size == 32 ? RISCV::FMV_W_X
: RISCV::FMV_H_X;
auto FMV = MIB.buildInstr(Opcode, {DstReg}, {GPRReg});
unsigned Opcode = RISCV::INIT_UNDEF;
MachineInstrBuilder FMV;
if (Subtarget->hasStdExtF() || Subtarget->hasStdExtD() ||
Subtarget->hasStdExtZfh()) {
Opcode = Size == 64 ? RISCV::FMV_D_X
: Size == 32 ? RISCV::FMV_W_X
: RISCV::FMV_H_X;
FMV = MIB.buildInstr(Opcode, {DstReg}, {GPRReg});
} else {
Opcode =
(Subtarget->is64Bit() && Size == 32) ? RISCV::ADDW : RISCV::ADD;
FMV = MIB.buildInstr(Opcode, {DstReg}, {GPRReg, Register(RISCV::X0)});
}
if (!FMV.constrainAllUses(TII, TRI, RBI))
return false;
} else {
Expand Down
17 changes: 15 additions & 2 deletions llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,9 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
.legalFor(ST.hasStdExtF(), {s32})
.legalFor(ST.hasStdExtD(), {s64})
.legalFor(ST.hasStdExtZfh(), {s16})
.lowerFor({s32, s64, s128});
.customFor(!ST.is64Bit(), {s32})
.customFor(ST.is64Bit(), {s32, s64})
.lowerFor({s64, s128});

getActionDefinitionsBuilder({G_FPTOSI, G_FPTOUI})
.legalFor(ST.hasStdExtF(), {{sXLen, s32}})
Expand Down Expand Up @@ -869,6 +871,12 @@ bool RISCVLegalizerInfo::shouldBeInConstantPool(const APInt &APImm,
return !(!SeqLo.empty() && (SeqLo.size() + 2) <= STI.getMaxBuildIntsCost());
}

bool RISCVLegalizerInfo::shouldBeInFConstantPool(const APFloat &APImm) const {
if (APImm.isZero() || APImm.isExactlyValue(1.0))
return false;
return true;
}

bool RISCVLegalizerInfo::legalizeVScale(MachineInstr &MI,
MachineIRBuilder &MIB) const {
const LLT XLenTy(STI.getXLenVT());
Expand Down Expand Up @@ -1358,7 +1366,12 @@ bool RISCVLegalizerInfo::legalizeCustom(
return false;
case TargetOpcode::G_ABS:
return Helper.lowerAbsToMaxNeg(MI);
// TODO: G_FCONSTANT
case TargetOpcode::G_FCONSTANT: {
const ConstantFP *ConstVal = MI.getOperand(1).getFPImm();
if (!shouldBeInFConstantPool(ConstVal->getValue()))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we convert it to an integer G_CONSTANT here? That's what ARM does.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven’t figured out how to directly turn G_FCONSTANT into a G_CONSTANT, so instead I converted ConstVal->getValue() with bitcastToAPInt().

Copy link
Collaborator

@topperc topperc Sep 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean we can do this and not need any changes to register bank selection or instrution selection

+  case TargetOpcode::G_FCONSTANT: {
+    const ConstantFP *ConstVal = MI.getOperand(1).getFPImm();
+    if (shouldBeInFConstantPool(ConstVal->getValue()))
+      return Helper.lowerFConstant(MI);
+
+    APInt AsInteger =
+        MI.getOperand(1).getFPImm()->getValueAPF().bitcastToAPInt();
+    MIRBuilder.buildConstant(
+        MI.getOperand(0),
+        *ConstantInt::get(MF.getFunction().getContext(), AsInteger));
+    MI.eraseFromParent();
+    return true;
+  }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed reply. This approach is indeed much simpler.

return true;
return Helper.lowerFConstant(MI);
}
case TargetOpcode::G_CONSTANT: {
const Function &F = MF.getFunction();
// TODO: if PSI and BFI are present, add " ||
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class RISCVLegalizerInfo : public LegalizerInfo {

private:
bool shouldBeInConstantPool(const APInt &APImm, bool ShouldOptForSize) const;
bool shouldBeInFConstantPool(const APFloat &APImm) const;
bool legalizeShlAshrLshr(MachineInstr &MI, MachineIRBuilder &MIRBuilder,
GISelChangeObserver &Observer) const;

Expand Down
33 changes: 19 additions & 14 deletions llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ using namespace llvm;
RISCVRegisterBankInfo::RISCVRegisterBankInfo(unsigned HwMode)
: RISCVGenRegisterBankInfo(HwMode) {}

static const RegisterBankInfo::ValueMapping *getFPValueMapping(unsigned Size) {
static const RegisterBankInfo::ValueMapping *
getFPValueMapping(unsigned Size, bool HasFPExt = true) {
unsigned Idx;
switch (Size) {
default:
Expand All @@ -121,10 +122,10 @@ static const RegisterBankInfo::ValueMapping *getFPValueMapping(unsigned Size) {
Idx = RISCV::FPRB16Idx;
break;
case 32:
Idx = RISCV::FPRB32Idx;
Idx = HasFPExt ? RISCV::FPRB32Idx : RISCV::GPRB32Idx;
break;
case 64:
Idx = RISCV::FPRB64Idx;
Idx = HasFPExt ? RISCV::FPRB64Idx : RISCV::GPRB64Idx;
break;
}
return &RISCV::ValueMappings[Idx];
Expand Down Expand Up @@ -219,6 +220,10 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
const TargetSubtargetInfo &STI = MF.getSubtarget();
const TargetRegisterInfo &TRI = *STI.getRegisterInfo();

bool HasFPExt = STI.hasFeature(RISCV::FeatureStdExtF) ||
STI.hasFeature(RISCV::FeatureStdExtD) ||
STI.hasFeature(RISCV::FeatureStdExtZfh);

unsigned GPRSize = getMaximumSize(RISCV::GPRBRegBankID);
assert((GPRSize == 32 || GPRSize == 64) && "Unexpected GPR size");

Expand Down Expand Up @@ -266,7 +271,7 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
if (Ty.isVector())
Mapping = getVRBValueMapping(Size.getKnownMinValue());
else if (isPreISelGenericFloatingPointOpcode(Opc))
Mapping = getFPValueMapping(Size.getFixedValue());
Mapping = getFPValueMapping(Size.getFixedValue(), HasFPExt);
else
Mapping = GPRValueMapping;

Expand Down Expand Up @@ -301,7 +306,7 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
if (DstTy.isVector())
Mapping = getVRBValueMapping(DstMinSize);
else if (anyUseOnlyUseFP(Dst, MRI, TRI))
Mapping = getFPValueMapping(DstMinSize);
Mapping = getFPValueMapping(DstMinSize, HasFPExt);

return getInstructionMapping(DefaultMappingID, /*Cost=*/1, Mapping,
NumOperands);
Expand Down Expand Up @@ -339,7 +344,7 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
// assume this was a floating point load in the IR. If it was
// not, we would have had a bitcast before reaching that
// instruction.
OpdsMapping[0] = getFPValueMapping(Size);
OpdsMapping[0] = getFPValueMapping(Size, HasFPExt);
break;
}

Expand Down Expand Up @@ -367,7 +372,7 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {

MachineInstr *DefMI = MRI.getVRegDef(MI.getOperand(0).getReg());
if (onlyDefinesFP(*DefMI, MRI, TRI))
OpdsMapping[0] = getFPValueMapping(Ty.getSizeInBits());
OpdsMapping[0] = getFPValueMapping(Ty.getSizeInBits(), HasFPExt);
break;
}
case TargetOpcode::G_SELECT: {
Expand Down Expand Up @@ -432,7 +437,7 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {

const ValueMapping *Mapping = GPRValueMapping;
if (NumFP >= 2)
Mapping = getFPValueMapping(Ty.getSizeInBits());
Mapping = getFPValueMapping(Ty.getSizeInBits(), HasFPExt);

OpdsMapping[0] = OpdsMapping[2] = OpdsMapping[3] = Mapping;
break;
Expand All @@ -444,13 +449,13 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
case RISCV::G_FCLASS: {
LLT Ty = MRI.getType(MI.getOperand(1).getReg());
OpdsMapping[0] = GPRValueMapping;
OpdsMapping[1] = getFPValueMapping(Ty.getSizeInBits());
OpdsMapping[1] = getFPValueMapping(Ty.getSizeInBits(), HasFPExt);
break;
}
case TargetOpcode::G_SITOFP:
case TargetOpcode::G_UITOFP: {
LLT Ty = MRI.getType(MI.getOperand(0).getReg());
OpdsMapping[0] = getFPValueMapping(Ty.getSizeInBits());
OpdsMapping[0] = getFPValueMapping(Ty.getSizeInBits(), HasFPExt);
OpdsMapping[1] = GPRValueMapping;
break;
}
Expand All @@ -468,7 +473,7 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
LLT Ty = MRI.getType(MI.getOperand(0).getReg());
if (GPRSize == 32 && Ty.getSizeInBits() == 64) {
assert(MF.getSubtarget<RISCVSubtarget>().hasStdExtD());
OpdsMapping[0] = getFPValueMapping(Ty.getSizeInBits());
OpdsMapping[0] = getFPValueMapping(Ty.getSizeInBits(), HasFPExt);
OpdsMapping[1] = GPRValueMapping;
OpdsMapping[2] = GPRValueMapping;
}
Expand All @@ -481,7 +486,7 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
assert(MF.getSubtarget<RISCVSubtarget>().hasStdExtD());
OpdsMapping[0] = GPRValueMapping;
OpdsMapping[1] = GPRValueMapping;
OpdsMapping[2] = getFPValueMapping(Ty.getSizeInBits());
OpdsMapping[2] = getFPValueMapping(Ty.getSizeInBits(), HasFPExt);
}
break;
}
Expand All @@ -495,7 +500,7 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
if ((GPRSize == 32 && ScalarTy.getSizeInBits() == 64) ||
onlyDefinesFP(*DefMI, MRI, TRI)) {
assert(MF.getSubtarget<RISCVSubtarget>().hasStdExtD());
OpdsMapping[1] = getFPValueMapping(ScalarTy.getSizeInBits());
OpdsMapping[1] = getFPValueMapping(ScalarTy.getSizeInBits(), HasFPExt);
} else
OpdsMapping[1] = GPRValueMapping;
break;
Expand All @@ -514,7 +519,7 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
OpdsMapping[Idx] =
getVRBValueMapping(Ty.getSizeInBits().getKnownMinValue());
else if (isPreISelGenericFloatingPointOpcode(Opc))
OpdsMapping[Idx] = getFPValueMapping(Ty.getSizeInBits());
OpdsMapping[Idx] = getFPValueMapping(Ty.getSizeInBits(), HasFPExt);
else
OpdsMapping[Idx] = GPRValueMapping;
}
Expand Down
44 changes: 16 additions & 28 deletions llvm/test/CodeGen/RISCV/GlobalISel/constantpool.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,47 +15,37 @@
define void @constpool_f32(ptr %p) {
; RV32-SMALL-LABEL: constpool_f32:
; RV32-SMALL: # %bb.0:
; RV32-SMALL-NEXT: lui a1, %hi(.LCPI0_0)
; RV32-SMALL-NEXT: lw a1, %lo(.LCPI0_0)(a1)
; RV32-SMALL-NEXT: lui a1, 260096
; RV32-SMALL-NEXT: sw a1, 0(a0)
; RV32-SMALL-NEXT: ret
;
; RV32-MEDIUM-LABEL: constpool_f32:
; RV32-MEDIUM: # %bb.0:
; RV32-MEDIUM-NEXT: .Lpcrel_hi0:
; RV32-MEDIUM-NEXT: auipc a1, %pcrel_hi(.LCPI0_0)
; RV32-MEDIUM-NEXT: lw a1, %pcrel_lo(.Lpcrel_hi0)(a1)
; RV32-MEDIUM-NEXT: lui a1, 260096
; RV32-MEDIUM-NEXT: sw a1, 0(a0)
; RV32-MEDIUM-NEXT: ret
;
; RV32-PIC-LABEL: constpool_f32:
; RV32-PIC: # %bb.0:
; RV32-PIC-NEXT: .Lpcrel_hi0:
; RV32-PIC-NEXT: auipc a1, %pcrel_hi(.LCPI0_0)
; RV32-PIC-NEXT: lw a1, %pcrel_lo(.Lpcrel_hi0)(a1)
; RV32-PIC-NEXT: lui a1, 260096
; RV32-PIC-NEXT: sw a1, 0(a0)
; RV32-PIC-NEXT: ret
;
; RV64-SMALL-LABEL: constpool_f32:
; RV64-SMALL: # %bb.0:
; RV64-SMALL-NEXT: lui a1, %hi(.LCPI0_0)
; RV64-SMALL-NEXT: lw a1, %lo(.LCPI0_0)(a1)
; RV64-SMALL-NEXT: lui a1, 260096
; RV64-SMALL-NEXT: sw a1, 0(a0)
; RV64-SMALL-NEXT: ret
;
; RV64-MEDIUM-LABEL: constpool_f32:
; RV64-MEDIUM: # %bb.0:
; RV64-MEDIUM-NEXT: .Lpcrel_hi0:
; RV64-MEDIUM-NEXT: auipc a1, %pcrel_hi(.LCPI0_0)
; RV64-MEDIUM-NEXT: lw a1, %pcrel_lo(.Lpcrel_hi0)(a1)
; RV64-MEDIUM-NEXT: lui a1, 260096
; RV64-MEDIUM-NEXT: sw a1, 0(a0)
; RV64-MEDIUM-NEXT: ret
;
; RV64-PIC-LABEL: constpool_f32:
; RV64-PIC: # %bb.0:
; RV64-PIC-NEXT: .Lpcrel_hi0:
; RV64-PIC-NEXT: auipc a1, %pcrel_hi(.LCPI0_0)
; RV64-PIC-NEXT: lw a1, %pcrel_lo(.Lpcrel_hi0)(a1)
; RV64-PIC-NEXT: lui a1, 260096
; RV64-PIC-NEXT: sw a1, 0(a0)
; RV64-PIC-NEXT: ret
store float 1.0, ptr %p
Expand All @@ -75,9 +65,9 @@ define void @constpool_f64(ptr %p) {
;
; RV32-MEDIUM-LABEL: constpool_f64:
; RV32-MEDIUM: # %bb.0:
; RV32-MEDIUM-NEXT: .Lpcrel_hi1:
; RV32-MEDIUM-NEXT: .Lpcrel_hi0:
; RV32-MEDIUM-NEXT: auipc a1, %pcrel_hi(.LCPI1_0)
; RV32-MEDIUM-NEXT: addi a1, a1, %pcrel_lo(.Lpcrel_hi1)
; RV32-MEDIUM-NEXT: addi a1, a1, %pcrel_lo(.Lpcrel_hi0)
; RV32-MEDIUM-NEXT: lw a2, 0(a1)
; RV32-MEDIUM-NEXT: lw a1, 4(a1)
; RV32-MEDIUM-NEXT: sw a2, 0(a0)
Expand All @@ -86,9 +76,9 @@ define void @constpool_f64(ptr %p) {
;
; RV32-PIC-LABEL: constpool_f64:
; RV32-PIC: # %bb.0:
; RV32-PIC-NEXT: .Lpcrel_hi1:
; RV32-PIC-NEXT: .Lpcrel_hi0:
; RV32-PIC-NEXT: auipc a1, %pcrel_hi(.LCPI1_0)
; RV32-PIC-NEXT: addi a1, a1, %pcrel_lo(.Lpcrel_hi1)
; RV32-PIC-NEXT: addi a1, a1, %pcrel_lo(.Lpcrel_hi0)
; RV32-PIC-NEXT: lw a2, 0(a1)
; RV32-PIC-NEXT: lw a1, 4(a1)
; RV32-PIC-NEXT: sw a2, 0(a0)
Expand All @@ -97,24 +87,22 @@ define void @constpool_f64(ptr %p) {
;
; RV64-SMALL-LABEL: constpool_f64:
; RV64-SMALL: # %bb.0:
; RV64-SMALL-NEXT: lui a1, %hi(.LCPI1_0)
; RV64-SMALL-NEXT: ld a1, %lo(.LCPI1_0)(a1)
; RV64-SMALL-NEXT: li a1, 1023
; RV64-SMALL-NEXT: slli a1, a1, 52
; RV64-SMALL-NEXT: sd a1, 0(a0)
; RV64-SMALL-NEXT: ret
;
; RV64-MEDIUM-LABEL: constpool_f64:
; RV64-MEDIUM: # %bb.0:
; RV64-MEDIUM-NEXT: .Lpcrel_hi1:
; RV64-MEDIUM-NEXT: auipc a1, %pcrel_hi(.LCPI1_0)
; RV64-MEDIUM-NEXT: ld a1, %pcrel_lo(.Lpcrel_hi1)(a1)
; RV64-MEDIUM-NEXT: li a1, 1023
; RV64-MEDIUM-NEXT: slli a1, a1, 52
; RV64-MEDIUM-NEXT: sd a1, 0(a0)
; RV64-MEDIUM-NEXT: ret
;
; RV64-PIC-LABEL: constpool_f64:
; RV64-PIC: # %bb.0:
; RV64-PIC-NEXT: .Lpcrel_hi1:
; RV64-PIC-NEXT: auipc a1, %pcrel_hi(.LCPI1_0)
; RV64-PIC-NEXT: ld a1, %pcrel_lo(.Lpcrel_hi1)(a1)
; RV64-PIC-NEXT: li a1, 1023
; RV64-PIC-NEXT: slli a1, a1, 52
; RV64-PIC-NEXT: sd a1, 0(a0)
; RV64-PIC-NEXT: ret
store double 1.0, ptr %p
Expand Down
Loading
Loading