Skip to content
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 @@ -926,6 +926,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
11 changes: 10 additions & 1 deletion llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ class X86InstructionSelector : public InstructionSelector {
unsigned getPtrLoadStoreOp(const LLT &Ty, const RegisterBank &RB,
unsigned Opc) const;

bool checkMemoryOpSize(const MachineInstr &MI, LLT Sz) const;

bool selectLoadStoreOp(MachineInstr &I, MachineRegisterInfo &MRI,
MachineFunction &MF) const;
bool selectFrameIndexOrGep(MachineInstr &I, MachineRegisterInfo &MRI,
Expand Down Expand Up @@ -355,6 +357,13 @@ bool X86InstructionSelector::selectCopy(MachineInstr &I,
return true;
}

bool X86InstructionSelector::checkMemoryOpSize(const MachineInstr &MI,
LLT Sz) const {
assert(MI.hasOneMemOperand() &&
"Expected load/store to have only one mem op!");
return (*MI.memoperands_begin())->getMemoryType() == Sz;
}

bool X86InstructionSelector::select(MachineInstr &I) {
assert(I.getParent() && "Instruction should be in a basic block!");
assert(I.getParent()->getParent() && "Instruction should be in a function!");
Expand All @@ -364,7 +373,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
93 changes: 74 additions & 19 deletions llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "llvm/CodeGen/GlobalISel/LegalizerHelper.h"
#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
#include "llvm/CodeGen/MachineConstantPool.h"
#include "llvm/CodeGen/MachineFrameInfo.h"
#include "llvm/CodeGen/TargetOpcodes.h"
#include "llvm/CodeGen/ValueTypes.h"
#include "llvm/IR/DerivedTypes.h"
Expand Down Expand Up @@ -490,31 +491,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 +666,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
28 changes: 22 additions & 6 deletions llvm/lib/Target/X86/X86InstrFragments.td
Original file line number Diff line number Diff line change
Expand Up @@ -841,13 +841,21 @@ 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 GISelPredicateCode = [{ return checkMemoryOpSize(MI, LLT::scalar(16)); }];
}

def X86fild32 : PatFrag<(ops node:$ptr), (X86fild node:$ptr), [{
return cast<MemIntrinsicSDNode>(N)->getMemoryVT() == MVT::i32;
}]>;
}]> {
let GISelPredicateCode = [{ return checkMemoryOpSize(MI, LLT::scalar(32)); }];
}

def X86fild64 : PatFrag<(ops node:$ptr), (X86fild node:$ptr), [{
return cast<MemIntrinsicSDNode>(N)->getMemoryVT() == MVT::i64;
}]>;
}]> {
let GISelPredicateCode = [{ return checkMemoryOpSize(MI, LLT::scalar(64)); }];
}

def X86fist32 : PatFrag<(ops node:$val, node:$ptr),
(X86fist node:$val, node:$ptr), [{
Expand All @@ -862,15 +870,23 @@ 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 GISelPredicateCode = [{ return checkMemoryOpSize(MI, LLT::scalar(16)); }];
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't need a custom code predicate for this. There is already a MemoryVT and ScalarMemoryVT field on PatFrag

Copy link
Contributor

Choose a reason for hiding this comment

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

Bump

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @arsenm, I don't get the idea. Do you suggest to specify the type of $val and get rid of predicate for both SDAG and GISEL? I've tried to find examples of PatFrag using its memory fields but was unsuccessful.
Otherwise GlobalISel will always try to find its own version of predicate regardless of SDAG implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, not the type of val, but the memory type for X86_fp_to_mem. Many of the common custom predicates were moved to specific fields on PatFrag that TableGen recognizes, like the memory alignment to avoid custom predicate code. This is how extending and atomic loads are specified, but the mechanism is supposed to be general.

The example would be how atomic_load_32 and atomic_load_64 work, they each set MemoryVT and wrap another PatFrag for atomic_load, which in turn wraps a PatFrag around ld

}

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 GISelPredicateCode = [{ return checkMemoryOpSize(MI, LLT::scalar(32)); }];
}

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 GISelPredicateCode = [{ return checkMemoryOpSize(MI, LLT::scalar(64)); }];
}

//===----------------------------------------------------------------------===//
// 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