Skip to content

[X86][GlobalISel] - Legalize And Select of G_FPTOSI/G_SITOFP in X87 mode #137377

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.

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 11 commits into from
Jun 2, 2025
Merged
10 changes: 10 additions & 0 deletions llvm/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,16 @@ class LegalizeRuleSet {
Types2);
}

/// The instruction is custom when the predicate is true and type indexes 0
/// and 1 are all in their respective lists.
LegalizeRuleSet &
customForCartesianProduct(bool Pred, std::initializer_list<LLT> Types0,
std::initializer_list<LLT> Types1) {
if (!Pred)
return *this;
return actionForCartesianProduct(LegalizeAction::Custom, Types0, Types1);
}

/// Unconditionally custom lower.
LegalizeRuleSet &custom() {
return customIf(always);
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ bool X86InstructionSelector::select(MachineInstr &I) {
MachineRegisterInfo &MRI = MF.getRegInfo();

unsigned Opcode = I.getOpcode();
if (!isPreISelGenericOpcode(Opcode)) {
if (!isPreISelGenericOpcode(Opcode) && !I.isPreISelOpcode()) {
// Certain non-generic instructions also need some special handling.

if (Opcode == TargetOpcode::LOAD_STACK_GUARD)
Expand Down
92 changes: 73 additions & 19 deletions llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -490,31 +490,25 @@ X86LegalizerInfo::X86LegalizerInfo(const X86Subtarget &STI,
});

getActionDefinitionsBuilder(G_SITOFP)
.legalIf([=](const LegalityQuery &Query) {
return (HasSSE1 &&
(typePairInSet(0, 1, {{s32, s32}})(Query) ||
(Is64Bit && typePairInSet(0, 1, {{s32, s64}})(Query)))) ||
(HasSSE2 &&
(typePairInSet(0, 1, {{s64, s32}})(Query) ||
(Is64Bit && typePairInSet(0, 1, {{s64, s64}})(Query))));
})
.clampScalar(1, s32, sMaxScalar)
.legalFor(HasSSE1, {{s32, s32}})
.legalFor(HasSSE1 && Is64Bit, {{s32, s64}})
.legalFor(HasSSE2, {{s64, s32}})
.legalFor(HasSSE2 && Is64Bit, {{s64, s64}})
.clampScalar(1, (UseX87 && !HasSSE1) ? s16 : s32, sMaxScalar)
.widenScalarToNextPow2(1)
.customForCartesianProduct(UseX87, {s32, s64, s80}, {s16, s32, s64})
.clampScalar(0, s32, HasSSE2 ? s64 : s32)
.widenScalarToNextPow2(0);

getActionDefinitionsBuilder(G_FPTOSI)
.legalIf([=](const LegalityQuery &Query) {
return (HasSSE1 &&
(typePairInSet(0, 1, {{s32, s32}})(Query) ||
(Is64Bit && typePairInSet(0, 1, {{s64, s32}})(Query)))) ||
(HasSSE2 &&
(typePairInSet(0, 1, {{s32, s64}})(Query) ||
(Is64Bit && typePairInSet(0, 1, {{s64, s64}})(Query))));
})
.clampScalar(1, s32, HasSSE2 ? s64 : s32)
.legalFor(HasSSE1, {{s32, s32}})
.legalFor(HasSSE1 && Is64Bit, {{s64, s32}})
.legalFor(HasSSE2, {{s32, s64}})
.legalFor(HasSSE2 && Is64Bit, {{s64, s64}})
.clampScalar(0, (UseX87 && !HasSSE1) ? s16 : s32, sMaxScalar)
.widenScalarToNextPow2(0)
.clampScalar(0, s32, sMaxScalar)
.customForCartesianProduct(UseX87, {s16, s32, s64}, {s32, s64, s80})
.clampScalar(1, s32, HasSSE2 ? s64 : s32)
.widenScalarToNextPow2(1);

// For G_UITOFP and G_FPTOUI without AVX512, we have to custom legalize types
Expand Down Expand Up @@ -671,10 +665,70 @@ bool X86LegalizerInfo::legalizeCustom(LegalizerHelper &Helper, MachineInstr &MI,
return legalizeUITOFP(MI, MRI, Helper);
case TargetOpcode::G_STORE:
return legalizeNarrowingStore(MI, MRI, Helper);
case TargetOpcode::G_SITOFP:
return legalizeSITOFP(MI, MRI, Helper);
case TargetOpcode::G_FPTOSI:
return legalizeFPTOSI(MI, MRI, Helper);
}
llvm_unreachable("expected switch to return");
}

bool X86LegalizerInfo::legalizeSITOFP(MachineInstr &MI,
MachineRegisterInfo &MRI,
LegalizerHelper &Helper) const {
MachineIRBuilder &MIRBuilder = Helper.MIRBuilder;
MachineFunction &MF = *MI.getMF();
auto [Dst, DstTy, Src, SrcTy] = MI.getFirst2RegLLTs();

assert((SrcTy.getSizeInBits() == 16 || SrcTy.getSizeInBits() == 32 ||
SrcTy.getSizeInBits() == 64) &&
"Unexpected source type for SITOFP in X87 mode.");

TypeSize MemSize = SrcTy.getSizeInBytes();
MachinePointerInfo PtrInfo;
Align Alignmt = Helper.getStackTemporaryAlignment(SrcTy);
auto SlotPointer = Helper.createStackTemporary(MemSize, Alignmt, PtrInfo);
MachineMemOperand *StoreMMO = MF.getMachineMemOperand(
PtrInfo, MachineMemOperand::MOStore, MemSize, Align(MemSize));

// Store the integer value on the FPU stack.
MIRBuilder.buildStore(Src, SlotPointer, *StoreMMO);

MachineMemOperand *LoadMMO = MF.getMachineMemOperand(
PtrInfo, MachineMemOperand::MOLoad, MemSize, Align(MemSize));
MIRBuilder.buildInstr(X86::G_FILD)
.addDef(Dst)
.addUse(SlotPointer.getReg(0))
.addMemOperand(LoadMMO);

MI.eraseFromParent();
return true;
}

bool X86LegalizerInfo::legalizeFPTOSI(MachineInstr &MI,
MachineRegisterInfo &MRI,
LegalizerHelper &Helper) const {
MachineFunction &MF = *MI.getMF();
MachineIRBuilder &MIRBuilder = Helper.MIRBuilder;
auto [Dst, DstTy, Src, SrcTy] = MI.getFirst2RegLLTs();

TypeSize MemSize = DstTy.getSizeInBytes();
MachinePointerInfo PtrInfo;
Align Alignmt = Helper.getStackTemporaryAlignment(DstTy);
auto SlotPointer = Helper.createStackTemporary(MemSize, Alignmt, PtrInfo);
MachineMemOperand *StoreMMO = MF.getMachineMemOperand(
PtrInfo, MachineMemOperand::MOStore, MemSize, Align(MemSize));

MIRBuilder.buildInstr(X86::G_FIST)
.addUse(Src)
.addUse(SlotPointer.getReg(0))
.addMemOperand(StoreMMO);

MIRBuilder.buildLoad(Dst, SlotPointer, PtrInfo, Align(MemSize));
MI.eraseFromParent();
return true;
}

bool X86LegalizerInfo::legalizeBuildVector(MachineInstr &MI,
MachineRegisterInfo &MRI,
LegalizerHelper &Helper) const {
Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/Target/X86/GISel/X86LegalizerInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ class X86LegalizerInfo : public LegalizerInfo {

bool legalizeNarrowingStore(MachineInstr &MI, MachineRegisterInfo &MRI,
LegalizerHelper &Helper) const;

bool legalizeSITOFP(MachineInstr &MI, MachineRegisterInfo &MRI,
LegalizerHelper &Helper) const;

bool legalizeFPTOSI(MachineInstr &MI, MachineRegisterInfo &MRI,
LegalizerHelper &Helper) const;
};
} // namespace llvm
#endif
12 changes: 12 additions & 0 deletions llvm/lib/Target/X86/GISel/X86RegisterBankInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ bool X86RegisterBankInfo::onlyUsesFP(const MachineInstr &MI,
case TargetOpcode::G_FPTOSI:
case TargetOpcode::G_FPTOUI:
case TargetOpcode::G_FCMP:
case X86::G_FIST:
case TargetOpcode::G_LROUND:
case TargetOpcode::G_LLROUND:
case TargetOpcode::G_INTRINSIC_TRUNC:
Expand All @@ -129,6 +130,7 @@ bool X86RegisterBankInfo::onlyDefinesFP(const MachineInstr &MI,
switch (MI.getOpcode()) {
case TargetOpcode::G_SITOFP:
case TargetOpcode::G_UITOFP:
case X86::G_FILD:
return true;
default:
break;
Expand Down Expand Up @@ -296,6 +298,16 @@ X86RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
// VECRReg)
getInstrPartialMappingIdxs(MI, MRI, /* isFP= */ true, OpRegBankIdx);
break;
case X86::G_FIST:
case X86::G_FILD: {
auto &Op0 = MI.getOperand(0);
auto &Op1 = MI.getOperand(1);
const LLT Ty0 = MRI.getType(Op0.getReg());
const LLT Ty1 = MRI.getType(Op1.getReg());
OpRegBankIdx[0] = getPartialMappingIdx(MI, Ty0, /* isFP= */ true);
OpRegBankIdx[1] = getPartialMappingIdx(MI, Ty1, /* isFP= */ false);
break;
}
case TargetOpcode::G_SITOFP:
case TargetOpcode::G_FPTOSI:
case TargetOpcode::G_UITOFP:
Expand Down
34 changes: 28 additions & 6 deletions llvm/lib/Target/X86/X86InstrFragments.td
Original file line number Diff line number Diff line change
Expand Up @@ -844,13 +844,24 @@ def X86fldf80 : PatFrag<(ops node:$ptr), (X86fld node:$ptr), [{

def X86fild16 : PatFrag<(ops node:$ptr), (X86fild node:$ptr), [{
return cast<MemIntrinsicSDNode>(N)->getMemoryVT() == MVT::i16;
}]>;
}]> {
let IsLoad = true;
let MemoryVT = i16;
}

def X86fild32 : PatFrag<(ops node:$ptr), (X86fild node:$ptr), [{
return cast<MemIntrinsicSDNode>(N)->getMemoryVT() == MVT::i32;
}]>;
}]> {
let IsLoad = true;
let MemoryVT = i32;
}

def X86fild64 : PatFrag<(ops node:$ptr), (X86fild node:$ptr), [{
return cast<MemIntrinsicSDNode>(N)->getMemoryVT() == MVT::i64;
}]>;
}]> {
let IsLoad = true;
let MemoryVT = i64;
}

def X86fist32 : PatFrag<(ops node:$val, node:$ptr),
(X86fist node:$val, node:$ptr), [{
Expand All @@ -865,15 +876,26 @@ def X86fist64 : PatFrag<(ops node:$val, node:$ptr),
def X86fp_to_i16mem : PatFrag<(ops node:$val, node:$ptr),
(X86fp_to_mem node:$val, node:$ptr), [{
return cast<MemIntrinsicSDNode>(N)->getMemoryVT() == MVT::i16;
}]>;
}]> {
let IsStore = true;
let MemoryVT = i16;
}

def X86fp_to_i32mem : PatFrag<(ops node:$val, node:$ptr),
(X86fp_to_mem node:$val, node:$ptr), [{
return cast<MemIntrinsicSDNode>(N)->getMemoryVT() == MVT::i32;
}]>;
}]> {
let IsStore = true;
let MemoryVT = i32;
}

