[InstSimplify] fold select with implied condition

This is an almost direct move of the functionality from InstCombine to
InstSimplify. There's no reason not to do this in InstSimplify because
we never create a new value with this transform.

(There's a question of whether any dominance-based transform belongs in
either of these passes, but that's a separate issue.)

I've changed 1 of the conditions for the fold (1 of the blocks for the
branch must be the block we started with) into an assert because I'm not
sure how that could ever be false.

We need 1 extra check to make sure that the instruction itself is in a
basic block because passes other than InstCombine may be using InstSimplify
as an analysis on values that are not wired up yet.

The 3-way compare changes show that InstCombine has some kind of
phase-ordering hole. Otherwise, we would have already gotten the intended
final result that we now show here.

llvm-svn: 347896

diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 9fc10af..ddebcfa 100644 (file)
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -3924,6 +3924,42 @@ static Value *simplifySelectWithFCmp(Value *Cond, Value *T, Value *F) {
   return nullptr;
 }
 
+/// Try to determine the result of a select based on a dominating condition.
+static Value *foldSelectWithDominatingCond(Value *Cond, Value *TV, Value *FV,
+                                           const SimplifyQuery &Q) {
+  // First, make sure that we have a select in a basic block.
+  // We don't know if we are called from some incomplete state.
+  if (!Q.CxtI || !Q.CxtI->getParent())
+    return nullptr;
+
+  // TODO: This is a poor/cheap way to determine dominance. Should we use the
+  // dominator tree in the SimplifyQuery instead?
+  const BasicBlock *SelectBB = Q.CxtI->getParent();
+  const BasicBlock *PredBB = SelectBB->getSinglePredecessor();
+  if (!PredBB)
+    return nullptr;
+
+  // We need a conditional branch in the predecessor.
+  Value *PredCond;
+  BasicBlock *TrueBB, *FalseBB;
+  if (!match(PredBB->getTerminator(), m_Br(m_Value(PredCond), TrueBB, FalseBB)))
+    return nullptr;
+
+  // The branch should get simplified. Don't bother simplifying the select.
+  if (TrueBB == FalseBB)
+    return nullptr;
+
+  assert((TrueBB == SelectBB || FalseBB == SelectBB) &&
+         "Predecessor block does not point to successor?");
+
+  // Is the select condition implied by the predecessor condition?
+  bool CondIsTrue = TrueBB == SelectBB;
+  Optional<bool> Implied = isImpliedCondition(PredCond, Cond, Q.DL, CondIsTrue);
+  if (!Implied)
+    return nullptr;
+  return *Implied ? TV : FV;
+}
+
 /// Given operands for a SelectInst, see if we can fold the result.
 /// If not, this returns null.
 static Value *SimplifySelectInst(Value *Cond, Value *TrueVal, Value *FalseVal,
@@ -3966,6 +4002,9 @@ static Value *SimplifySelectInst(Value *Cond, Value *TrueVal, Value *FalseVal,
   if (Value *V = foldSelectWithBinaryOp(Cond, TrueVal, FalseVal))
     return V;
 
+  if (Value *V = foldSelectWithDominatingCond(Cond, TrueVal, FalseVal, Q))
+    return V;
+
   return nullptr;
 }
 

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index db75c93..19858ae 100644 (file)
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -2021,24 +2021,6 @@ Instruction *InstCombiner::visitSelectInst(SelectInst &SI) {
     }
   }
 
-  // See if we can determine the result of this select based on a dominating
-  // condition.
-  BasicBlock *Parent = SI.getParent();
-  if (BasicBlock *Dom = Parent->getSinglePredecessor()) {
-    auto *PBI = dyn_cast_or_null<BranchInst>(Dom->getTerminator());
-    if (PBI && PBI->isConditional() &&
-        PBI->getSuccessor(0) != PBI->getSuccessor(1) &&
-        (PBI->getSuccessor(0) == Parent || PBI->getSuccessor(1) == Parent)) {
-      bool CondIsTrue = PBI->getSuccessor(0) == Parent;
-      Optional<bool> Implication = isImpliedCondition(
-          PBI->getCondition(), SI.getCondition(), DL, CondIsTrue);
-      if (Implication) {
-        Value *V = *Implication ? TrueVal : FalseVal;
-        return replaceInstUsesWith(SI, V);
-      }
-    }
-  }
-
   // If we can compute the condition, there's no need for a select.
   // Like the above fold, we are attempting to reduce compile-time cost by
   // putting this fold here with limitations rather than in InstSimplify.

diff --git a/llvm/test/Transforms/InstCombine/unrecognized_three-way-comparison.ll b/llvm/test/Transforms/InstCombine/unrecognized_three-way-comparison.ll
index 551efa7..dcd046e 100644 (file)
--- a/llvm/test/Transforms/InstCombine/unrecognized_three-way-comparison.ll
+++ b/llvm/test/Transforms/InstCombine/unrecognized_three-way-comparison.ll
@@ -1,6 +1,4 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; Various patterns of three-ways comparison that are not currently recognized.
-
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
 target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128"
