Skip to content

Commit 938ed8a

Browse files
committed
[MIPS] Address instruction selection failure for abs.[sd]
Previously, the choice between the instruction selection of ISD::FABS was decided at the point of setting the MIPS target lowering operation choice either `Custom` lowering or `Legal`. This lead to instruction selection failures as functions could be marked as having no NaNs. Changing the lowering to always be `Custom` and directly handling the the cases where MIPS selects the instructions for ISD::FABS resolves this crash. Thanks to kray for reporting the issue and to Simon Atanasyan for producing the reduced test case. This resolves PR/53722. Differential Revision: https://reviews.llvm.org/D124651
1 parent 9c8179f commit 938ed8a

File tree

4 files changed

+379
-9
lines changed

4 files changed

+379
-9
lines changed

llvm/lib/Target/Mips/MipsISelLowering.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ const char *MipsTargetLowering::getTargetNodeName(unsigned Opcode) const {
180180
case MipsISD::Ret: return "MipsISD::Ret";
181181
case MipsISD::ERet: return "MipsISD::ERet";
182182
case MipsISD::EH_RETURN: return "MipsISD::EH_RETURN";
183+
case MipsISD::FAbs: return "MipsISD::FAbs";
183184
case MipsISD::FMS: return "MipsISD::FMS";
184185
case MipsISD::FPBrcond: return "MipsISD::FPBrcond";
185186
case MipsISD::FPCmp: return "MipsISD::FPCmp";
@@ -341,15 +342,12 @@ MipsTargetLowering::MipsTargetLowering(const MipsTargetMachine &TM,
341342
setOperationAction(ISD::SETCC, MVT::f32, Custom);
342343
setOperationAction(ISD::SETCC, MVT::f64, Custom);
343344
setOperationAction(ISD::BRCOND, MVT::Other, Custom);
345+
setOperationAction(ISD::FABS, MVT::f32, Custom);
346+
setOperationAction(ISD::FABS, MVT::f64, Custom);
344347
setOperationAction(ISD::FCOPYSIGN, MVT::f32, Custom);
345348
setOperationAction(ISD::FCOPYSIGN, MVT::f64, Custom);
346349
setOperationAction(ISD::FP_TO_SINT, MVT::i32, Custom);
347350

348-
if (!(TM.Options.NoNaNsFPMath || Subtarget.inAbs2008Mode())) {
349-
setOperationAction(ISD::FABS, MVT::f32, Custom);
350-
setOperationAction(ISD::FABS, MVT::f64, Custom);
351-
}
352-
353351
if (Subtarget.isGP64bit()) {
354352
setOperationAction(ISD::GlobalAddress, MVT::i64, Custom);
355353
setOperationAction(ISD::BlockAddress, MVT::i64, Custom);
@@ -2414,11 +2412,14 @@ MipsTargetLowering::lowerFCOPYSIGN(SDValue Op, SelectionDAG &DAG) const {
24142412
return lowerFCOPYSIGN32(Op, DAG, Subtarget.hasExtractInsert());
24152413
}
24162414

2417-
static SDValue lowerFABS32(SDValue Op, SelectionDAG &DAG,
2418-
bool HasExtractInsert) {
2415+
SDValue MipsTargetLowering::lowerFABS32(SDValue Op, SelectionDAG &DAG,
2416+
bool HasExtractInsert) const {
24192417
SDLoc DL(Op);
24202418
SDValue Res, Const1 = DAG.getConstant(1, DL, MVT::i32);
24212419

2420+
if (DAG.getTarget().Options.NoNaNsFPMath || Subtarget.inAbs2008Mode())
2421+
return DAG.getNode(MipsISD::FAbs, DL, Op.getValueType(), Op.getOperand(0));
2422+
24222423
// If operand is of type f64, extract the upper 32-bit. Otherwise, bitcast it
24232424
// to i32.
24242425
SDValue X = (Op.getValueType() == MVT::f32)
@@ -2451,11 +2452,14 @@ static SDValue lowerFABS32(SDValue Op, SelectionDAG &DAG,
24512452
return DAG.getNode(MipsISD::BuildPairF64, DL, MVT::f64, LowX, Res);
24522453
}
24532454

2454-
static SDValue lowerFABS64(SDValue Op, SelectionDAG &DAG,
2455-
bool HasExtractInsert) {
2455+
SDValue MipsTargetLowering::lowerFABS64(SDValue Op, SelectionDAG &DAG,
2456+
bool HasExtractInsert) const {
24562457
SDLoc DL(Op);
24572458
SDValue Res, Const1 = DAG.getConstant(1, DL, MVT::i32);
24582459

2460+
if (DAG.getTarget().Options.NoNaNsFPMath || Subtarget.inAbs2008Mode())
2461+
return DAG.getNode(MipsISD::FAbs, DL, Op.getValueType(), Op.getOperand(0));
2462+
24592463
// Bitcast to integer node.
24602464
SDValue X = DAG.getNode(ISD::BITCAST, DL, MVT::i64, Op.getOperand(0));
24612465

llvm/lib/Target/Mips/MipsISelLowering.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ class TargetRegisterClass;
9999
// Floating Point Compare
100100
FPCmp,
101101

102+
// Floating point Abs
103+
FAbs,
104+
102105
// Floating point select
103106
FSELECT,
104107

@@ -541,6 +544,10 @@ class TargetRegisterClass;
541544
SDValue lowerVAARG(SDValue Op, SelectionDAG &DAG) const;
542545
SDValue lowerFCOPYSIGN(SDValue Op, SelectionDAG &DAG) const;
543546
SDValue lowerFABS(SDValue Op, SelectionDAG &DAG) const;
547+
SDValue lowerFABS32(SDValue Op, SelectionDAG &DAG,
548+
bool HasExtractInsert) const;
549+
SDValue lowerFABS64(SDValue Op, SelectionDAG &DAG,
550+
bool HasExtractInsert) const;
544551
SDValue lowerFRAMEADDR(SDValue Op, SelectionDAG &DAG) const;
545552
SDValue lowerRETURNADDR(SDValue Op, SelectionDAG &DAG) const;
546553
SDValue lowerEH_RETURN(SDValue Op, SelectionDAG &DAG) const;

llvm/lib/Target/Mips/MipsSEISelDAGToDAG.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -956,6 +956,38 @@ bool MipsSEDAGToDAGISel::trySelect(SDNode *Node) {
956956
break;
957957
}
958958

959+
case MipsISD::FAbs: {
960+
MVT ResTy = Node->getSimpleValueType(0);
961+
assert((ResTy == MVT::f64 || ResTy == MVT::f32) &&
962+
"Unsupported float type!");
963+
unsigned Opc = 0;
964+
if (ResTy == MVT::f64)
965+
Opc = (Subtarget->isFP64bit() ? Mips::FABS_D64 : Mips::FABS_D32);
966+
else
967+
Opc = Mips::FABS_S;
968+
969+
if (Subtarget->inMicroMipsMode()) {
970+
switch (Opc) {
971+
case Mips::FABS_D64:
972+
Opc = Mips::FABS_D64_MM;
973+
break;
974+
case Mips::FABS_D32:
975+
Opc = Mips::FABS_D32_MM;
976+
break;
977+
case Mips::FABS_S:
978+
Opc = Mips::FABS_S_MM;
979+
break;
980+
default:
981+
llvm_unreachable("Unknown opcode for MIPS floating point abs!");
982+
}
983+
}
984+
985+
ReplaceNode(Node,
986+
CurDAG->getMachineNode(Opc, DL, ResTy, Node->getOperand(0)));
987+
988+
return true;
989+
}
990+
959991
// Manually match MipsISD::Ins nodes to get the correct instruction. It has
960992
// to be done in this fashion so that we respect the differences between
961993
// dins and dinsm, as the difference is that the size operand has the range

0 commit comments

Comments
 (0)