[llvm] 5224bae - [RISCV] Fix a bug in i32 FP_TO_UINT_SAT lowering on RV64.

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 13 08:45:54 PDT 2022


Author: Craig Topper
Date: 2022-09-13T08:41:32-07:00
New Revision: 5224bae6130722dd41561422df1133eab60eb969

URL: https://github.com/llvm/llvm-project/commit/5224bae6130722dd41561422df1133eab60eb969
DIFF: https://github.com/llvm/llvm-project/commit/5224bae6130722dd41561422df1133eab60eb969.diff

LOG: [RISCV] Fix a bug in i32 FP_TO_UINT_SAT lowering on RV64.

We use the saturating behavior of fcvt.wu.h/s/d but forgot to
take into account that fcvt.wu will sign extend the saturated
result. According to computeKnownBits a promoted FP_TO_UINT_SAT
is expected to zero extend the saturated value.

In many case the upper bits aren't be demanded so this wouldn't
be an issue. But if we computeKnownBits caused an AND to be removed
it would be a bug.

This patch inserts an AND during to zero the upper bits.

Unfortunately, this pessimizes code if we aren't able to tell if
the upper bits are demanded. To fix that we could custom type
promote the FP_TO_UINT_SAT with SEXT_INREG after it, but I'll
leave that for future work.

I haven't found a failure from this, I was revisiting the code to
add vector support and spotted it.

Differential Revision: https://reviews.llvm.org/D133746

Added: 
    

Modified: 
    llvm/lib/Target/RISCV/RISCVISelLowering.cpp
    llvm/test/CodeGen/RISCV/double-convert.ll
    llvm/test/CodeGen/RISCV/float-convert.ll
    llvm/test/CodeGen/RISCV/half-convert.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index e621c3455050a..891af62c8b4f3 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -1837,6 +1837,11 @@ static SDValue lowerFP_TO_INT_SAT(SDValue Op, SelectionDAG &DAG,
       Opc, DL, DstVT, Src,
       DAG.getTargetConstant(RISCVFPRndMode::RTZ, DL, Subtarget.getXLenVT()));
 
+  // fcvt.wu.* sign extends bit 31 on RV64. FP_TO_UINT_SAT expects to zero
+  // extend.
+  if (Opc == RISCVISD::FCVT_WU_RV64)
+    FpToInt = DAG.getZeroExtendInReg(FpToInt, DL, MVT::i32);
+
   SDValue ZeroInt = DAG.getConstant(0, DL, DstVT);
   return DAG.getSelectCC(DL, Src, Src, ZeroInt, FpToInt, ISD::CondCode::SETUO);
 }
