[llvm] r237739 - Revert r237539: "Reapply r237520 with another fix for infinite looping"

Hans Wennborg hans at hanshq.net
Tue May 19 16:06:30 PDT 2015


Author: hans
Date: Tue May 19 18:06:30 2015
New Revision: 237739

URL: http://llvm.org/viewvc/llvm-project?rev=237739&view=rev
Log:
Revert r237539: "Reapply r237520 with another fix for infinite looping"

This caused PR23583.

Removed:
    llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll
Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
    llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
    llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
    llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
    llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp?rev=237739&r1=237738&r2=237739&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp Tue May 19 18:06:30 2015
@@ -435,15 +435,6 @@ Instruction *InstCombiner::visitTrunc(Tr
   if (Instruction *Result = commonCastTransforms(CI))
     return Result;
 
-  // Test if the trunc is the user of a select which is part of a
-  // minimum or maximum operation. If so, don't do any more simplification.
-  // Even simplifying demanded bits can break the canonical form of a 
-  // min/max.
-  Value *LHS, *RHS;
-  if (SelectInst *SI = dyn_cast<SelectInst>(CI.getOperand(0)))
-    if (matchSelectPattern(SI, LHS, RHS) != SPF_UNKNOWN)
-      return nullptr;
-  
   // See if we can simplify any instructions used by the input whose sole
   // purpose is to compute bits we don't care about.
   if (SimplifyDemandedInstructionBits(CI))

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=237739&r1=237738&r2=237739&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Tue May 19 18:06:30 2015
@@ -3970,19 +3970,6 @@ Instruction *InstCombiner::visitFCmpInst
     }
   }
 
-  // Test if the FCmpInst instruction is used exclusively by a select as
-  // part of a minimum or maximum operation. If so, refrain from doing
-  // any other folding. This helps out other analyses which understand
-  // non-obfuscated minimum and maximum idioms, such as ScalarEvolution
-  // and CodeGen. And in this case, at least one of the comparison
-  // operands has at least one user besides the compare (the select),
-  // which would often largely negate the benefit of folding anyway.
-  if (I.hasOneUse())
-    if (SelectInst *SI = dyn_cast<SelectInst>(*I.user_begin()))
-      if ((SI->getOperand(1) == Op0 && SI->getOperand(2) == Op1) ||
-          (SI->getOperand(2) == Op0 && SI->getOperand(1) == Op1))
-        return nullptr;
-
   // Handle fcmp with constant RHS
   if (Constant *RHSC = dyn_cast<Constant>(Op1)) {
     if (Instruction *LHSI = dyn_cast<Instruction>(Op0))

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp?rev=237739&r1=237738&r2=237739&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp Tue May 19 18:06:30 2015
@@ -1154,30 +1154,18 @@ Instruction *InstCombiner::visitSelectIn
       }
 
   // See if we can fold the select into one of our operands.
