Skip to content

Commit 198626a

Browse files
sdardiststellar
authored andcommitted
[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 (cherry picked from commit 938ed8a)
1 parent b75bf75 commit 198626a

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
@@ -192,6 +192,7 @@ const char *MipsTargetLowering::getTargetNodeName(unsigned Opcode) const {
192192
case MipsISD::Ret: return "MipsISD::Ret";
193193
case MipsISD::ERet: return "MipsISD::ERet";
194194
case MipsISD::EH_RETURN: return "MipsISD::EH_RETURN";
195+
case MipsISD::FAbs: return "MipsISD::FAbs";
195196
case MipsISD::FMS: return "MipsISD::FMS";
196197
case MipsISD::FPBrcond: return "MipsISD::FPBrcond";
197198
case MipsISD::FPCmp: return "MipsISD::FPCmp";
@@ -353,15 +354,12 @@ MipsTargetLowering::MipsTargetLowering(const MipsTargetMachine &TM,
353354
setOperationAction(ISD::SETCC, MVT::f32, Custom);
354355
setOperationAction(ISD::SETCC, MVT::f64, Custom);
355356
setOperationAction(ISD::BRCOND, MVT::Other, Custom);
357+
setOperationAction(ISD::FABS, MVT::f32, Custom);
358+
setOperationAction(ISD::FABS, MVT::f64, Custom);
356359
setOperationAction(ISD::FCOPYSIGN, MVT::f32, Custom);
357360
setOperationAction(ISD::FCOPYSIGN, MVT::f64, Custom);
358361
setOperationAction(ISD::FP_TO_SINT, MVT::i32, Custom);
359362

360-
if (!(TM.Options.NoNaNsFPMath || Subtarget.inAbs2008Mode())) {
361-
setOperationAction(ISD::FABS, MVT::f32, Custom);
362-
setOperationAction(ISD::FABS, MVT::f64, Custom);
363-
}
364-
365363
if (Subtarget.isGP64bit()) {
366364
setOperationAction(ISD::GlobalAddress, MVT::i64, Custom);
367365
setOperationAction(ISD::BlockAddress, MVT::i64, Custom);
@@ -2421,11 +2419,14 @@ MipsTargetLowering::lowerFCOPYSIGN(SDValue Op, SelectionDAG &DAG) const {
24212419
return lowerFCOPYSIGN32(Op, DAG, Subtarget.hasExtractInsert());
24222420
}
24232421

2424-
static SDValue lowerFABS32(SDValue Op, SelectionDAG &DAG,
2425-
bool HasExtractInsert) {
2422+
SDValue MipsTargetLowering::lowerFABS32(SDValue Op, SelectionDAG &DAG,
2423+
bool HasExtractInsert) const {
24262424
SDLoc DL(Op);
24272425
SDValue Res, Const1 = DAG.getConstant(1, DL, MVT::i32);
24282426

2427+
if (DAG.getTarget().Options.NoNaNsFPMath || Subtarget.inAbs2008Mode())
2428+
return DAG.getNode(MipsISD::FAbs, DL, Op.getValueType(), Op.getOperand(0));
2429+
24292430
// If operand is of type f64, extract the upper 32-bit. Otherwise, bitcast it
24302431
// to i32.
24312432
SDValue X = (Op.getValueType() == MVT::f32)
@@ -2458,11 +2459,14 @@ static SDValue lowerFABS32(SDValue Op, SelectionDAG &DAG,
24582459
return DAG.getNode(MipsISD::BuildPairF64, DL, MVT::f64, LowX, Res);
24592460
}
24602461

2461-
static SDValue lowerFABS64(SDValue Op, SelectionDAG &DAG,
2462-
bool HasExtractInsert) {
2462+
SDValue MipsTargetLowering::lowerFABS64(SDValue Op, SelectionDAG &DAG,
2463+
bool HasExtractInsert) const {
24632464
SDLoc DL(Op);
24642465
SDValue Res, Const1 = DAG.getConstant(1, DL, MVT::i32);
24652466

2467+
if (DAG.getTarget().Options.NoNaNsFPMath || Subtarget.inAbs2008Mode())
2468+
return DAG.getNode(MipsISD::FAbs, DL, Op.getValueType(), Op.getOperand(0));
2469+
24662470
// Bitcast to integer node.
24672471
SDValue X = DAG.getNode(ISD::BITCAST, DL, MVT::i64, Op.getOperand(0));
24682472

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

@@ -540,6 +543,10 @@ class TargetRegisterClass;
540543
SDValue lowerVAARG(SDValue Op, SelectionDAG &DAG) const;
541544
SDValue lowerFCOPYSIGN(SDValue Op, SelectionDAG &DAG) const;
542545
SDValue lowerFABS(SDValue Op, SelectionDAG &DAG) const;
546+
SDValue lowerFABS32(SDValue Op, SelectionDAG &DAG,
547+
bool HasExtractInsert) const;
548+
SDValue lowerFABS64(SDValue Op, SelectionDAG &DAG,
549+
bool HasExtractInsert) const;
543550
SDValue lowerFRAMEADDR(SDValue Op, SelectionDAG &DAG) const;
544551
SDValue lowerRETURNADDR(SDValue Op, SelectionDAG &DAG) const;
545552
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)