-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[RISCV][GISel] Add support for G_FCMP with F and D extensions. #70624
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,6 +67,8 @@ class RISCVInstructionSelector : public InstructionSelector { | |
bool selectSExtInreg(MachineInstr &MI, MachineIRBuilder &MIB) const; | ||
bool selectSelect(MachineInstr &MI, MachineIRBuilder &MIB, | ||
MachineRegisterInfo &MRI) const; | ||
bool selectFPCompare(MachineInstr &MI, MachineIRBuilder &MIB, | ||
MachineRegisterInfo &MRI) const; | ||
|
||
ComplexRendererFns selectShiftMask(MachineOperand &Root) const; | ||
ComplexRendererFns selectAddrRegImm(MachineOperand &Root) const; | ||
|
@@ -415,6 +417,8 @@ bool RISCVInstructionSelector::select(MachineInstr &MI) { | |
} | ||
case TargetOpcode::G_SELECT: | ||
return selectSelect(MI, MIB, MRI); | ||
case TargetOpcode::G_FCMP: | ||
return selectFPCompare(MI, MIB, MRI); | ||
default: | ||
return false; | ||
} | ||
|
@@ -820,6 +824,130 @@ bool RISCVInstructionSelector::selectSelect(MachineInstr &MI, | |
return constrainSelectedInstRegOperands(*Result, TII, TRI, RBI); | ||
} | ||
|
||
// Convert an FCMP predicate to one of the supported F or D instructions. | ||
static unsigned getFCmpOpcode(CmpInst::Predicate Pred, unsigned Size) { | ||
assert((Size == 32 || Size == 64) && "Unsupported size"); | ||
switch (Pred) { | ||
default: | ||
llvm_unreachable("Unsupported predicate"); | ||
case CmpInst::FCMP_OLT: | ||
return Size == 32 ? RISCV::FLT_S : RISCV::FLT_D; | ||
case CmpInst::FCMP_OLE: | ||
return Size == 32 ? RISCV::FLE_S : RISCV::FLE_D; | ||
case CmpInst::FCMP_OEQ: | ||
return Size == 32 ? RISCV::FEQ_S : RISCV::FEQ_D; | ||
} | ||
} | ||
|
||
// Try legalizing an FCMP by swapping or inverting the predicate to one that | ||
// is supported. | ||
static bool legalizeFCmpPredicate(Register &LHS, Register &RHS, | ||
CmpInst::Predicate &Pred, bool &NeedInvert) { | ||
auto isLegalFCmpPredicate = [](CmpInst::Predicate Pred) { | ||
return Pred == CmpInst::FCMP_OLT || Pred == CmpInst::FCMP_OLE || | ||
Pred == CmpInst::FCMP_OEQ; | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you need to check the case where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. tablegen has patterns for oeq, olt, and ole so selectImpl gets it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. If that’s the case, then it’s up to you if you’d like to have the assert here. I made my initial comment thinking we had to handle it with custom code. |
||
|
||
assert(!isLegalFCmpPredicate(Pred) && "Predicate already legal?"); | ||
|
||
CmpInst::Predicate InvPred = CmpInst::getSwappedPredicate(Pred); | ||
if (isLegalFCmpPredicate(InvPred)) { | ||
Pred = InvPred; | ||
std::swap(LHS, RHS); | ||
return true; | ||
} | ||
|
||
InvPred = CmpInst::getInversePredicate(Pred); | ||
NeedInvert = true; | ||
if (isLegalFCmpPredicate(InvPred)) { | ||
Pred = InvPred; | ||
return true; | ||
} | ||
InvPred = CmpInst::getSwappedPredicate(InvPred); | ||
if (isLegalFCmpPredicate(InvPred)) { | ||
Pred = InvPred; | ||
std::swap(LHS, RHS); | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
// Emit a sequence of instructions to compare LHS and RHS using Pred. Return | ||
// the result in DstReg. | ||
// FIXME: Maybe we should expand this earlier. | ||
bool RISCVInstructionSelector::selectFPCompare(MachineInstr &MI, | ||
MachineIRBuilder &MIB, | ||
MachineRegisterInfo &MRI) const { | ||
auto &CmpMI = cast<GFCmp>(MI); | ||
CmpInst::Predicate Pred = CmpMI.getCond(); | ||
|
||
Register DstReg = CmpMI.getReg(0); | ||
Register LHS = CmpMI.getLHSReg(); | ||
Register RHS = CmpMI.getRHSReg(); | ||
|
||
unsigned Size = MRI.getType(LHS).getSizeInBits(); | ||
assert((Size == 32 || Size == 64) && "Unexpected size"); | ||
|
||
Register TmpReg = DstReg; | ||
|
||
bool NeedInvert = false; | ||
// First try swapping operands or inverting. | ||
if (legalizeFCmpPredicate(LHS, RHS, Pred, NeedInvert)) { | ||
if (NeedInvert) | ||
TmpReg = MRI.createVirtualRegister(&RISCV::GPRRegClass); | ||
auto Cmp = MIB.buildInstr(getFCmpOpcode(Pred, Size), {TmpReg}, {LHS, RHS}); | ||
michaelmaitland marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!Cmp.constrainAllUses(TII, TRI, RBI)) | ||
return false; | ||
} else if (Pred == CmpInst::FCMP_ONE || Pred == CmpInst::FCMP_UEQ) { | ||
// fcmp one LHS, RHS => (OR (FLT LHS, RHS), (FLT RHS, LHS)) | ||
NeedInvert = Pred == CmpInst::FCMP_UEQ; | ||
auto Cmp1 = MIB.buildInstr(getFCmpOpcode(CmpInst::FCMP_OLT, Size), | ||
{&RISCV::GPRRegClass}, {LHS, RHS}); | ||
if (!Cmp1.constrainAllUses(TII, TRI, RBI)) | ||
return false; | ||
auto Cmp2 = MIB.buildInstr(getFCmpOpcode(CmpInst::FCMP_OLT, Size), | ||
{&RISCV::GPRRegClass}, {RHS, LHS}); | ||
if (!Cmp2.constrainAllUses(TII, TRI, RBI)) | ||
return false; | ||
if (NeedInvert) | ||
TmpReg = MRI.createVirtualRegister(&RISCV::GPRRegClass); | ||
auto Or = | ||
MIB.buildInstr(RISCV::OR, {TmpReg}, {Cmp1.getReg(0), Cmp2.getReg(0)}); | ||
if (!Or.constrainAllUses(TII, TRI, RBI)) | ||
return false; | ||
} else if (Pred == CmpInst::FCMP_ORD || Pred == CmpInst::FCMP_UNO) { | ||
// fcmp ord LHS, RHS => (AND (FEQ LHS, LHS), (FEQ RHS, RHS)) | ||
// FIXME: If LHS and RHS are the same we can use a single FEQ. | ||
NeedInvert = Pred == CmpInst::FCMP_UNO; | ||
auto Cmp1 = MIB.buildInstr(getFCmpOpcode(CmpInst::FCMP_OEQ, Size), | ||
{&RISCV::GPRRegClass}, {LHS, LHS}); | ||
if (!Cmp1.constrainAllUses(TII, TRI, RBI)) | ||
return false; | ||
auto Cmp2 = MIB.buildInstr(getFCmpOpcode(CmpInst::FCMP_OEQ, Size), | ||
{&RISCV::GPRRegClass}, {RHS, RHS}); | ||
if (!Cmp2.constrainAllUses(TII, TRI, RBI)) | ||
return false; | ||
if (NeedInvert) | ||
TmpReg = MRI.createVirtualRegister(&RISCV::GPRRegClass); | ||
auto And = | ||
MIB.buildInstr(RISCV::AND, {TmpReg}, {Cmp1.getReg(0), Cmp2.getReg(0)}); | ||
if (!And.constrainAllUses(TII, TRI, RBI)) | ||
return false; | ||
} else | ||
llvm_unreachable("Unhandled predicate"); | ||
Comment on lines
+937
to
+938
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: add braces here to keep it uniform with prior if blocks |
||
|
||
// Emit an XORI to invert the result if needed. | ||
if (NeedInvert) { | ||
auto Xor = MIB.buildInstr(RISCV::XORI, {DstReg}, {TmpReg}).addImm(1); | ||
if (!Xor.constrainAllUses(TII, TRI, RBI)) | ||
return false; | ||
} | ||
|
||
MI.eraseFromParent(); | ||
return true; | ||
} | ||
|
||
namespace llvm { | ||
InstructionSelector * | ||
createRISCVInstructionSelector(const RISCVTargetMachine &TM, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kind of stuff should go in LegalizerHelper. We should be able to add the cond code to the legalizer queries for G_ICMP/G_FCMP and replicate the DAG's setCondCodeAction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be fine to claim support for all condition codes in the legalizer. Do target independent combines and later down the pass pipeline, they can do RISCV-V specific optimizations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My immediate goal was just to get -O0 working. I will start looking at how to put this in LegalizerHelper, but I don't want that to block this patch if possible.