-  if (SI.getType()->isIntOrIntVectorTy()) {
+  if (SI.getType()->isIntegerTy()) {
     if (Instruction *FoldI = FoldSelectIntoOp(SI, TrueVal, FalseVal))
       return FoldI;
 
     Value *LHS, *RHS, *LHS2, *RHS2;
-    Instruction::CastOps CastOp;
-    SelectPatternFlavor SPF = matchSelectPattern(&SI, LHS, RHS, &CastOp);
+    SelectPatternFlavor SPF = matchSelectPattern(&SI, LHS, RHS);
 
+    // MAX(MAX(a, b), a) -> MAX(a, b)
+    // MIN(MIN(a, b), a) -> MIN(a, b)
+    // MAX(MIN(a, b), a) -> a
+    // MIN(MAX(a, b), a) -> a
     if (SPF) {
-      // Canonicalize so that type casts are outside select patterns.
-      if (LHS->getType()->getPrimitiveSizeInBits() !=
-          SI.getType()->getPrimitiveSizeInBits()) {
-        CmpInst::Predicate Pred = getICmpPredicateForMinMax(SPF);
-        Value *Cmp = Builder->CreateICmp(Pred, LHS, RHS);
-        Value *NewSI = Builder->CreateCast(CastOp,
-                                           Builder->CreateSelect(Cmp, LHS, RHS),
-                                           SI.getType());
-        return ReplaceInstUsesWith(SI, NewSI);
-      }
-
-      // MAX(MAX(a, b), a) -> MAX(a, b)
-      // MIN(MIN(a, b), a) -> MIN(a, b)
-      // MAX(MIN(a, b), a) -> a
-      // MIN(MAX(a, b), a) -> a
       if (SelectPatternFlavor SPF2 = matchSelectPattern(LHS, LHS2, RHS2))
         if (Instruction *R = FoldSPFofSPF(cast<Instruction>(LHS),SPF2,LHS2,RHS2,
                                           SI, SPF, RHS))

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp?rev=237739&r1=237738&r2=237739&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp Tue May 19 18:06:30 2015
@@ -13,7 +13,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "InstCombineInternal.h"
-#include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/PatternMatch.h"
 
@@ -407,12 +406,6 @@ Value *InstCombiner::SimplifyDemandedUse
     break;
   }
   case Instruction::Select:
-    // If this is a select as part of a min/max pattern, don't simplify any
-    // further in case we break the structure.
-    Value *LHS, *RHS;
-    if (matchSelectPattern(I, LHS, RHS) != SPF_UNKNOWN)
-      return nullptr;
-      
     if (SimplifyDemandedBits(I->getOperandUse(2), DemandedMask, RHSKnownZero,
                              RHSKnownOne, Depth + 1) ||
         SimplifyDemandedBits(I->getOperandUse(1), DemandedMask, LHSKnownZero,

Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=237739&r1=237738&r2=237739&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Tue May 19 18:06:30 2015
@@ -714,22 +714,6 @@ Instruction *InstCombiner::FoldOpIntoSel
         return nullptr;
     }
 
-    // Test if a CmpInst instruction is used exclusively by a select as
-    // part of a minimum or maximum operation. If so, refrain from doing
-    // any other folding. This helps out other analyses which understand
-    // non-obfuscated minimum and maximum idioms, such as ScalarEvolution
-    // and CodeGen. And in this case, at least one of the comparison
-    // operands has at least one user besides the compare (the select),
-    // which would often largely negate the benefit of folding anyway.
-    if (auto *CI = dyn_cast<CmpInst>(SI->getCondition())) {
-      if (CI->hasOneUse()) {
-        Value *Op0 = CI->getOperand(0), *Op1 = CI->getOperand(1);
-        if ((SI->getOperand(1) == Op0 && SI->getOperand(2) == Op1) ||
-            (SI->getOperand(2) == Op0 && SI->getOperand(1) == Op1))
-          return nullptr;
-      }
-    }
-
     Value *SelectTrueVal = FoldOperationIntoSelectOperand(Op, TV, this);
     Value *SelectFalseVal = FoldOperationIntoSelectOperand(Op, FV, this);
 
@@ -739,6 +723,7 @@ Instruction *InstCombiner::FoldOpIntoSel
   return nullptr;
 }
 
+
 /// FoldOpIntoPhi - Given a binary operator, cast instruction, or select which
 /// has a PHI node as operand #0, see if we can fold the instruction into the
 /// PHI (which is only possible if all operands to the PHI are constants).

Removed: llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll?rev=237738&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll (removed)
@@ -1,99 +0,0 @@
-; RUN: opt -S -instcombine < %s | FileCheck %s
-
-; CHECK-LABEL: @t1
-; CHECK-NEXT: icmp
-; CHECK-NEXT: select
-; CHECK-NEXT: sext
-define i64 @t1(i32 %a) {
-  ; This is the canonical form for a type-changing min/max.
-  %1 = icmp slt i32 %a, 5
-  %2 = select i1 %1, i32 %a, i32 5
-  %3 = sext i32 %2 to i64
-  ret i64 %3
-}
-
-; CHECK-LABEL: @t2
-; CHECK-NEXT: icmp
-; CHECK-NEXT: select
-; CHECK-NEXT: sext
-define i64 @t2(i32 %a) {
-  ; Check this is converted into canonical form, as above.
-  %1 = icmp slt i32 %a, 5
-  %2 = sext i32 %a to i64
-  %3 = select i1 %1, i64 %2, i64 5
-  ret i64 %3
-}
-
-; CHECK-LABEL: @t3
-; CHECK-NEXT: icmp
-; CHECK-NEXT: select
-; CHECK-NEXT: zext
-define i64 @t3(i32 %a) {
-  ; Same as @t2, with flipped operands and zext instead of sext.
-  %1 = icmp ult i32 %a, 5
-  %2 = zext i32 %a to i64
-  %3 = select i1 %1, i64 5, i64 %2
-  ret i64 %3
-}
-
-; CHECK-LABEL: @t4
-; CHECK-NEXT: icmp
-; CHECK-NEXT: select
-; CHECK-NEXT: trunc
-define i32 @t4(i64 %a) {
-  ; Same again, with trunc.
-  %1 = icmp slt i64 %a, 5
-  %2 = trunc i64 %a to i32
-  %3 = select i1 %1, i32 %2, i32 5
-  ret i32 %3
-}
-
-; CHECK-LABEL: @t5
-; CHECK-NEXT: icmp
-; CHECK-NEXT: zext
-; CHECK-NEXT: select
-define i64 @t5(i32 %a) {
-  ; Same as @t3, but with mismatched signedness between icmp and zext.
-  ; InstCombine should leave this alone.
-  %1 = icmp slt i32 %a, 5
-  %2 = zext i32 %a to i64
-  %3 = select i1 %1, i64 5, i64 %2
-  ret i64 %3
-}
-
-; CHECK-LABEL: @t6
-; CHECK-NEXT: icmp
-; CHECK-NEXT: select
-; CHECK-NEXT: sitofp
-define float @t6(i32 %a) {
-  %1 = icmp slt i32 %a, 0
-  %2 = select i1 %1, i32 %a, i32 0
-  %3 = sitofp i32 %2 to float
-  ret float %3
-}
-
-; CHECK-LABEL: @t7
-; CHECK-NEXT: icmp
-; CHECK-NEXT: select
-; CHECK-NEXT: trunc
-define i16 @t7(i32 %a) {
-  %1 = icmp slt i32 %a, -32768
-  %2 = trunc i32 %a to i16
-  %3 = select i1 %1, i16 %2, i16 -32768
-  ret i16 %3
-}
-
-; Just check for no infinite loop. InstSimplify liked to
-; "simplify" -32767 by removing all the sign bits,
-; which led to a canonicalization fight between different
-; parts of instcombine.
-define i32 @t8(i64 %a, i32 %b) {
-  %1 = icmp slt i64 %a, -32767
-  %2 = select i1 %1, i64 %a, i64 -32767
-  %3 = trunc i64 %2 to i32
-  %4 = icmp slt i32 %b, 42
-  %5 = select i1 %4, i32 42, i32 %3
-  %6 = icmp ne i32 %5, %b
-  %7 = zext i1 %6 to i32
-  ret i32 %7
-}





More information about the llvm-commits mailing list