def X86fp_to_i64mem : PatFrag<(ops node:$val, node:$ptr),
(X86fp_to_mem node:$val, node:$ptr), [{
return cast<MemIntrinsicSDNode>(N)->getMemoryVT() == MVT::i64;
Copy link
Contributor

Choose a reason for hiding this comment

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

I expected that we don't need SDAG predicate as well, do we?

Copy link
Contributor

Choose a reason for hiding this comment

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

The memory type should replace this

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 feel that should be done in a separate PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm ok with both options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I will address this in a separate PR.

}]>;
}]> {
let IsStore = true;
let MemoryVT = i64;
}

//===----------------------------------------------------------------------===//
// FPStack pattern fragments
Expand Down
31 changes: 31 additions & 0 deletions llvm/lib/Target/X86/X86InstrGISel.td
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//===- X86InstrGISel.td - X86 GISel target specific opcodes -*- tablegen -*===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
//
// X86 GlobalISel target pseudo instruction definitions. This is kept
// separately from the other tablegen files for organizational purposes, but
// share the same infrastructure.
//
//===----------------------------------------------------------------------===//

class X86GenericInstruction : GenericInstruction { let Namespace = "X86"; }

def G_FILD : X86GenericInstruction {
let OutOperandList = (outs type0:$dst);
let InOperandList = (ins ptype1:$src);
let hasSideEffects = false;
let mayLoad = true;
}
def G_FIST : X86GenericInstruction {
let OutOperandList = (outs);
let InOperandList = (ins type0:$src1, ptype1:$src2);
let hasSideEffects = false;
let mayStore = true;
}

def : GINodeEquiv<G_FILD, X86fild>;
def : GINodeEquiv<G_FIST, X86fp_to_mem>;
5 changes: 5 additions & 0 deletions llvm/lib/Target/X86/X86InstrInfo.td
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ include "X86InstrFormats.td"
//
include "X86InstrUtils.td"

//===----------------------------------------------------------------------===//
// Global ISel
//
include "X86InstrGISel.td"

//===----------------------------------------------------------------------===//
// Subsystems.
//===----------------------------------------------------------------------===//
Expand Down
Loading
Loading