[llvm] r237453 - Canonicalize min/max expressions correctly.

James Molloy james.molloy at arm.com
Fri May 15 09:10:59 PDT 2015


Author: jamesm
Date: Fri May 15 11:10:59 2015
New Revision: 237453

URL: http://llvm.org/viewvc/llvm-project?rev=237453&view=rev
Log:
Canonicalize min/max expressions correctly.

This patch introduces a canonical form for min/max idioms where one operand
is extended or truncated. This often happens when the other operand is a
constant. For example:

  %1 = icmp slt i32 %a, i32 0
  %2 = sext i32 %a to i64
  %3 = select i1 %1, i64 %2, i64 0

Would now be canonicalized into:

  %1 = icmp slt i32 %a, i32 0
  %2 = select i1 %1, i32 %a, i32 0
  %3 = sext i32 %2 to i64

This builds upon a patch posted by David Majenemer
(https://www.marc.info/?l=llvm-commits&m=143008038714141&w=2). That pass
passively stopped instcombine from ruining canonical patterns. This
patch additionally actively makes instcombine canonicalize too.

Canonicalization of expressions involving a change in type from int->fp
or fp->int are not yet implemented.

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

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=237453&r1=237452&r2=237453&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Fri May 15 11:10:59 2015
@@ -3970,6 +3970,19 @@ 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=237453&r1=237452&r2=237453&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp Fri May 15 11:10:59 2015
@@ -1154,18 +1154,30 @@ Instruction *InstCombiner::visitSelectIn
       }
 
   // See if we can fold the select into one of our operands.
-  if (SI.getType()->isIntegerTy()) {
+  if (SI.getType()->isIntOrIntVectorTy()) {
     if (Instruction *FoldI = FoldSelectIntoOp(SI, TrueVal, FalseVal))
       return FoldI;
 
     Value *LHS, *RHS, *LHS2, *RHS2;
-    SelectPatternFlavor SPF = matchSelectPattern(&SI, LHS, RHS);
+    Instruction::CastOps CastOp;
+    SelectPatternFlavor SPF = matchSelectPattern(&SI, LHS, RHS, &CastOp);
 
-    // 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/InstructionCombining.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=237453&r1=237452&r2=237453&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Fri May 15 11:10:59 2015
@@ -714,6 +714,22 @@ 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);
 
@@ -723,7 +739,6 @@ 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).

Added: llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll?rev=237453&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll (added)
+++ llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll Fri May 15 11:10:59 2015
@@ -0,0 +1,73 @@
+; 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
+}





More information about the llvm-commits mailing list