[PATCH] D41333: [ValueTracking] ignore FP signed-zero when detecting a casted-to-integer fmin/fmax pattern

[Sanjay Patel via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL321456: [ValueTracking] ignore FP signed-zero when detecting a casted-to-integer… (authored by spatel, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41333?vs=127290&id=128173#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D41333

Files:
  llvm/trunk/lib/Analysis/ValueTracking.cpp
  llvm/trunk/test/Transforms/InstCombine/minmax-fp.ll


Index: llvm/trunk/lib/Analysis/ValueTracking.cpp
===================================================================
--- llvm/trunk/lib/Analysis/ValueTracking.cpp
+++ llvm/trunk/lib/Analysis/ValueTracking.cpp
@@ -4238,14 +4238,14 @@
   LHS = CmpLHS;
   RHS = CmpRHS;
 
-  // If the predicate is an "or-equal"  (FP) predicate, then signed zeroes may
-  // return inconsistent results between implementations.
-  //   (0.0 <= -0.0) ? 0.0 : -0.0 // Returns 0.0
-  //   minNum(0.0, -0.0)          // May return -0.0 or 0.0 (IEEE 754-2008 5.3.1)
-  // Therefore we behave conservatively and only proceed if at least one of the
-  // operands is known to not be zero, or if we don't care about signed zeroes.
+  // Signed zero may return inconsistent results between implementations.
+  //  (0.0 <= -0.0) ? 0.0 : -0.0 // Returns 0.0
+  //  minNum(0.0, -0.0)          // May return -0.0 or 0.0 (IEEE 754-2008 5.3.1)
+  // Therefore, we behave conservatively and only proceed if at least one of the
+  // operands is known to not be zero or if we don't care about signed zero.
   switch (Pred) {
   default: break;
+  // FIXME: Include OGT/OLT/UGT/ULT.
   case CmpInst::FCMP_OGE: case CmpInst::FCMP_OLE:
   case CmpInst::FCMP_UGE: case CmpInst::FCMP_ULE:
     if (!FMF.noSignedZeros() && !isKnownNonZero(CmpLHS) &&
@@ -4493,14 +4493,24 @@
 
   // Deal with type mismatches.
   if (CastOp && CmpLHS->getType() != TrueVal->getType()) {
-    if (Value *C = lookThroughCast(CmpI, TrueVal, FalseVal, CastOp))
+    if (Value *C = lookThroughCast(CmpI, TrueVal, FalseVal, CastOp)) {
+      // If this is a potential fmin/fmax with a cast to integer, then ignore
+      // -0.0 because there is no corresponding integer value.
+      if (*CastOp == Instruction::FPToSI || *CastOp == Instruction::FPToUI)
+        FMF.setNoSignedZeros();
       return ::matchSelectPattern(Pred, FMF, CmpLHS, CmpRHS,
                                   cast<CastInst>(TrueVal)->getOperand(0), C,
                                   LHS, RHS);
-    if (Value *C = lookThroughCast(CmpI, FalseVal, TrueVal, CastOp))
+    }
+    if (Value *C = lookThroughCast(CmpI, FalseVal, TrueVal, CastOp)) {
+      // If this is a potential fmin/fmax with a cast to integer, then ignore
+      // -0.0 because there is no corresponding integer value.
+      if (*CastOp == Instruction::FPToSI || *CastOp == Instruction::FPToUI)
+        FMF.setNoSignedZeros();
       return ::matchSelectPattern(Pred, FMF, CmpLHS, CmpRHS,
                                   C, cast<CastInst>(FalseVal)->getOperand(0),
                                   LHS, RHS);
+    }
   }
   return ::matchSelectPattern(Pred, FMF, CmpLHS, CmpRHS, TrueVal, FalseVal,
                               LHS, RHS);
Index: llvm/trunk/test/Transforms/InstCombine/minmax-fp.ll
===================================================================
--- llvm/trunk/test/Transforms/InstCombine/minmax-fp.ll
+++ llvm/trunk/test/Transforms/InstCombine/minmax-fp.ll
@@ -155,20 +155,33 @@
   ret i8 %3
 }
 
-; <= comparison, where %a could be -0.0. Not safe.
+; %a could be -0.0, but it doesn't matter because the conversion to int is the same for 0.0 or -0.0.
 define i8 @t14(float %a) {
 ; CHECK-LABEL: @t14(
-; CHECK-NEXT:    [[TMP1:%.*]] = fcmp ule float %a, 0.000000e+00
-; CHECK-NEXT:    [[TMP2:%.*]] = fptosi float %a to i8
-; CHECK-NEXT:    [[TMP3:%.*]] = select i1 [[TMP1]], i8 [[TMP2]], i8 0
-; CHECK-NEXT:    ret i8 [[TMP3]]
+; CHECK-NEXT:    [[DOTINV:%.*]] = fcmp oge float %a, 0.000000e+00
+; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[DOTINV]], float 0.000000e+00, float %a
+; CHECK-NEXT:    [[TMP2:%.*]] = fptosi float [[TMP1]] to i8
+; CHECK-NEXT:    ret i8 [[TMP2]]
 ;
   %1 = fcmp ule float %a, 0.0
   %2 = fptosi float %a to i8
   %3 = select i1 %1, i8 %2, i8 0
   ret i8 %3
 }
 
+define i8 @t14_commute(float %a) {
+; CHECK-LABEL: @t14_commute(
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp ogt float %a, 0.000000e+00
+; CHECK-NEXT:    [[TMP2:%.*]] = select i1 [[TMP1]], float %a, float 0.000000e+00
+; CHECK-NEXT:    [[TMP3:%.*]] = fptosi float [[TMP2]] to i8
+; CHECK-NEXT:    ret i8 [[TMP3]]
+;
+  %1 = fcmp ule float %a, 0.0
+  %2 = fptosi float %a to i8
+  %3 = select i1 %1, i8 0, i8 %2
+  ret i8 %3
+}
+
 define i8 @t15(float %a) {
 ; CHECK-LABEL: @t15(
 ; CHECK-NEXT:    [[DOTINV:%.*]] = fcmp nsz oge float %a, 0.000000e+00


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41333.128173.patch
Type: text/x-patch
Size: 4342 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171226/907b429f/attachment.bin>