@@ -8,17 +6,12 @@ target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f3
 declare void @foo(i32 %x)
 
 define i32 @compare_against_arbitrary_value(i32 %x, i32 %c) {
-; TODO: We can prove that if %x s> %c then %x != c, so there should be no actual
-;       calculations in callfoo block. @foo can be invoked with 1. We only do it
-;       for constants that are not 0 currently while it could be generalized.
 ; CHECK-LABEL: @compare_against_arbitrary_value(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = icmp sgt i32 [[X:%.*]], [[C:%.*]]
 ; CHECK-NEXT:    br i1 [[TMP0]], label [[CALLFOO:%.*]], label [[EXIT:%.*]]
 ; CHECK:       callfoo:
-; CHECK-NEXT:    [[CMP1:%.*]] = icmp ne i32 [[X]], [[C]]
-; CHECK-NEXT:    [[SELECT2:%.*]] = zext i1 [[CMP1]] to i32
-; CHECK-NEXT:    call void @foo(i32 [[SELECT2]])
+; CHECK-NEXT:    call void @foo(i32 1)
 ; CHECK-NEXT:    br label [[EXIT]]
 ; CHECK:       exit:
 ; CHECK-NEXT:    ret i32 42
@@ -353,17 +346,12 @@ exit:
 }
 
 define i32 @compare_against_arbitrary_value_type_mismatch(i64 %x, i64 %c) {
-; TODO: We can prove that if %x s> %c then %x != c, so there should be no actual
-;       calculations in callfoo block. @foo can be invoked with 1. We only do it
-;       for constants that are not 0 currently while it could be generalized.
 ; CHECK-LABEL: @compare_against_arbitrary_value_type_mismatch(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = icmp sgt i64 [[X:%.*]], [[C:%.*]]
 ; CHECK-NEXT:    br i1 [[TMP0]], label [[CALLFOO:%.*]], label [[EXIT:%.*]]
 ; CHECK:       callfoo:
-; CHECK-NEXT:    [[CMP1:%.*]] = icmp ne i64 [[X]], [[C]]
-; CHECK-NEXT:    [[SELECT2:%.*]] = zext i1 [[CMP1]] to i32
-; CHECK-NEXT:    call void @foo(i32 [[SELECT2]])
+; CHECK-NEXT:    call void @foo(i32 1)
 ; CHECK-NEXT:    br label [[EXIT]]
 ; CHECK:       exit:
 ; CHECK-NEXT:    ret i32 42

diff --git a/llvm/test/Transforms/InstCombine/select-implied.ll b/llvm/test/Transforms/InstSimplify/select-implied.ll
similarity index 94%
rename from llvm/test/Transforms/InstCombine/select-implied.ll
rename to llvm/test/Transforms/InstSimplify/select-implied.ll
index e925031..a456e29 100644 (file)
--- a/llvm/test/Transforms/InstCombine/select-implied.ll
+++ b/llvm/test/Transforms/InstSimplify/select-implied.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -instcombine -S | FileCheck %s
+; RUN: opt < %s -instsimplify -S | FileCheck %s
 
 ; A == B implies A >u B is false.
 
@@ -98,8 +98,8 @@ end:
 }
 
 ; We know the condition of the select is true based on a dominating condition.
-; Therefore, we can replace %cond with %len. However, now the inner icmp is
-; always false and can be elided.
+; Therefore, we can replace %cond with %len. 
+; TODO: len == 8 is known false in bb. This is handled by other passes, but should it be handled here? 
 
 define void @test4(i32 %len) {
 ; CHECK-LABEL: @test4(
@@ -108,8 +108,10 @@ define void @test4(i32 %len) {
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[LEN]], 4
 ; CHECK-NEXT:    br i1 [[CMP]], label [[BB:%.*]], label [[B1:%.*]]
 ; CHECK:       bb:
-; CHECK-NEXT:    br i1 false, label [[B0:%.*]], label [[B1]]
+; CHECK-NEXT:    [[CMP11:%.*]] = icmp eq i32 [[LEN]], 8
+; CHECK-NEXT:    br i1 [[CMP11]], label [[B0:%.*]], label [[B1]]
 ; CHECK:       b0:
+; CHECK-NEXT:    call void @foo(i32 [[LEN]])
 ; CHECK-NEXT:    br label [[B1]]
 ; CHECK:       b1:
 ; CHECK-NEXT:    [[TMP1:%.*]] = phi i32 [ [[LEN]], [[BB]] ], [ undef, [[B0]] ], [ [[TMP0]], [[ENTRY:%.*]] ]