[PATCH] D72794: [LegalizeDAG][Mips] Add an assert to protect a uint_to_fp implementation from double rounding. Add a i32->f32 uint_to_fp implementation that avoids this code.

[Craig Topper via Phabricator via llvm-commits]

craig.topper updated this revision to Diff 238419.
craig.topper added a comment.
Herald added a subscriber: jrtc27.

I was able to move the other version of this code from TargetLowering back to LegalizeDAG. So just need to add the new type to it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72794/new/

https://reviews.llvm.org/D72794

Files:
  llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
  llvm/test/CodeGen/Mips/uitofp.ll


Index: llvm/test/CodeGen/Mips/uitofp.ll
===================================================================
--- llvm/test/CodeGen/Mips/uitofp.ll
+++ llvm/test/CodeGen/Mips/uitofp.ll
@@ -7,17 +7,18 @@
 ; CHECK-NEXT:    addiu $sp, $sp, -8
 ; CHECK-NEXT:    addiu $1, $zero, 1
 ; CHECK-NEXT:    sw $1, 4($sp)
-; CHECK-NEXT:    lui $1, %hi($CPI0_0)
-; CHECK-NEXT:    addiu $1, $1, %lo($CPI0_0)
-; CHECK-NEXT:    lw $2, 4($sp)
-; CHECK-NEXT:    srl $3, $2, 29
-; CHECK-NEXT:    andi $3, $3, 4
-; CHECK-NEXT:    addu $1, $1, $3
-; CHECK-NEXT:    lwc1 $f0, 0($1)
-; CHECK-NEXT:    mtc1 $2, $f1
+; CHECK-NEXT:    lw $1, 4($sp)
+; CHECK-NEXT:    srl $2, $1, 1
+; CHECK-NEXT:    andi $3, $1, 1
+; CHECK-NEXT:    or $2, $3, $2
+; CHECK-NEXT:    mtc1 $2, $f0
+; CHECK-NEXT:    cvt.s.w $f0, $f0
+; CHECK-NEXT:    add.s $f0, $f0, $f0
+; CHECK-NEXT:    mtc1 $1, $f1
 ; CHECK-NEXT:    cvt.s.w $f1, $f1
-; CHECK-NEXT:    add.s $f0, $f1, $f0
-; CHECK-NEXT:    swc1 $f0, 0($sp)
+; CHECK-NEXT:    slti $1, $1, 0
+; CHECK-NEXT:    movn.s $f1, $f0, $1
+; CHECK-NEXT:    swc1 $f1, 0($sp)
 ; CHECK-NEXT:    jr $ra
 ; CHECK-NEXT:    addiu $sp, $sp, 8
 entry:
Index: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
+++ llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
@@ -2421,10 +2421,11 @@
   assert(!isSigned && "Legalize cannot Expand SINT_TO_FP for i64 yet");
 
   // TODO: Generalize this for use with other types.
-  if (SrcVT == MVT::i64 && DestVT == MVT::f32) {
-    LLVM_DEBUG(dbgs() << "Converting unsigned i64 to f32\n");
+  if ((SrcVT == MVT::i32 || SrcVT == MVT::i64) && DestVT == MVT::f32) {
+    LLVM_DEBUG(dbgs() << "Converting unsigned i32/i64 to f32\n");
     // For unsigned conversions, convert them to signed conversions using the
-    // algorithm from the x86_64 __floatundidf in compiler_rt.
+    // algorithm from the x86_64 __floatundisf in compiler_rt. That method
+    // should be valid for i32->f32 as well.
 
     // TODO: This really should be implemented using a branch rather than a
     // select.  We happen to get lucky and machinesink does the right
@@ -2469,8 +2470,13 @@
     return DAG.getSelect(dl, DestVT, SignBitTest, Slow, Fast);
   }
 
-  // FIXME: This can produce slightly incorrect results. See details in
-  // FIXME: https://reviews.llvm.org/D69275
+  // The following optimization is valid only if every value in SrcVT (when
+  // treated as signed) is representable in DestVT.  Check that the mantissa
+  // size of DestVT is >= than the number of bits in SrcVT -1.
+  assert(APFloat::semanticsPrecision(DAG.EVTToAPFloatSemantics(DestVT)) >=
+             SrcVT.getSizeInBits() - 1 &&
+         "Cannot perform lossless SINT_TO_FP!");
+
   SDValue Tmp1;
   if (Node->isStrictFPOpcode()) {
     Tmp1 = DAG.getNode(ISD::STRICT_SINT_TO_FP, dl, { DestVT, MVT::Other },


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72794.238419.patch
Type: text/x-patch
Size: 2906 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200116/5e578559/attachment.bin>