[RISC-V] Do not crash when using -ftrapping-math

[kamleshbhalui]

Summary

When using clang option -frapping-math, RISC-V backend crash.
This patch enable Lowering for strictp in riscv to get rid of crash.

[luismarques]

llvm/test/CodeGen/RISCV/fp-strict.ll

2–5

It's probably a good idea to use the hardfloat ABI, to cut down on the amount of GPR to FPR conversion instructions, etc.

131

Shouldn't this test also have an fcmps oeq case, showing the proper behavior in terms of NaN signaling, by using the correct RISC-V instruction for that? I haven't yet delved into the lowerSETCC implementation but it seems that having such tests here would clarify if this is correctly implemented.

[kamleshbhalui]

llvm/test/CodeGen/RISCV/fp-strict.ll

2–5

thanks , will consider it in next diff update.

[asb]

Disclaimer: I haven't swotted up on these constrained intrinsics to review the proposed lowering yet, but added a quick note on the setOperationAction calls. I agree with Luis the test cases would probably be easier to read if using the hard float ABI. I think you're lacking any test coverage for f32 as well.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

214

Should the instances of MVT::i64 be XLenVT?

I also would have imagined it would make sense for these setOperationAction calls to be guarded on hasStdExtF/hasStdExtD appropriately. Is that not the case here?

[kamleshbhalui]

enhanced and more testcases.

[kamleshbhalui]

ping?

[Jim]

llvm/lib/Target/RISCV/RISCVISelLowering.h

212

Don't delete blank line here.

[kamleshbhalui]

llvm/lib/Target/RISCV/RISCVISelLowering.h

212

ok, will fix it in next update

[kamleshbhalui]

@asb and @luismarques do you suggest anything here ?

[lenary]

You will need to rebase past D80952, which made constrained fp a target opt-in - though in all honesty, with that patch, this patch is less of a priority as the compiler will not crash any more.

I still don't know how we (the reviewers) should work out whether the lowering of the constrained fp operators is "correct" or not, which is what's holding up this patch anyway.

[kamleshbhalui]

rebased with the master.

llvm/lib/Target/RISCV/RISCVISelLowering.h