@@ -8893,6 +8898,11 @@ static SDValue performFP_TO_INT_SATCombine(SDNode *N,
   SDValue FpToInt = DAG.getNode(Opc, DL, XLenVT, Src,
                                 DAG.getTargetConstant(FRM, DL, XLenVT));
 
+  // fcvt.wu.* sign extends bit 31 on RV64. FP_TO_UINT_SAT expects to zero
+  // extend.
+  if (Opc == RISCVISD::FCVT_WU_RV64)
+    FpToInt = DAG.getZeroExtendInReg(FpToInt, DL, MVT::i32);
+
   // RISCV FP-to-int conversions saturate to the destination register size, but
   // don't produce 0 for nan.
   SDValue ZeroInt = DAG.getConstant(0, DL, DstVT);

diff  --git a/llvm/test/CodeGen/RISCV/double-convert.ll b/llvm/test/CodeGen/RISCV/double-convert.ll
index fc3790d7de346..d35b57b164b7e 100644
--- a/llvm/test/CodeGen/RISCV/double-convert.ll
+++ b/llvm/test/CodeGen/RISCV/double-convert.ll
@@ -289,14 +289,25 @@ define i32 @fcvt_wu_d_multiple_use(double %x, i32* %y) nounwind {
 }
 
 define i32 @fcvt_wu_d_sat(double %a) nounwind {
-; CHECKIFD-LABEL: fcvt_wu_d_sat:
-; CHECKIFD:       # %bb.0: # %start
-; CHECKIFD-NEXT:    feq.d a0, fa0, fa0
-; CHECKIFD-NEXT:    beqz a0, .LBB6_2
-; CHECKIFD-NEXT:  # %bb.1:
-; CHECKIFD-NEXT:    fcvt.wu.d a0, fa0, rtz
-; CHECKIFD-NEXT:  .LBB6_2: # %start
-; CHECKIFD-NEXT:    ret
+; RV32IFD-LABEL: fcvt_wu_d_sat:
+; RV32IFD:       # %bb.0: # %start
+; RV32IFD-NEXT:    feq.d a0, fa0, fa0
+; RV32IFD-NEXT:    beqz a0, .LBB6_2
+; RV32IFD-NEXT:  # %bb.1:
+; RV32IFD-NEXT:    fcvt.wu.d a0, fa0, rtz
+; RV32IFD-NEXT:  .LBB6_2: # %start
+; RV32IFD-NEXT:    ret
+;
+; RV64IFD-LABEL: fcvt_wu_d_sat:
+; RV64IFD:       # %bb.0: # %start
+; RV64IFD-NEXT:    feq.d a0, fa0, fa0
+; RV64IFD-NEXT:    beqz a0, .LBB6_2
+; RV64IFD-NEXT:  # %bb.1:
+; RV64IFD-NEXT:    fcvt.wu.d a0, fa0, rtz
+; RV64IFD-NEXT:    slli a0, a0, 32
+; RV64IFD-NEXT:    srli a0, a0, 32
+; RV64IFD-NEXT:  .LBB6_2: # %start
+; RV64IFD-NEXT:    ret
 ;
 ; RV32I-LABEL: fcvt_wu_d_sat:
 ; RV32I:       # %bb.0: # %start
@@ -1965,14 +1976,25 @@ start:
 declare i8 @llvm.fptoui.sat.i8.f64(double)
 
 define zeroext i32 @fcvt_wu_d_sat_zext(double %a) nounwind {
-; CHECKIFD-LABEL: fcvt_wu_d_sat_zext:
-; CHECKIFD:       # %bb.0: # %start
-; CHECKIFD-NEXT:    feq.d a0, fa0, fa0
-; CHECKIFD-NEXT:    beqz a0, .LBB33_2
-; CHECKIFD-NEXT:  # %bb.1:
-; CHECKIFD-NEXT:    fcvt.wu.d a0, fa0, rtz
-; CHECKIFD-NEXT:  .LBB33_2: # %start
-; CHECKIFD-NEXT:    ret
+; RV32IFD-LABEL: fcvt_wu_d_sat_zext:
+; RV32IFD:       # %bb.0: # %start
+; RV32IFD-NEXT:    feq.d a0, fa0, fa0
+; RV32IFD-NEXT:    beqz a0, .LBB33_2
+; RV32IFD-NEXT:  # %bb.1:
+; RV32IFD-NEXT:    fcvt.wu.d a0, fa0, rtz
+; RV32IFD-NEXT:  .LBB33_2: # %start
+; RV32IFD-NEXT:    ret
+;
+; RV64IFD-LABEL: fcvt_wu_d_sat_zext:
+; RV64IFD:       # %bb.0: # %start
+; RV64IFD-NEXT:    feq.d a0, fa0, fa0
+; RV64IFD-NEXT:    beqz a0, .LBB33_2
+; RV64IFD-NEXT:  # %bb.1:
+; RV64IFD-NEXT:    fcvt.wu.d a0, fa0, rtz
+; RV64IFD-NEXT:    slli a0, a0, 32
+; RV64IFD-NEXT:    srli a0, a0, 32
+; RV64IFD-NEXT:  .LBB33_2: # %start
+; RV64IFD-NEXT:    ret
 ;
 ; RV32I-LABEL: fcvt_wu_d_sat_zext:
 ; RV32I:       # %bb.0: # %start

diff  --git a/llvm/test/CodeGen/RISCV/float-convert.ll b/llvm/test/CodeGen/RISCV/float-convert.ll
index 7737e86db12f4..5b62d4667d0a0 100644
--- a/llvm/test/CodeGen/RISCV/float-convert.ll
+++ b/llvm/test/CodeGen/RISCV/float-convert.ll
@@ -216,14 +216,25 @@ define i32 @fcvt_wu_s_multiple_use(float %x, i32* %y) nounwind {
 }
 
 define i32 @fcvt_wu_s_sat(float %a) nounwind {
-; CHECKIF-LABEL: fcvt_wu_s_sat:
-; CHECKIF:       # %bb.0: # %start
-; CHECKIF-NEXT:    feq.s a0, fa0, fa0
-; CHECKIF-NEXT:    beqz a0, .LBB4_2
-; CHECKIF-NEXT:  # %bb.1:
-; CHECKIF-NEXT:    fcvt.wu.s a0, fa0, rtz
-; CHECKIF-NEXT:  .LBB4_2: # %start
-; CHECKIF-NEXT:    ret
+; RV32IF-LABEL: fcvt_wu_s_sat:
+; RV32IF:       # %bb.0: # %start
+; RV32IF-NEXT:    feq.s a0, fa0, fa0
+; RV32IF-NEXT:    beqz a0, .LBB4_2
+; RV32IF-NEXT:  # %bb.1:
+; RV32IF-NEXT:    fcvt.wu.s a0, fa0, rtz
+; RV32IF-NEXT:  .LBB4_2: # %start
+; RV32IF-NEXT:    ret
+;
+; RV64IF-LABEL: fcvt_wu_s_sat:
+; RV64IF:       # %bb.0: # %start
+; RV64IF-NEXT:    feq.s a0, fa0, fa0
+; RV64IF-NEXT:    beqz a0, .LBB4_2
+; RV64IF-NEXT:  # %bb.1:
+; RV64IF-NEXT:    fcvt.wu.s a0, fa0, rtz
+; RV64IF-NEXT:    slli a0, a0, 32
+; RV64IF-NEXT:    srli a0, a0, 32
+; RV64IF-NEXT:  .LBB4_2: # %start
+; RV64IF-NEXT:    ret
 ;
 ; RV32I-LABEL: fcvt_wu_s_sat:
 ; RV32I:       # %bb.0: # %start
@@ -1768,14 +1779,25 @@ start:
 declare i8 @llvm.fptoui.sat.i8.f32(float)
 
 define zeroext i32 @fcvt_wu_s_sat_zext(float %a) nounwind {
-; CHECKIF-LABEL: fcvt_wu_s_sat_zext:
-; CHECKIF:       # %bb.0: # %start
-; CHECKIF-NEXT:    feq.s a0, fa0, fa0
-; CHECKIF-NEXT:    beqz a0, .LBB31_2
-; CHECKIF-NEXT:  # %bb.1:
-; CHECKIF-NEXT:    fcvt.wu.s a0, fa0, rtz
-; CHECKIF-NEXT:  .LBB31_2: # %start
-; CHECKIF-NEXT:    ret
+; RV32IF-LABEL: fcvt_wu_s_sat_zext:
+; RV32IF:       # %bb.0: # %start
+; RV32IF-NEXT:    feq.s a0, fa0, fa0
+; RV32IF-NEXT:    beqz a0, .LBB31_2
+; RV32IF-NEXT:  # %bb.1:
+; RV32IF-NEXT:    fcvt.wu.s a0, fa0, rtz
+; RV32IF-NEXT:  .LBB31_2: # %start
+; RV32IF-NEXT:    ret
+;
+; RV64IF-LABEL: fcvt_wu_s_sat_zext:
+; RV64IF:       # %bb.0: # %start
+; RV64IF-NEXT:    feq.s a0, fa0, fa0
+; RV64IF-NEXT:    beqz a0, .LBB31_2
+; RV64IF-NEXT:  # %bb.1:
+; RV64IF-NEXT:    fcvt.wu.s a0, fa0, rtz
+; RV64IF-NEXT:    slli a0, a0, 32
+; RV64IF-NEXT:    srli a0, a0, 32
+; RV64IF-NEXT:  .LBB31_2: # %start
+; RV64IF-NEXT:    ret
 ;
 ; RV32I-LABEL: fcvt_wu_s_sat_zext:
 ; RV32I:       # %bb.0: # %start

diff  --git a/llvm/test/CodeGen/RISCV/half-convert.ll b/llvm/test/CodeGen/RISCV/half-convert.ll
index e5c6b3aebbd08..b582ac23fe13f 100644
--- a/llvm/test/CodeGen/RISCV/half-convert.ll
+++ b/llvm/test/CodeGen/RISCV/half-convert.ll
@@ -690,14 +690,25 @@ define i32 @fcvt_wu_h_multiple_use(half %x, i32* %y) nounwind {
 }
 
 define i32 @fcvt_wu_h_sat(half %a) nounwind {
-; CHECKIZFH-LABEL: fcvt_wu_h_sat:
-; CHECKIZFH:       # %bb.0: # %start
-; CHECKIZFH-NEXT:    feq.h a0, fa0, fa0
-; CHECKIZFH-NEXT:    beqz a0, .LBB8_2
-; CHECKIZFH-NEXT:  # %bb.1:
-; CHECKIZFH-NEXT:    fcvt.wu.h a0, fa0, rtz
-; CHECKIZFH-NEXT:  .LBB8_2: # %start
-; CHECKIZFH-NEXT:    ret
+; RV32IZFH-LABEL: fcvt_wu_h_sat:
+; RV32IZFH:       # %bb.0: # %start
+; RV32IZFH-NEXT:    feq.h a0, fa0, fa0
+; RV32IZFH-NEXT:    beqz a0, .LBB8_2
+; RV32IZFH-NEXT:  # %bb.1:
+; RV32IZFH-NEXT:    fcvt.wu.h a0, fa0, rtz
+; RV32IZFH-NEXT:  .LBB8_2: # %start
+; RV32IZFH-NEXT:    ret
+;
+; RV64IZFH-LABEL: fcvt_wu_h_sat:
+; RV64IZFH:       # %bb.0: # %start
+; RV64IZFH-NEXT:    feq.h a0, fa0, fa0
+; RV64IZFH-NEXT:    beqz a0, .LBB8_2
+; RV64IZFH-NEXT:  # %bb.1:
+; RV64IZFH-NEXT:    fcvt.wu.h a0, fa0, rtz
+; RV64IZFH-NEXT:    slli a0, a0, 32
+; RV64IZFH-NEXT:    srli a0, a0, 32
+; RV64IZFH-NEXT:  .LBB8_2: # %start
+; RV64IZFH-NEXT:    ret
 ;
 ; RV32IDZFH-LABEL: fcvt_wu_h_sat:
 ; RV32IDZFH:       # %bb.0: # %start
@@ -714,6 +725,8 @@ define i32 @fcvt_wu_h_sat(half %a) nounwind {
 ; RV64IDZFH-NEXT:    beqz a0, .LBB8_2
 ; RV64IDZFH-NEXT:  # %bb.1:
 ; RV64IDZFH-NEXT:    fcvt.wu.h a0, fa0, rtz
+; RV64IDZFH-NEXT:    slli a0, a0, 32
+; RV64IDZFH-NEXT:    srli a0, a0, 32
 ; RV64IDZFH-NEXT:  .LBB8_2: # %start
 ; RV64IDZFH-NEXT:    ret
 ;
@@ -3019,14 +3032,25 @@ start:
 declare i8 @llvm.fptoui.sat.i8.f16(half)
 
 define zeroext i32 @fcvt_wu_h_sat_zext(half %a) nounwind {
-; CHECKIZFH-LABEL: fcvt_wu_h_sat_zext:
-; CHECKIZFH:       # %bb.0: # %start
-; CHECKIZFH-NEXT:    feq.h a0, fa0, fa0
-; CHECKIZFH-NEXT:    beqz a0, .LBB39_2
-; CHECKIZFH-NEXT:  # %bb.1:
-; CHECKIZFH-NEXT:    fcvt.wu.h a0, fa0, rtz
-; CHECKIZFH-NEXT:  .LBB39_2: # %start
-; CHECKIZFH-NEXT:    ret
+; RV32IZFH-LABEL: fcvt_wu_h_sat_zext:
+; RV32IZFH:       # %bb.0: # %start
+; RV32IZFH-NEXT:    feq.h a0, fa0, fa0
+; RV32IZFH-NEXT:    beqz a0, .LBB39_2
+; RV32IZFH-NEXT:  # %bb.1:
+; RV32IZFH-NEXT:    fcvt.wu.h a0, fa0, rtz
+; RV32IZFH-NEXT:  .LBB39_2: # %start
+; RV32IZFH-NEXT:    ret
+;
+; RV64IZFH-LABEL: fcvt_wu_h_sat_zext:
+; RV64IZFH:       # %bb.0: # %start
+; RV64IZFH-NEXT:    feq.h a0, fa0, fa0
+; RV64IZFH-NEXT:    beqz a0, .LBB39_2
+; RV64IZFH-NEXT:  # %bb.1:
+; RV64IZFH-NEXT:    fcvt.wu.h a0, fa0, rtz
+; RV64IZFH-NEXT:    slli a0, a0, 32
+; RV64IZFH-NEXT:    srli a0, a0, 32
+; RV64IZFH-NEXT:  .LBB39_2: # %start
+; RV64IZFH-NEXT:    ret
 ;
 ; RV32IDZFH-LABEL: fcvt_wu_h_sat_zext:
 ; RV32IDZFH:       # %bb.0: # %start
@@ -3043,6 +3067,8 @@ define zeroext i32 @fcvt_wu_h_sat_zext(half %a) nounwind {
 ; RV64IDZFH-NEXT:    beqz a0, .LBB39_2
 ; RV64IDZFH-NEXT:  # %bb.1:
 ; RV64IDZFH-NEXT:    fcvt.wu.h a0, fa0, rtz
+; RV64IDZFH-NEXT:    slli a0, a0, 32
+; RV64IDZFH-NEXT:    srli a0, a0, 32
 ; RV64IDZFH-NEXT:  .LBB39_2: # %start
 ; RV64IDZFH-NEXT:    ret
 ;


        


More information about the llvm-commits mailing list