[llvm] r268086 - [ValueTracking] matchSelectPattern needs to be more careful around FP

[David Majnemer via llvm-commits]

Author: majnemer
Date: Fri Apr 29 13:40:34 2016
New Revision: 268086

URL: http://llvm.org/viewvc/llvm-project?rev=268086&view=rev
Log:
[ValueTracking] matchSelectPattern needs to be more careful around FP

matchSelectPattern attempts to see through casts which mask min/max
patterns from being more obvious.  Under certain circumstances, it would
misidentify a sequence of instructions as a min/max because it assumed
that folding casts would preserve the result.  This is not the case for
floating point <-> integer casts.

This fixes PR27575.

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

Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=268086&r1=268085&r2=268086&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
+++ llvm/trunk/lib/Analysis/ValueTracking.cpp Fri Apr 29 13:40:34 2016
@@ -3699,12 +3699,11 @@ static Value *lookThroughCast(CmpInst *C
                               Instruction::CastOps *CastOp) {
   CastInst *CI = dyn_cast<CastInst>(V1);
   Constant *C = dyn_cast<Constant>(V2);
-  CastInst *CI2 = dyn_cast<CastInst>(V2);
   if (!CI)
     return nullptr;
   *CastOp = CI->getOpcode();
 
-  if (CI2) {
+  if (auto *CI2 = dyn_cast<CastInst>(V2)) {
     // If V1 and V2 are both the same cast from the same type, we can look
     // through V1.
     if (CI2->getOpcode() == CI->getOpcode() &&
@@ -3715,39 +3714,52 @@ static Value *lookThroughCast(CmpInst *C
     return nullptr;
   }
 
-  if (isa<SExtInst>(CI) && CmpI->isSigned()) {
-    Constant *T = ConstantExpr::getTrunc(C, CI->getSrcTy());
-    // This is only valid if the truncated value can be sign-extended
-    // back to the original value.
-    if (ConstantExpr::getSExt(T, C->getType()) == C)
-      return T;
-    return nullptr;
-  }
   if (isa<ZExtInst>(CI) && CmpI->isUnsigned())
     return ConstantExpr::getTrunc(C, CI->getSrcTy());
 
   if (isa<TruncInst>(CI))
     return ConstantExpr::getIntegerCast(C, CI->getSrcTy(), CmpI->isSigned());
 
+  if (isa<FPTruncInst>(CI))
+    return ConstantExpr::getFPExtend(C, CI->getSrcTy(), true);
+
+  if (isa<FPExtInst>(CI))
+    return ConstantExpr::getFPTrunc(C, CI->getSrcTy(), true);
+
+  // Sophisticated constants can have values which we cannot easily reason
+  // about.  Skip them for the fp<->int case.
+  if (isa<ConstantExpr>(C))
+    return nullptr;
+
+  Constant *CastedTo = nullptr;
+
+  // This is only valid if the truncated value can be sign-extended
+  // back to the original value.
+  if (isa<SExtInst>(CI) && CmpI->isSigned())
+    CastedTo = ConstantExpr::getTrunc(C, CI->getSrcTy(), true);
+
   if (isa<FPToUIInst>(CI))
-    return ConstantExpr::getUIToFP(C, CI->getSrcTy(), true);
+    CastedTo = ConstantExpr::getUIToFP(C, CI->getSrcTy(), true);
 
   if (isa<FPToSIInst>(CI))
-    return ConstantExpr::getSIToFP(C, CI->getSrcTy(), true);
+    CastedTo = ConstantExpr::getSIToFP(C, CI->getSrcTy(), true);
 
   if (isa<UIToFPInst>(CI))
-    return ConstantExpr::getFPToUI(C, CI->getSrcTy(), true);
+    CastedTo = ConstantExpr::getFPToUI(C, CI->getSrcTy(), true);
 
   if (isa<SIToFPInst>(CI))
-    return ConstantExpr::getFPToSI(C, CI->getSrcTy(), true);
+    CastedTo = ConstantExpr::getFPToSI(C, CI->getSrcTy(), true);
 
-  if (isa<FPTruncInst>(CI))
-    return ConstantExpr::getFPExtend(C, CI->getSrcTy(), true);
+  if (!CastedTo)
+    return nullptr;
 
-  if (isa<FPExtInst>(CI))
-    return ConstantExpr::getFPTrunc(C, CI->getSrcTy(), true);
+  Constant *CastedBack =
+      ConstantExpr::getCast(CI->getOpcode(), CastedTo, C->getType(), true);
+  // Make sure the cast doesn't lose any information.
+  if (CastedBack != C)
+    return nullptr;
 
-  return nullptr;
+  return CastedTo;
 }
 
 SelectPatternResult llvm::matchSelectPattern(Value *V,

Modified: llvm/trunk/test/Transforms/InstCombine/minmax-fp.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/minmax-fp.ll?rev=268086&r1=268085&r2=268086&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/minmax-fp.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/minmax-fp.ll Fri Apr 29 13:40:34 2016
@@ -154,3 +154,29 @@ define i8 @t15(float %a) {
   %3 = select i1 %1, i8 %2, i8 0
   ret i8 %3
 }
+
+; CHECK-LABEL: @t16
+; CHECK:  %[[cmp:.*]] = icmp sgt i32 %x, 0
+; CHECK:  %[[cst:.*]] = sitofp i32 %x to double
+; CHECK:  %[[sel:.*]] = select i1 %[[cmp]], double %[[cst]], double 5.000000e-01
+; CHECK:  ret double %[[sel]]
+define double @t16(i32 %x) {
+entry:
+  %cmp = icmp sgt i32 %x, 0
+  %cst = sitofp i32 %x to double
+  %sel = select i1 %cmp, double %cst, double 5.000000e-01
+  ret double %sel
+}
+
+; CHECK-LABEL: @t17
+; CHECK:  %[[cmp:.*]] = icmp sgt i32 %x, 2
+; CHECK:  %[[sel:.*]] = select i1 %[[cmp]], i32 %x, i32 2
+; CHECK:  %[[cst:.*]] = sitofp i32 %[[sel]] to double
+; CHECK:  ret double %[[cst]]
+define double @t17(i32 %x) {
+entry:
+  %cmp = icmp sgt i32 %x, 2
+  %cst = sitofp i32 %x to double
+  %sel = select i1 %cmp, double %cst, double 2.0
+  ret double %sel
+}