private:
   SDValue lowerGlobalTLSAddress(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerSELECT(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerVASTART(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerFRAMEADDR(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerRETURNADDR(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerShiftLeftParts(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerShiftRightParts(SDValue Op, SelectionDAG &DAG, bool IsSRA) const;
   SDValue LowerINTRINSIC_WO_CHAIN(SDValue Op, SelectionDAG &DAG) const;
-
+  SDValue lowerSETCC(SDValue Op, SelectionDAG &DAG) const;
   bool isEligibleForTailCallOptimization(
       CCState &CCInfo, CallLoweringInfo &CLI, MachineFunction &MF,
       const SmallVector<CCValAssign, 16> &ArgLocs) const;
 
   TargetLowering::AtomicExpansionKind
   shouldExpandAtomicRMWInIR(AtomicRMWInst *AI) const override;
   virtual Value *emitMaskedAtomicRMWIntrinsic(
       IRBuilder<> &Builder, AtomicRMWInst *AI, Value *AlignedAddr, Value *Incr,

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
   // TODO: On M-mode only targets, the cycle[h] CSR may not be present.
   // Unfortunately this can't be determined just from the ISA naming string.
   setOperationAction(ISD::READCYCLECOUNTER, MVT::i64,
                      Subtarget.is64Bit() ? Legal : Custom);
 
   setOperationAction(ISD::TRAP, MVT::Other, Legal);
   setOperationAction(ISD::DEBUGTRAP, MVT::Other, Legal);
   setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::Other, Custom);
+  setOperationAction(ISD::STRICT_SINT_TO_FP, MVT::i64, Legal);
+  setOperationAction(ISD::STRICT_UINT_TO_FP, MVT::i64, Legal);
+  setOperationAction(ISD::STRICT_FSETCC, MVT::f64, Custom);
+  setOperationAction(ISD::STRICT_FSETCC, MVT::f32, Custom);
+  setOperationAction(ISD::STRICT_FSETCCS, MVT::f64, Custom);
+  setOperationAction(ISD::STRICT_FSETCCS, MVT::f32, Custom);
 
   if (Subtarget.hasStdExtA()) {
     setMaxAtomicSizeInBitsSupported(Subtarget.getXLen());
     setMinCmpXchgSizeInBits(32);
   } else {
     setMaxAtomicSizeInBitsSupported(0);
   }
 
case ISD::BITCAST: {
     if (Op.getValueType() != MVT::f32 || Op0.getValueType() != MVT::i32)
       return SDValue();
     SDValue NewOp0 = DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, Op0);
     SDValue FPConv = DAG.getNode(RISCVISD::FMV_W_X_RV64, DL, MVT::f32, NewOp0);
     return FPConv;
   }
   case ISD::INTRINSIC_WO_CHAIN:
     return LowerINTRINSIC_WO_CHAIN(Op, DAG);
+  case ISD::STRICT_FSETCC:
+  case ISD::STRICT_FSETCCS:
+    return lowerSETCC(Op, DAG);
   }
 }
 
 static SDValue getTargetNode(GlobalAddressSDNode *N, SDLoc DL, EVT Ty,
                              SelectionDAG &DAG, unsigned Flags) {
   return DAG.getTargetGlobalAddress(N->getGlobal(), DL, Ty, 0, Flags);
 }
 
SDValue RISCVTargetLowering::lowerShiftRightParts(SDValue Op, SelectionDAG &DAG,
 
   Lo = DAG.getNode(ISD::SELECT, DL, VT, CC, LoTrue, LoFalse);
   Hi = DAG.getNode(ISD::SELECT, DL, VT, CC, HiTrue, HiFalse);
 
   SDValue Parts[2] = {Lo, Hi};
   return DAG.getMergeValues(Parts, DL);
 }
 
+SDValue RISCVTargetLowering::lowerSETCC(SDValue Op, SelectionDAG &DAG) const {
+  SDLoc DL(Op);
+  EVT VT = Op.getValueType();
+  SDValue Chain = Op.getOperand(0);
+  SDValue LHS = Op.getOperand(1);
+  SDValue RHS = Op.getOperand(2);
+  ISD::CondCode CC = cast<CondCodeSDNode>(Op.getOperand(3))->get();
+  SDValue Result =
+      DAG.getNode(ISD::SETCC, DL, VT, LHS, RHS, DAG.getCondCode(CC));
+  return DAG.getMergeValues({Result, Chain}, DL);
+}
+
 SDValue RISCVTargetLowering::LowerINTRINSIC_WO_CHAIN(SDValue Op,
                                                      SelectionDAG &DAG) const {
   unsigned IntNo = cast<ConstantSDNode>(Op.getOperand(0))->getZExtValue();
   SDLoc DL(Op);
   switch (IntNo) {
   default:
     return SDValue();    // Don't custom lower most intrinsics.
   case Intrinsic::thread_pointer: {

llvm/lib/Target/RISCV/RISCVInstrInfoD.td

 /// Float constants
 def : Pat<(f64 (fpimm0)), (FCVT_D_W X0)>;
 
 // double->[u]int. Round-to-zero must be used.
 def : Pat<(fp_to_sint FPR64:$rs1), (FCVT_W_D FPR64:$rs1, 0b001)>;
 def : Pat<(fp_to_uint FPR64:$rs1), (FCVT_WU_D FPR64:$rs1, 0b001)>;
 
 // [u]int->double.
-def : Pat<(sint_to_fp GPR:$rs1), (FCVT_D_W GPR:$rs1)>;
-def : Pat<(uint_to_fp GPR:$rs1), (FCVT_D_WU GPR:$rs1)>;
+def : Pat<(any_sint_to_fp GPR:$rs1), (FCVT_D_W GPR:$rs1)>;
+def : Pat<(any_uint_to_fp GPR:$rs1), (FCVT_D_WU GPR:$rs1)>;
 } // Predicates = [HasStdExtD, IsRV32]
 
 let Predicates = [HasStdExtD, IsRV64] in {
 
 /// Float constants
 def : Pat<(f64 (fpimm0)), (FMV_D_X X0)>;
 
 def : Pat<(bitconvert GPR:$rs1), (FMV_D_X GPR:$rs1)>;
 def : Pat<(bitconvert FPR64:$rs1), (FMV_X_D FPR64:$rs1)>;
 
 // FP->[u]int32 is mostly handled by the FP->[u]int64 patterns. This is safe
 // because fpto[u|s]i produce poison if the value can't fit into the target.
 // We match the single case below because fcvt.wu.d sign-extends its result so
 // is cheaper than fcvt.lu.d+sext.w.
 def : Pat<(sext_inreg (zexti32 (fp_to_uint FPR64:$rs1)), i32),
           (FCVT_WU_D $rs1, 0b001)>;
 
 // [u]int32->fp
-def : Pat<(sint_to_fp (sext_inreg GPR:$rs1, i32)), (FCVT_D_W $rs1)>;
-def : Pat<(uint_to_fp (zexti32 GPR:$rs1)), (FCVT_D_WU $rs1)>;
+def : Pat<(any_sint_to_fp (sext_inreg GPR:$rs1, i32)), (FCVT_D_W $rs1)>;
+def : Pat<(any_uint_to_fp (zexti32 GPR:$rs1)), (FCVT_D_WU $rs1)>;
 
 def : Pat<(fp_to_sint FPR64:$rs1), (FCVT_L_D FPR64:$rs1, 0b001)>;
 def : Pat<(fp_to_uint FPR64:$rs1), (FCVT_LU_D FPR64:$rs1, 0b001)>;
 
 // [u]int64->fp. Match GCC and default to using dynamic rounding mode.
-def : Pat<(sint_to_fp GPR:$rs1), (FCVT_D_L GPR:$rs1, 0b111)>;
-def : Pat<(uint_to_fp GPR:$rs1), (FCVT_D_LU GPR:$rs1, 0b111)>;
+def : Pat<(any_sint_to_fp GPR:$rs1), (FCVT_D_L GPR:$rs1, 0b111)>;
+def : Pat<(any_uint_to_fp GPR:$rs1), (FCVT_D_LU GPR:$rs1, 0b111)>;
 } // Predicates = [HasStdExtD, IsRV64]

llvm/lib/Target/RISCV/RISCVInstrInfoF.td

 } // Predicates = [HasStdExtF]
 
 let Predicates = [HasStdExtF, IsRV32] in {
 // float->[u]int. Round-to-zero must be used.
 def : Pat<(fp_to_sint FPR32:$rs1), (FCVT_W_S $rs1, 0b001)>;
 def : Pat<(fp_to_uint FPR32:$rs1), (FCVT_WU_S $rs1, 0b001)>;
 
 // [u]int->float. Match GCC and default to using dynamic rounding mode.
-def : Pat<(sint_to_fp GPR:$rs1), (FCVT_S_W $rs1, 0b111)>;
-def : Pat<(uint_to_fp GPR:$rs1), (FCVT_S_WU $rs1, 0b111)>;
+def : Pat<(any_sint_to_fp GPR:$rs1), (FCVT_S_W $rs1, 0b111)>;
+def : Pat<(any_uint_to_fp GPR:$rs1), (FCVT_S_WU $rs1, 0b111)>;
 } // Predicates = [HasStdExtF, IsRV32]
 
 let Predicates = [HasStdExtF, IsRV64] in {
 def : Pat<(riscv_fmv_w_x_rv64 GPR:$src), (FMV_W_X GPR:$src)>;
 def : Pat<(riscv_fmv_x_anyextw_rv64 FPR32:$src), (FMV_X_W FPR32:$src)>;
 def : Pat<(sexti32 (riscv_fmv_x_anyextw_rv64 FPR32:$src)),
           (FMV_X_W FPR32:$src)>;
 
 // FP->[u]int32 is mostly handled by the FP->[u]int64 patterns. This is safe
 // because fpto[u|s]i produces poison if the value can't fit into the target.
 // We match the single case below because fcvt.wu.s sign-extends its result so
 // is cheaper than fcvt.lu.s+sext.w.
 def : Pat<(sext_inreg (assertzexti32 (fp_to_uint FPR32:$rs1)), i32),
           (FCVT_WU_S $rs1, 0b001)>;
 
 // FP->[u]int64
 def : Pat<(fp_to_sint FPR32:$rs1), (FCVT_L_S $rs1, 0b001)>;
 def : Pat<(fp_to_uint FPR32:$rs1), (FCVT_LU_S $rs1, 0b001)>;
 
 // [u]int->fp. Match GCC and default to using dynamic rounding mode.
-def : Pat<(sint_to_fp (sext_inreg GPR:$rs1, i32)), (FCVT_S_W $rs1, 0b111)>;
-def : Pat<(uint_to_fp (zexti32 GPR:$rs1)), (FCVT_S_WU $rs1, 0b111)>;
-def : Pat<(sint_to_fp GPR:$rs1), (FCVT_S_L $rs1, 0b111)>;
-def : Pat<(uint_to_fp GPR:$rs1), (FCVT_S_LU $rs1, 0b111)>;
+def : Pat<(any_sint_to_fp (sext_inreg GPR:$rs1, i32)), (FCVT_S_W $rs1, 0b111)>;
+def : Pat<(any_uint_to_fp (zexti32 GPR:$rs1)), (FCVT_S_WU $rs1, 0b111)>;
+def : Pat<(any_sint_to_fp GPR:$rs1), (FCVT_S_L $rs1, 0b111)>;
+def : Pat<(any_uint_to_fp GPR:$rs1), (FCVT_S_LU $rs1, 0b111)>;
 } // Predicates = [HasStdExtF, IsRV64]

llvm/test/CodeGen/RISCV/fp-strict.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -O1 -mtriple=riscv64 -mattr="+d" -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64D
+; RUN: llc  -O1 -mtriple=riscv32 -mattr="+d" -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32D
+
+declare i1 @llvm.experimental.constrained.fcmps.f64(double, double, metadata, metadata)
+declare i1 @llvm.experimental.constrained.fcmp.f64(double, double, metadata, metadata)
+
+define i1 @f_lt(double %a, double %b) strictfp nounwind {
+; RV64D-LABEL: f_lt:
+; RV64D:       # %bb.0: # %entry
+; RV64D-NEXT:    fmv.d.x ft0, a1
+; RV64D-NEXT:    fmv.d.x ft1, a0
+; RV64D-NEXT:    flt.d a0, ft1, ft0
+; RV64D-NEXT:    ret
+;
+; RV32D-LABEL: f_lt:
+; RV32D:       # %bb.0: # %entry
+; RV32D-NEXT:    addi sp, sp, -16
+; RV32D-NEXT:    sw a2, 8(sp)
+; RV32D-NEXT:    sw a3, 12(sp)
+; RV32D-NEXT:    fld ft0, 8(sp)
+; RV32D-NEXT:    sw a0, 8(sp)
+; RV32D-NEXT:    sw a1, 12(sp)
+; RV32D-NEXT:    fld ft1, 8(sp)
+; RV32D-NEXT:    flt.d a0, ft1, ft0
+; RV32D-NEXT:    addi sp, sp, 16
+; RV32D-NEXT:    ret
+entry:
+  %cmp = call i1 @llvm.experimental.constrained.fcmps.f64(double %a, double %b, metadata !"olt", metadata !"fpexcept.strict")
+  ret i1 %cmp
+}
+
+define i1 @f_leq(double %a, double %b) strictfp nounwind {
+; RV64D-LABEL: f_leq:
+; RV64D:       # %bb.0: # %entry
+; RV64D-NEXT:    fmv.d.x ft0, a1
+; RV64D-NEXT:    fmv.d.x ft1, a0
+; RV64D-NEXT:    fle.d a0, ft1, ft0
+; RV64D-NEXT:    ret
+;
+; RV32D-LABEL: f_leq:
+; RV32D:       # %bb.0: # %entry
+; RV32D-NEXT:    addi sp, sp, -16
+; RV32D-NEXT:    sw a2, 8(sp)
+; RV32D-NEXT:    sw a3, 12(sp)
+; RV32D-NEXT:    fld ft0, 8(sp)
+; RV32D-NEXT:    sw a0, 8(sp)
+; RV32D-NEXT:    sw a1, 12(sp)
+; RV32D-NEXT:    fld ft1, 8(sp)
+; RV32D-NEXT:    fle.d a0, ft1, ft0
+; RV32D-NEXT:    addi sp, sp, 16
+; RV32D-NEXT:    ret
+entry:
+  %cmp = call i1 @llvm.experimental.constrained.fcmps.f64(double %a, double %b, metadata !"ole", metadata !"fpexcept.strict")
+  ret i1 %cmp
+}
+
+define i1 @f_gt(double %a, double %b) strictfp nounwind {
+; RV64D-LABEL: f_gt:
+; RV64D:       # %bb.0: # %entry
+; RV64D-NEXT:    fmv.d.x ft0, a0
+; RV64D-NEXT:    fmv.d.x ft1, a1
+; RV64D-NEXT:    flt.d a0, ft1, ft0
+; RV64D-NEXT:    ret
+;
+; RV32D-LABEL: f_gt:
+; RV32D:       # %bb.0: # %entry
+; RV32D-NEXT:    addi sp, sp, -16
+; RV32D-NEXT:    sw a0, 8(sp)
+; RV32D-NEXT:    sw a1, 12(sp)
+; RV32D-NEXT:    fld ft0, 8(sp)
+; RV32D-NEXT:    sw a2, 8(sp)
+; RV32D-NEXT:    sw a3, 12(sp)
+; RV32D-NEXT:    fld ft1, 8(sp)
+; RV32D-NEXT:    flt.d a0, ft1, ft0
+; RV32D-NEXT:    addi sp, sp, 16
+; RV32D-NEXT:    ret
+entry:
+  %cmp = call i1 @llvm.experimental.constrained.fcmps.f64(double %a, double %b, metadata !"ogt", metadata !"fpexcept.strict")
+  ret i1 %cmp
+}
+
+define i1 @f_geq(double %a, double %b) strictfp nounwind {
+; RV64D-LABEL: f_geq:
+; RV64D:       # %bb.0: # %entry
+; RV64D-NEXT:    fmv.d.x ft0, a0
+; RV64D-NEXT:    fmv.d.x ft1, a1
+; RV64D-NEXT:    fle.d a0, ft1, ft0
+; RV64D-NEXT:    ret
+;
+; RV32D-LABEL: f_geq:
+; RV32D:       # %bb.0: # %entry
+; RV32D-NEXT:    addi sp, sp, -16
+; RV32D-NEXT:    sw a0, 8(sp)
+; RV32D-NEXT:    sw a1, 12(sp)
+; RV32D-NEXT:    fld ft0, 8(sp)
+; RV32D-NEXT:    sw a2, 8(sp)
+; RV32D-NEXT:    sw a3, 12(sp)
+; RV32D-NEXT:    fld ft1, 8(sp)
+; RV32D-NEXT:    fle.d a0, ft1, ft0
+; RV32D-NEXT:    addi sp, sp, 16
+; RV32D-NEXT:    ret
+entry:
+  %cmp = call i1 @llvm.experimental.constrained.fcmps.f64(double %a, double %b, metadata !"oge", metadata !"fpexcept.strict")
+  ret i1 %cmp
+}
+
+define i1 @f_eq(double %a, double %b) strictfp nounwind {
+; RV64D-LABEL: f_eq:
+; RV64D:       # %bb.0: # %entry
+; RV64D-NEXT:    fmv.d.x ft0, a1
+; RV64D-NEXT:    fmv.d.x ft1, a0
+; RV64D-NEXT:    feq.d a0, ft1, ft0
+; RV64D-NEXT:    ret
+;
+; RV32D-LABEL: f_eq:
+; RV32D:       # %bb.0: # %entry
+; RV32D-NEXT:    addi sp, sp, -16
+; RV32D-NEXT:    sw a2, 8(sp)
+; RV32D-NEXT:    sw a3, 12(sp)
+; RV32D-NEXT:    fld ft0, 8(sp)
+; RV32D-NEXT:    sw a0, 8(sp)
+; RV32D-NEXT:    sw a1, 12(sp)
+; RV32D-NEXT:    fld ft1, 8(sp)
+; RV32D-NEXT:    feq.d a0, ft1, ft0
+; RV32D-NEXT:    addi sp, sp, 16
+; RV32D-NEXT:    ret
+entry:
+  %cmp = call i1 @llvm.experimental.constrained.fcmp.f64(double %a, double %b, metadata !"oeq", metadata !"fpexcept.strict")
+  ret i1 %cmp
+}
+
+define i1 @f_neq(double %a, double %b) strictfp nounwind  {
+; RV64D-LABEL: f_neq:
+; RV64D:       # %bb.0: # %entry
+; RV64D-NEXT:    fmv.d.x ft0, a1
+; RV64D-NEXT:    fmv.d.x ft1, a0
+; RV64D-NEXT:    feq.d a0, ft1, ft0
+; RV64D-NEXT:    xori a0, a0, 1
+; RV64D-NEXT:    ret
+;
+; RV32D-LABEL: f_neq:
+; RV32D:       # %bb.0: # %entry
+; RV32D-NEXT:    addi sp, sp, -16
+; RV32D-NEXT:    sw a2, 8(sp)
+; RV32D-NEXT:    sw a3, 12(sp)
+; RV32D-NEXT:    fld ft0, 8(sp)
+; RV32D-NEXT:    sw a0, 8(sp)
+; RV32D-NEXT:    sw a1, 12(sp)
+; RV32D-NEXT:    fld ft1, 8(sp)
+; RV32D-NEXT:    feq.d a0, ft1, ft0
+; RV32D-NEXT:    xori a0, a0, 1
+; RV32D-NEXT:    addi sp, sp, 16
+; RV32D-NEXT:    ret
+entry:
+  %cmp = call i1 @llvm.experimental.constrained.fcmp.f64(double %a, double %b, metadata !"une", metadata !"fpexcept.strict")
+  ret i1 %cmp
+}
+