[ValueTracking] Handle recursive select/PHI in ComputeKnownBits #114689

goldsteinn · 2024-11-03T00:19:05Z

This is a follow up to #114008 and #113707

llvmbot · 2024-11-03T00:19:40Z

@llvm/pr-subscribers-llvm-analysis

Author: None (goldsteinn)

Changes

This is a follow up to #114008 and #113707

Full diff: https://github.com/llvm/llvm-project/pull/114689.diff

5 Files Affected:

(modified) llvm/lib/Analysis/ValueTracking.cpp (+35-8)
(modified) llvm/test/Analysis/ScalarEvolution/cycled_phis.ll (+2-2)
(modified) llvm/test/Analysis/ScalarEvolution/unknown_phis.ll (+2-2)
(modified) llvm/test/Transforms/InstCombine/known-phi-recurse.ll (+105)
(modified) llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll (+9-15)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 5c20c24d0ae00a..c65a4c800178b2 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -1572,6 +1572,7 @@ static void computeKnownBitsFromOperator(const Operator *I,
         // edges so we don't overly clamp analysis.
         unsigned IncDepth = MaxAnalysisRecursionDepth - 1;
 
+		Instruction *CxtI = P->getIncomingBlock(u)->getTerminator();
         // If the Use is a select of this phi, use the knownbit of the other
         // operand to break the recursion.
         if (auto *SI = dyn_cast<SelectInst>(IncValue)) {
@@ -1580,14 +1581,22 @@ static void computeKnownBitsFromOperator(const Operator *I,
                                                : SI->getTrueValue();
             IncDepth = Depth + 1;
           }
+        } else if (auto *IncPhi = dyn_cast<PHINode>(IncValue);
+                   IncPhi && IncPhi->getNumIncomingValues() == 2) {
+          for (int Idx = 0; Idx < 2; ++Idx) {
+            if (IncPhi->getIncomingValue(Idx) == P) {
+              IncValue = IncPhi->getIncomingValue(1 - Idx);
+              CxtI = IncPhi->getIncomingBlock(1 - Idx)->getTerminator();
+              break;
+            }
+          }
         }
 
         // Change the context instruction to the "edge" that flows into the
         // phi. This is important because that is where the value is actually
         // "evaluated" even though it is used later somewhere else. (see also
         // D69571).
-        SimplifyQuery RecQ = Q.getWithoutCondContext();
-        RecQ.CxtI = P->getIncomingBlock(u)->getTerminator();
+        SimplifyQuery RecQ = Q.getWithoutCondContext().getWithInstruction(CxtI);
 
         Known2 = KnownBits(BitWidth);
         computeKnownBits(IncValue, DemandedElts, Known2, IncDepth, RecQ);
@@ -3023,18 +3032,36 @@ static bool isKnownNonZeroFromOperator(const Operator *I,
       return true;
 
     // Check if all incoming values are non-zero using recursion.
-    SimplifyQuery RecQ = Q.getWithoutCondContext();
-    unsigned NewDepth = std::max(Depth, MaxAnalysisRecursionDepth - 1);
     return llvm::all_of(PN->operands(), [&](const Use &U) {
-      if (U.get() == PN)
+      Value *IncValue = U.get();
+      if (IncValue == PN)
         return true;
-      RecQ.CxtI = PN->getIncomingBlock(U)->getTerminator();
+
+      Instruction *CxtI = PN->getIncomingBlock(U)->getTerminator();
+      unsigned NewDepth = std::max(Depth, MaxAnalysisRecursionDepth - 1);
+      if (auto *SI = dyn_cast<SelectInst>(IncValue)) {
+        if (SI->getTrueValue() == PN || SI->getFalseValue() == PN) {
+          IncValue = SI->getTrueValue() == PN ? SI->getFalseValue()
+                                              : SI->getTrueValue();
+          NewDepth = Depth;
+        }
+      } else if (auto *IncPhi = dyn_cast<PHINode>(IncValue);
+                 IncPhi && IncPhi->getNumIncomingValues() == 2) {
+        for (int Idx = 0; Idx < 2; ++Idx) {
+          if (IncPhi->getIncomingValue(Idx) == PN) {
+            IncValue = IncPhi->getIncomingValue(1 - Idx);
+            CxtI = IncPhi->getIncomingBlock(1 - Idx)->getTerminator();
+            break;
+          }
+        }
+      }
+      SimplifyQuery RecQ = Q.getWithoutCondContext().getWithInstruction(CxtI);
       // Check if the branch on the phi excludes zero.
       ICmpInst::Predicate Pred;
       Value *X;
       BasicBlock *TrueSucc, *FalseSucc;
       if (match(RecQ.CxtI,
-                m_Br(m_c_ICmp(Pred, m_Specific(U.get()), m_Value(X)),
+                m_Br(m_c_ICmp(Pred, m_Specific(IncValue), m_Value(X)),
                      m_BasicBlock(TrueSucc), m_BasicBlock(FalseSucc)))) {
         // Check for cases of duplicate successors.
         if ((TrueSucc == PN->getParent()) != (FalseSucc == PN->getParent())) {
@@ -3046,7 +3073,7 @@ static bool isKnownNonZeroFromOperator(const Operator *I,
         }
       }
       // Finally recurse on the edge and check it directly.
-      return isKnownNonZero(U.get(), DemandedElts, RecQ, NewDepth);
+      return isKnownNonZero(IncValue, DemandedElts, RecQ, NewDepth);
     });
   }
   case Instruction::InsertElement: {
diff --git a/llvm/test/Analysis/ScalarEvolution/cycled_phis.ll b/llvm/test/Analysis/ScalarEvolution/cycled_phis.ll
index ec244595e8fe39..478bcf94daf697 100644
--- a/llvm/test/Analysis/ScalarEvolution/cycled_phis.ll
+++ b/llvm/test/Analysis/ScalarEvolution/cycled_phis.ll
@@ -8,9 +8,9 @@ define void @test_01() {
 ; CHECK-LABEL: 'test_01'
 ; CHECK-NEXT:  Classifying expressions for: @test_01
 ; CHECK-NEXT:    %phi_1 = phi i32 [ 10, %entry ], [ %phi_2, %loop ]
-; CHECK-NEXT:    --> %phi_1 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
+; CHECK-NEXT:    --> %phi_1 U: [0,31) S: [0,31) Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
 ; CHECK-NEXT:    %phi_2 = phi i32 [ 20, %entry ], [ %phi_1, %loop ]
-; CHECK-NEXT:    --> %phi_2 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
+; CHECK-NEXT:    --> %phi_2 U: [0,31) S: [0,31) Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
 ; CHECK-NEXT:    %cond = call i1 @cond()
 ; CHECK-NEXT:    --> %cond U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
 ; CHECK-NEXT:  Determining loop execution counts for: @test_01
diff --git a/llvm/test/Analysis/ScalarEvolution/unknown_phis.ll b/llvm/test/Analysis/ScalarEvolution/unknown_phis.ll
index bdfe38f67de0b9..c6d430f96b7de1 100644
--- a/llvm/test/Analysis/ScalarEvolution/unknown_phis.ll
+++ b/llvm/test/Analysis/ScalarEvolution/unknown_phis.ll
@@ -39,9 +39,9 @@ define void @merge_values_with_ranges_looped(ptr %a_len_ptr, ptr %b_len_ptr) {
 ; CHECK-NEXT:    %len_b = load i32, ptr %b_len_ptr, align 4, !range !0
 ; CHECK-NEXT:    --> %len_b U: [0,2147483647) S: [0,2147483647)
 ; CHECK-NEXT:    %p1 = phi i32 [ %len_a, %entry ], [ %p2, %loop ]
-; CHECK-NEXT:    --> %p1 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
+; CHECK-NEXT:    --> %p1 U: [0,-2147483648) S: [0,-2147483648) Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
 ; CHECK-NEXT:    %p2 = phi i32 [ %len_b, %entry ], [ %p1, %loop ]
-; CHECK-NEXT:    --> %p2 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
+; CHECK-NEXT:    --> %p2 U: [0,-2147483648) S: [0,-2147483648) Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
 ; CHECK-NEXT:    %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
 ; CHECK-NEXT:    --> {0,+,1}<nuw><nsw><%loop> U: [0,100) S: [0,100) Exits: 99 LoopDispositions: { %loop: Computable }
 ; CHECK-NEXT:    %iv.next = add i32 %iv, 1
diff --git a/llvm/test/Transforms/InstCombine/known-phi-recurse.ll b/llvm/test/Transforms/InstCombine/known-phi-recurse.ll
index fd3728324b8ea8..d1a90d0c2aef78 100644
--- a/llvm/test/Transforms/InstCombine/known-phi-recurse.ll
+++ b/llvm/test/Transforms/InstCombine/known-phi-recurse.ll
@@ -256,4 +256,109 @@ exit:
   ret i8 %bool
 }
 
+define i8 @knownbits_phi_phi_test() {
+; CHECK-LABEL: @knownbits_phi_phi_test(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[INDVAR:%.*]] = phi i8 [ 0, [[ENTRY:%.*]] ], [ [[CONTAIN:%.*]], [[LOOP_BB1:%.*]] ]
+; CHECK-NEXT:    [[COND0:%.*]] = call i1 @cond()
+; CHECK-NEXT:    br i1 [[COND0]], label [[LOOP_BB0:%.*]], label [[LOOP_BB1]]
+; CHECK:       loop.bb0:
+; CHECK-NEXT:    call void @side.effect()
+; CHECK-NEXT:    br label [[LOOP_BB1]]
+; CHECK:       loop.bb1:
+; CHECK-NEXT:    [[CONTAIN]] = phi i8 [ 1, [[LOOP_BB0]] ], [ [[INDVAR]], [[LOOP]] ]
+; CHECK-NEXT:    [[COND1:%.*]] = call i1 @cond()
+; CHECK-NEXT:    br i1 [[COND1]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret i8 [[CONTAIN]]
+;
+entry:
+  br label %loop
+
+loop:
+  %indvar = phi i8 [ 0, %entry ], [ %contain, %loop.bb1 ]
+  %cond0 = call i1 @cond()
+  br i1 %cond0, label %loop.bb0, label %loop.bb1
+loop.bb0:
+  call void @side.effect()
+  br label %loop.bb1
+loop.bb1:
+  %contain = phi i8 [ 1, %loop.bb0 ], [ %indvar, %loop ]
+  %cond1 = call i1 @cond()
+  br i1 %cond1, label %exit, label %loop
+
+exit:
+  %bool = and i8 %contain, 1
+  ret i8 %bool
+}
+
+
+define i1 @known_non_zero_phi_phi_test() {
+; CHECK-LABEL: @known_non_zero_phi_phi_test(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[COND0:%.*]] = call i1 @cond()
+; CHECK-NEXT:    br i1 [[COND0]], label [[LOOP_BB0:%.*]], label [[LOOP_BB1:%.*]]
+; CHECK:       loop.bb0:
+; CHECK-NEXT:    call void @side.effect()
+; CHECK-NEXT:    br label [[LOOP_BB1]]
+; CHECK:       loop.bb1:
+; CHECK-NEXT:    [[COND1:%.*]] = call i1 @cond()
+; CHECK-NEXT:    br i1 [[COND1]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret i1 false
+;
+entry:
+  br label %loop
+
+loop:
+  %indvar = phi i8 [ 2, %entry ], [ %contain, %loop.bb1 ]
+  %cond0 = call i1 @cond()
+  br i1 %cond0, label %loop.bb0, label %loop.bb1
+loop.bb0:
+  call void @side.effect()
+  br label %loop.bb1
+loop.bb1:
+  %contain = phi i8 [ 1, %loop.bb0 ], [ %indvar, %loop ]
+  %cond1 = call i1 @cond()
+  br i1 %cond1, label %exit, label %loop
+
+exit:
+  %bool = icmp eq i8 %contain, 0
+  ret i1 %bool
+}
+
+define i1 @known_non_zero_phi_select_test() {
+; CHECK-LABEL: @known_non_zero_phi_select_test(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[INDVAR:%.*]] = phi i8 [ 2, [[ENTRY:%.*]] ], [ [[CONTAIN:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[COND0:%.*]] = call i1 @cond()
+; CHECK-NEXT:    [[CONTAIN]] = select i1 [[COND0]], i8 1, i8 [[INDVAR]]
+; CHECK-NEXT:    [[COND1:%.*]] = call i1 @cond()
+; CHECK-NEXT:    br i1 [[COND1]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret i1 false
+;
+entry:
+  br label %loop
+
+loop:
+  %indvar = phi i8 [ 2, %entry ], [ %contain, %loop ]
+  %cond0 = call i1 @cond()
+  %contain = select i1 %cond0, i8 1, i8 %indvar
+  %cond1 = call i1 @cond()
+  br i1 %cond1, label %exit, label %loop
+
+exit:
+  %bool = icmp eq i8 %contain, 0
+  ret i1 %bool
+}
+
 declare i1 @cond()
+declare void @side.effect()
+
diff --git a/llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll b/llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll
index 03aee68fa4248c..7066207255fd49 100644
--- a/llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll
+++ b/llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll
@@ -8,29 +8,23 @@ define dso_local noundef i32 @main() {
 ; CHECK-LABEL: define dso_local noundef i32 @main() {
 ; CHECK-NEXT:  [[BB:.*]]:
 ; CHECK-NEXT:    [[ALLOCA:%.*]] = alloca [2 x ptr], align 16
-; CHECK-NEXT:    store ptr blockaddress(@main, %[[BB4:.*]]), ptr [[ALLOCA]], align 16, !tbaa [[TBAA0:![0-9]+]]
+; CHECK-NEXT:    store ptr blockaddress(@main, %[[BB1:.*]]), ptr [[ALLOCA]], align 16, !tbaa [[TBAA0:![0-9]+]]
 ; CHECK-NEXT:    [[GETELEMENTPTR:%.*]] = getelementptr inbounds [2 x ptr], ptr [[ALLOCA]], i64 0, i64 1
 ; CHECK-NEXT:    store ptr blockaddress(@main, %[[BB10:.*]]), ptr [[GETELEMENTPTR]], align 8, !tbaa [[TBAA0]]
-; CHECK-NEXT:    br label %[[BB1:.*]]
+; CHECK-NEXT:    br label %[[BB1]]
 ; CHECK:       [[BB1]]:
-; CHECK-NEXT:    [[PHI:%.*]] = phi i32 [ 0, %[[BB]] ], [ [[PHI8:%.*]], %[[BB7:.*]] ]
-; CHECK-NEXT:    [[PHI2:%.*]] = phi i32 [ 0, %[[BB]] ], [ [[PHI9:%.*]], %[[BB7]] ]
-; CHECK-NEXT:    switch i32 [[PHI]], label %[[BB7]] [
-; CHECK-NEXT:      i32 0, label %[[BB12:.*]]
-; CHECK-NEXT:      i32 1, label %[[BB4]]
+; CHECK-NEXT:    [[PHI:%.*]] = phi i32 [ 0, %[[BB]] ], [ 2, %[[BB12:.*]] ]
+; CHECK-NEXT:    [[PHI2:%.*]] = phi i32 [ 0, %[[BB]] ], [ [[PHI13:%.*]], %[[BB12]] ]
+; CHECK-NEXT:    switch i32 [[PHI]], label %[[BB1_UNREACHABLEDEFAULT:.*]] [
+; CHECK-NEXT:      i32 0, label %[[BB12]]
 ; CHECK-NEXT:      i32 2, label %[[BB6:.*]]
 ; CHECK-NEXT:    ]
-; CHECK:       [[BB4]]:
-; CHECK-NEXT:    [[PHI5:%.*]] = phi i32 [ [[PHI13:%.*]], %[[BB12]] ], [ [[PHI2]], %[[BB1]] ]
-; CHECK-NEXT:    br label %[[BB7]]
 ; CHECK:       [[BB6]]:
 ; CHECK-NEXT:    [[CALL:%.*]] = call i32 @foo(i32 noundef [[PHI2]])
 ; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[PHI2]], 1
 ; CHECK-NEXT:    br label %[[BB12]]
-; CHECK:       [[BB7]]:
-; CHECK-NEXT:    [[PHI8]] = phi i32 [ [[PHI]], %[[BB1]] ], [ 2, %[[BB4]] ]
-; CHECK-NEXT:    [[PHI9]] = phi i32 [ [[PHI2]], %[[BB1]] ], [ [[PHI5]], %[[BB4]] ]
-; CHECK-NEXT:    br label %[[BB1]], !llvm.loop [[LOOP4:![0-9]+]]
+; CHECK:       [[BB1_UNREACHABLEDEFAULT]]:
+; CHECK-NEXT:    unreachable
 ; CHECK:       [[BB10]]:
 ; CHECK-NEXT:    [[CALL11:%.*]] = call i32 @foo(i32 noundef [[PHI13]])
 ; CHECK-NEXT:    ret i32 0
@@ -39,7 +33,7 @@ define dso_local noundef i32 @main() {
 ; CHECK-NEXT:    [[SEXT:%.*]] = sext i32 [[PHI13]] to i64
 ; CHECK-NEXT:    [[GETELEMENTPTR14:%.*]] = getelementptr inbounds [2 x ptr], ptr [[ALLOCA]], i64 0, i64 [[SEXT]]
 ; CHECK-NEXT:    [[LOAD:%.*]] = load ptr, ptr [[GETELEMENTPTR14]], align 8, !tbaa [[TBAA0]]
-; CHECK-NEXT:    indirectbr ptr [[LOAD]], [label %[[BB4]], label %bb10]
+; CHECK-NEXT:    indirectbr ptr [[LOAD]], [label %[[BB1]], label %bb10], !llvm.loop [[LOOP4:![0-9]+]]
 ;
 bb:
   %alloca = alloca [2 x ptr], align 16

llvmbot · 2024-11-03T00:19:40Z

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes

This is a follow up to #114008 and #113707

Full diff: https://github.com/llvm/llvm-project/pull/114689.diff

5 Files Affected:

(modified) llvm/lib/Analysis/ValueTracking.cpp (+35-8)
(modified) llvm/test/Analysis/ScalarEvolution/cycled_phis.ll (+2-2)
(modified) llvm/test/Analysis/ScalarEvolution/unknown_phis.ll (+2-2)
(modified) llvm/test/Transforms/InstCombine/known-phi-recurse.ll (+105)
(modified) llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll (+9-15)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 5c20c24d0ae00a..c65a4c800178b2 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -1572,6 +1572,7 @@ static void computeKnownBitsFromOperator(const Operator *I,
         // edges so we don't overly clamp analysis.
         unsigned IncDepth = MaxAnalysisRecursionDepth - 1;
 
+		Instruction *CxtI = P->getIncomingBlock(u)->getTerminator();
         // If the Use is a select of this phi, use the knownbit of the other
         // operand to break the recursion.
         if (auto *SI = dyn_cast<SelectInst>(IncValue)) {
@@ -1580,14 +1581,22 @@ static void computeKnownBitsFromOperator(const Operator *I,
                                                : SI->getTrueValue();
             IncDepth = Depth + 1;
           }
+        } else if (auto *IncPhi = dyn_cast<PHINode>(IncValue);
+                   IncPhi && IncPhi->getNumIncomingValues() == 2) {
+          for (int Idx = 0; Idx < 2; ++Idx) {
+            if (IncPhi->getIncomingValue(Idx) == P) {
+              IncValue = IncPhi->getIncomingValue(1 - Idx);
+              CxtI = IncPhi->getIncomingBlock(1 - Idx)->getTerminator();
+              break;
+            }
+          }
         }
 
         // Change the context instruction to the "edge" that flows into the
         // phi. This is important because that is where the value is actually
         // "evaluated" even though it is used later somewhere else. (see also
         // D69571).
-        SimplifyQuery RecQ = Q.getWithoutCondContext();
-        RecQ.CxtI = P->getIncomingBlock(u)->getTerminator();
+        SimplifyQuery RecQ = Q.getWithoutCondContext().getWithInstruction(CxtI);
 
         Known2 = KnownBits(BitWidth);
         computeKnownBits(IncValue, DemandedElts, Known2, IncDepth, RecQ);
@@ -3023,18 +3032,36 @@ static bool isKnownNonZeroFromOperator(const Operator *I,
       return true;
 
     // Check if all incoming values are non-zero using recursion.
-    SimplifyQuery RecQ = Q.getWithoutCondContext();
-    unsigned NewDepth = std::max(Depth, MaxAnalysisRecursionDepth - 1);
     return llvm::all_of(PN->operands(), [&](const Use &U) {
-      if (U.get() == PN)
+      Value *IncValue = U.get();
+      if (IncValue == PN)
         return true;
-      RecQ.CxtI = PN->getIncomingBlock(U)->getTerminator();
+
+      Instruction *CxtI = PN->getIncomingBlock(U)->getTerminator();
+      unsigned NewDepth = std::max(Depth, MaxAnalysisRecursionDepth - 1);
+      if (auto *SI = dyn_cast<SelectInst>(IncValue)) {
+        if (SI->getTrueValue() == PN || SI->getFalseValue() == PN) {
+          IncValue = SI->getTrueValue() == PN ? SI->getFalseValue()
+                                              : SI->getTrueValue();
+          NewDepth = Depth;
+        }
+      } else if (auto *IncPhi = dyn_cast<PHINode>(IncValue);
+                 IncPhi && IncPhi->getNumIncomingValues() == 2) {
+        for (int Idx = 0; Idx < 2; ++Idx) {
+          if (IncPhi->getIncomingValue(Idx) == PN) {
+            IncValue = IncPhi->getIncomingValue(1 - Idx);
+            CxtI = IncPhi->getIncomingBlock(1 - Idx)->getTerminator();
+            break;
+          }
+        }
+      }
+      SimplifyQuery RecQ = Q.getWithoutCondContext().getWithInstruction(CxtI);
       // Check if the branch on the phi excludes zero.
       ICmpInst::Predicate Pred;
       Value *X;
       BasicBlock *TrueSucc, *FalseSucc;
       if (match(RecQ.CxtI,
-                m_Br(m_c_ICmp(Pred, m_Specific(U.get()), m_Value(X)),
+                m_Br(m_c_ICmp(Pred, m_Specific(IncValue), m_Value(X)),
                      m_BasicBlock(TrueSucc), m_BasicBlock(FalseSucc)))) {
         // Check for cases of duplicate successors.
         if ((TrueSucc == PN->getParent()) != (FalseSucc == PN->getParent())) {
@@ -3046,7 +3073,7 @@ static bool isKnownNonZeroFromOperator(const Operator *I,
         }
       }
       // Finally recurse on the edge and check it directly.
-      return isKnownNonZero(U.get(), DemandedElts, RecQ, NewDepth);
+      return isKnownNonZero(IncValue, DemandedElts, RecQ, NewDepth);
     });
   }
   case Instruction::InsertElement: {
diff --git a/llvm/test/Analysis/ScalarEvolution/cycled_phis.ll b/llvm/test/Analysis/ScalarEvolution/cycled_phis.ll
index ec244595e8fe39..478bcf94daf697 100644
--- a/llvm/test/Analysis/ScalarEvolution/cycled_phis.ll
+++ b/llvm/test/Analysis/ScalarEvolution/cycled_phis.ll
@@ -8,9 +8,9 @@ define void @test_01() {
 ; CHECK-LABEL: 'test_01'
 ; CHECK-NEXT:  Classifying expressions for: @test_01
 ; CHECK-NEXT:    %phi_1 = phi i32 [ 10, %entry ], [ %phi_2, %loop ]
-; CHECK-NEXT:    --> %phi_1 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
+; CHECK-NEXT:    --> %phi_1 U: [0,31) S: [0,31) Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
 ; CHECK-NEXT:    %phi_2 = phi i32 [ 20, %entry ], [ %phi_1, %loop ]
-; CHECK-NEXT:    --> %phi_2 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
+; CHECK-NEXT:    --> %phi_2 U: [0,31) S: [0,31) Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
 ; CHECK-NEXT:    %cond = call i1 @cond()
 ; CHECK-NEXT:    --> %cond U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
 ; CHECK-NEXT:  Determining loop execution counts for: @test_01
diff --git a/llvm/test/Analysis/ScalarEvolution/unknown_phis.ll b/llvm/test/Analysis/ScalarEvolution/unknown_phis.ll
index bdfe38f67de0b9..c6d430f96b7de1 100644
--- a/llvm/test/Analysis/ScalarEvolution/unknown_phis.ll
+++ b/llvm/test/Analysis/ScalarEvolution/unknown_phis.ll
@@ -39,9 +39,9 @@ define void @merge_values_with_ranges_looped(ptr %a_len_ptr, ptr %b_len_ptr) {
 ; CHECK-NEXT:    %len_b = load i32, ptr %b_len_ptr, align 4, !range !0
 ; CHECK-NEXT:    --> %len_b U: [0,2147483647) S: [0,2147483647)
 ; CHECK-NEXT:    %p1 = phi i32 [ %len_a, %entry ], [ %p2, %loop ]
-; CHECK-NEXT:    --> %p1 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
+; CHECK-NEXT:    --> %p1 U: [0,-2147483648) S: [0,-2147483648) Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
 ; CHECK-NEXT:    %p2 = phi i32 [ %len_b, %entry ], [ %p1, %loop ]
-; CHECK-NEXT:    --> %p2 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
+; CHECK-NEXT:    --> %p2 U: [0,-2147483648) S: [0,-2147483648) Exits: <<Unknown>> LoopDispositions: { %loop: Variant }
 ; CHECK-NEXT:    %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
 ; CHECK-NEXT:    --> {0,+,1}<nuw><nsw><%loop> U: [0,100) S: [0,100) Exits: 99 LoopDispositions: { %loop: Computable }
 ; CHECK-NEXT:    %iv.next = add i32 %iv, 1
diff --git a/llvm/test/Transforms/InstCombine/known-phi-recurse.ll b/llvm/test/Transforms/InstCombine/known-phi-recurse.ll
index fd3728324b8ea8..d1a90d0c2aef78 100644
--- a/llvm/test/Transforms/InstCombine/known-phi-recurse.ll
+++ b/llvm/test/Transforms/InstCombine/known-phi-recurse.ll
@@ -256,4 +256,109 @@ exit:
   ret i8 %bool
 }
 
+define i8 @knownbits_phi_phi_test() {
+; CHECK-LABEL: @knownbits_phi_phi_test(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[INDVAR:%.*]] = phi i8 [ 0, [[ENTRY:%.*]] ], [ [[CONTAIN:%.*]], [[LOOP_BB1:%.*]] ]
+; CHECK-NEXT:    [[COND0:%.*]] = call i1 @cond()
+; CHECK-NEXT:    br i1 [[COND0]], label [[LOOP_BB0:%.*]], label [[LOOP_BB1]]
+; CHECK:       loop.bb0:
+; CHECK-NEXT:    call void @side.effect()
+; CHECK-NEXT:    br label [[LOOP_BB1]]
+; CHECK:       loop.bb1:
+; CHECK-NEXT:    [[CONTAIN]] = phi i8 [ 1, [[LOOP_BB0]] ], [ [[INDVAR]], [[LOOP]] ]
+; CHECK-NEXT:    [[COND1:%.*]] = call i1 @cond()
+; CHECK-NEXT:    br i1 [[COND1]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret i8 [[CONTAIN]]
+;
+entry:
+  br label %loop
+
+loop:
+  %indvar = phi i8 [ 0, %entry ], [ %contain, %loop.bb1 ]
+  %cond0 = call i1 @cond()
+  br i1 %cond0, label %loop.bb0, label %loop.bb1
+loop.bb0:
+  call void @side.effect()
+  br label %loop.bb1
+loop.bb1:
+  %contain = phi i8 [ 1, %loop.bb0 ], [ %indvar, %loop ]
+  %cond1 = call i1 @cond()
+  br i1 %cond1, label %exit, label %loop
+
+exit:
+  %bool = and i8 %contain, 1
+  ret i8 %bool
+}
+
+
+define i1 @known_non_zero_phi_phi_test() {
+; CHECK-LABEL: @known_non_zero_phi_phi_test(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[COND0:%.*]] = call i1 @cond()
+; CHECK-NEXT:    br i1 [[COND0]], label [[LOOP_BB0:%.*]], label [[LOOP_BB1:%.*]]
+; CHECK:       loop.bb0:
+; CHECK-NEXT:    call void @side.effect()
+; CHECK-NEXT:    br label [[LOOP_BB1]]
+; CHECK:       loop.bb1:
+; CHECK-NEXT:    [[COND1:%.*]] = call i1 @cond()
+; CHECK-NEXT:    br i1 [[COND1]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret i1 false
+;
+entry:
+  br label %loop
+
+loop:
+  %indvar = phi i8 [ 2, %entry ], [ %contain, %loop.bb1 ]
+  %cond0 = call i1 @cond()
+  br i1 %cond0, label %loop.bb0, label %loop.bb1
+loop.bb0:
+  call void @side.effect()
+  br label %loop.bb1
+loop.bb1:
+  %contain = phi i8 [ 1, %loop.bb0 ], [ %indvar, %loop ]
+  %cond1 = call i1 @cond()
+  br i1 %cond1, label %exit, label %loop
+
+exit:
+  %bool = icmp eq i8 %contain, 0
+  ret i1 %bool
+}
+
+define i1 @known_non_zero_phi_select_test() {
+; CHECK-LABEL: @known_non_zero_phi_select_test(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[INDVAR:%.*]] = phi i8 [ 2, [[ENTRY:%.*]] ], [ [[CONTAIN:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[COND0:%.*]] = call i1 @cond()
+; CHECK-NEXT:    [[CONTAIN]] = select i1 [[COND0]], i8 1, i8 [[INDVAR]]
+; CHECK-NEXT:    [[COND1:%.*]] = call i1 @cond()
+; CHECK-NEXT:    br i1 [[COND1]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret i1 false
+;
+entry:
+  br label %loop
+
+loop:
+  %indvar = phi i8 [ 2, %entry ], [ %contain, %loop ]
+  %cond0 = call i1 @cond()
+  %contain = select i1 %cond0, i8 1, i8 %indvar
+  %cond1 = call i1 @cond()
+  br i1 %cond1, label %exit, label %loop
+
+exit:
+  %bool = icmp eq i8 %contain, 0
+  ret i1 %bool
+}
+
 declare i1 @cond()
+declare void @side.effect()
+
diff --git a/llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll b/llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll
index 03aee68fa4248c..7066207255fd49 100644
--- a/llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll
+++ b/llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll
@@ -8,29 +8,23 @@ define dso_local noundef i32 @main() {
 ; CHECK-LABEL: define dso_local noundef i32 @main() {
 ; CHECK-NEXT:  [[BB:.*]]:
 ; CHECK-NEXT:    [[ALLOCA:%.*]] = alloca [2 x ptr], align 16
-; CHECK-NEXT:    store ptr blockaddress(@main, %[[BB4:.*]]), ptr [[ALLOCA]], align 16, !tbaa [[TBAA0:![0-9]+]]
+; CHECK-NEXT:    store ptr blockaddress(@main, %[[BB1:.*]]), ptr [[ALLOCA]], align 16, !tbaa [[TBAA0:![0-9]+]]
 ; CHECK-NEXT:    [[GETELEMENTPTR:%.*]] = getelementptr inbounds [2 x ptr], ptr [[ALLOCA]], i64 0, i64 1
 ; CHECK-NEXT:    store ptr blockaddress(@main, %[[BB10:.*]]), ptr [[GETELEMENTPTR]], align 8, !tbaa [[TBAA0]]
-; CHECK-NEXT:    br label %[[BB1:.*]]
+; CHECK-NEXT:    br label %[[BB1]]
 ; CHECK:       [[BB1]]:
-; CHECK-NEXT:    [[PHI:%.*]] = phi i32 [ 0, %[[BB]] ], [ [[PHI8:%.*]], %[[BB7:.*]] ]
-; CHECK-NEXT:    [[PHI2:%.*]] = phi i32 [ 0, %[[BB]] ], [ [[PHI9:%.*]], %[[BB7]] ]
-; CHECK-NEXT:    switch i32 [[PHI]], label %[[BB7]] [
-; CHECK-NEXT:      i32 0, label %[[BB12:.*]]
-; CHECK-NEXT:      i32 1, label %[[BB4]]
+; CHECK-NEXT:    [[PHI:%.*]] = phi i32 [ 0, %[[BB]] ], [ 2, %[[BB12:.*]] ]
+; CHECK-NEXT:    [[PHI2:%.*]] = phi i32 [ 0, %[[BB]] ], [ [[PHI13:%.*]], %[[BB12]] ]
+; CHECK-NEXT:    switch i32 [[PHI]], label %[[BB1_UNREACHABLEDEFAULT:.*]] [
+; CHECK-NEXT:      i32 0, label %[[BB12]]
 ; CHECK-NEXT:      i32 2, label %[[BB6:.*]]
 ; CHECK-NEXT:    ]
-; CHECK:       [[BB4]]:
-; CHECK-NEXT:    [[PHI5:%.*]] = phi i32 [ [[PHI13:%.*]], %[[BB12]] ], [ [[PHI2]], %[[BB1]] ]
-; CHECK-NEXT:    br label %[[BB7]]
 ; CHECK:       [[BB6]]:
 ; CHECK-NEXT:    [[CALL:%.*]] = call i32 @foo(i32 noundef [[PHI2]])
 ; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[PHI2]], 1
 ; CHECK-NEXT:    br label %[[BB12]]
-; CHECK:       [[BB7]]:
-; CHECK-NEXT:    [[PHI8]] = phi i32 [ [[PHI]], %[[BB1]] ], [ 2, %[[BB4]] ]
-; CHECK-NEXT:    [[PHI9]] = phi i32 [ [[PHI2]], %[[BB1]] ], [ [[PHI5]], %[[BB4]] ]
-; CHECK-NEXT:    br label %[[BB1]], !llvm.loop [[LOOP4:![0-9]+]]
+; CHECK:       [[BB1_UNREACHABLEDEFAULT]]:
+; CHECK-NEXT:    unreachable
 ; CHECK:       [[BB10]]:
 ; CHECK-NEXT:    [[CALL11:%.*]] = call i32 @foo(i32 noundef [[PHI13]])
 ; CHECK-NEXT:    ret i32 0
@@ -39,7 +33,7 @@ define dso_local noundef i32 @main() {
 ; CHECK-NEXT:    [[SEXT:%.*]] = sext i32 [[PHI13]] to i64
 ; CHECK-NEXT:    [[GETELEMENTPTR14:%.*]] = getelementptr inbounds [2 x ptr], ptr [[ALLOCA]], i64 0, i64 [[SEXT]]
 ; CHECK-NEXT:    [[LOAD:%.*]] = load ptr, ptr [[GETELEMENTPTR14]], align 8, !tbaa [[TBAA0]]
-; CHECK-NEXT:    indirectbr ptr [[LOAD]], [label %[[BB4]], label %bb10]
+; CHECK-NEXT:    indirectbr ptr [[LOAD]], [label %[[BB1]], label %bb10], !llvm.loop [[LOOP4:![0-9]+]]
 ;
 bb:
   %alloca = alloca [2 x ptr], align 16

github-actions · 2024-11-03T00:22:18Z

✅ With the latest revision this PR passed the C/C++ code formatter.

dtcxzyw

Can you split this patch into two parts (computeKnownBits + isKnownNonZero)?
Changes in isKnownNonZero seem to cause regressions.

dtcxzyw · 2024-11-03T05:39:50Z

llvm/lib/Analysis/ValueTracking.cpp

@@ -1580,14 +1582,22 @@ static void computeKnownBitsFromOperator(const Operator *I,
                                               : SI->getTrueValue();
            IncDepth = Depth + 1;
          }
+        } else if (auto *IncPhi = dyn_cast<PHINode>(IncValue);


We need a helper function void breakSelfRecursion(Use &U, PHINode *PHI, Value*& ValOut, unsigned &DepthInOut, Instruction *&CtxIOut) to avoid duplicate logic.

I was thinking the same thing. Ill update.

Done, let me know if you want me to break it out to an NFC

tschuett · 2024-11-03T09:00:42Z

llvm/lib/Analysis/ValueTracking.cpp

@@ -580,6 +580,43 @@ static bool cmpExcludesZero(CmpInst::Predicate Pred, const Value *RHS) {
  return true;
 }

+static void breakSelfRecursivePHI(const Use *U, const PHINode *PHI,
+                                  Value *&ValOut, Instruction *&CtxIOut,


Can you achieve the same without mutable references, e.g., return something?

We could return a tuple.

Although this seems aligned with how we do output variables for other helpers.

tschuett · 2024-11-03T09:02:44Z

llvm/lib/Analysis/ValueTracking.cpp

+  CtxIOut = PHI->getIncomingBlock(*U)->getTerminator();
+  Value *V;
+  // If the Use is a select of this phi, compute analysis on other arm to be
+  // recusion.


llvm/lib/Analysis/ValueTracking.cpp

nikic

I don't fully understand the recursion limit handling here, both in the old and the new code. Why is it safe to use the full depth for the select case? Can't you have multiple phi incoming values that are all selects and perform a full-depth walk on all of them?

goldsteinn · 2024-11-03T15:09:39Z

I don't fully understand the recursion limit handling here, both in the old and the new code. Why is it safe to use the full depth for the select case? Can't you have multiple phi incoming values that are all selects and perform a full-depth walk on all of them?

I think that the pathological case is a recursive phi is Num_Phi_Nodes^(MaxDepth - CurDepth), if we know we aren't hitting a case that doesn't scale exponentially with Depth its seems reasonable enough. Also it seems highly unlikely even in a PHI with a thousand arms to have ALL of them be select-referential selects.

nikic · 2024-11-03T16:05:28Z

I don't fully understand the recursion limit handling here, both in the old and the new code. Why is it safe to use the full depth for the select case? Can't you have multiple phi incoming values that are all selects and perform a full-depth walk on all of them?

I think that the pathological case is a recursive phi is Num_Phi_Nodes^(MaxDepth - CurDepth), if we know we aren't hitting a case that doesn't scale exponentially with Depth its seems reasonable enough. Also it seems highly unlikely even in a PHI with a thousand arms to have ALL of them be select-referential selects.

But doesn't that scale potentially exponentially? Especially if you take into account the fact that the other select operand may in fact also be based on the phi (just take something like c ? phi + 1 : phi), so we get the usual recursive fan-out problem, just with a select interleaved in between.

goldsteinn · 2024-11-03T16:45:01Z

I don't fully understand the recursion limit handling here, both in the old and the new code. Why is it safe to use the full depth for the select case? Can't you have multiple phi incoming values that are all selects and perform a full-depth walk on all of them?

I think that the pathological case is a recursive phi is Num_Phi_Nodes^(MaxDepth - CurDepth), if we know we aren't hitting a case that doesn't scale exponentially with Depth its seems reasonable enough. Also it seems highly unlikely even in a PHI with a thousand arms to have ALL of them be select-referential selects.

But doesn't that scale potentially exponentially? Especially if you take into account the fact that the other select operand may in fact also be based on the phi (just take something like c ? phi + 1 : phi), so we get the usual recursive fan-out problem, just with a select interleaved in between.

Yes, you could still construct an input that would cause serious perf regressions here. Although based on available data this seems more of a theoretical problem than a practical one. Should we sacrifice practical analysis quality for a theoretical compile time regression?

nikic · 2024-11-03T17:08:37Z

I don't fully understand the recursion limit handling here, both in the old and the new code. Why is it safe to use the full depth for the select case? Can't you have multiple phi incoming values that are all selects and perform a full-depth walk on all of them?

I think that the pathological case is a recursive phi is Num_Phi_Nodes^(MaxDepth - CurDepth), if we know we aren't hitting a case that doesn't scale exponentially with Depth its seems reasonable enough. Also it seems highly unlikely even in a PHI with a thousand arms to have ALL of them be select-referential selects.

But doesn't that scale potentially exponentially? Especially if you take into account the fact that the other select operand may in fact also be based on the phi (just take something like c ? phi + 1 : phi), so we get the usual recursive fan-out problem, just with a select interleaved in between.

Yes, you could still construct an input that would cause serious perf regressions here. Although based on available data this seems more of a theoretical problem than a practical one. Should we sacrifice practical analysis quality for a theoretical compile time regression?

Yes, we should always protect against pathological cases, even if we have not encountered them in practice yet.

goldsteinn · 2024-11-03T18:18:37Z

I don't fully understand the recursion limit handling here, both in the old and the new code. Why is it safe to use the full depth for the select case? Can't you have multiple phi incoming values that are all selects and perform a full-depth walk on all of them?

I think that the pathological case is a recursive phi is Num_Phi_Nodes^(MaxDepth - CurDepth), if we know we aren't hitting a case that doesn't scale exponentially with Depth its seems reasonable enough. Also it seems highly unlikely even in a PHI with a thousand arms to have ALL of them be select-referential selects.

But doesn't that scale potentially exponentially? Especially if you take into account the fact that the other select operand may in fact also be based on the phi (just take something like c ? phi + 1 : phi), so we get the usual recursive fan-out problem, just with a select interleaved in between.

Yes, you could still construct an input that would cause serious perf regressions here. Although based on available data this seems more of a theoretical problem than a practical one. Should we sacrifice practical analysis quality for a theoretical compile time regression?

Yes, we should always protect against pathological cases, even if we have not encountered them in practice yet.

Okay, will update

tschuett · 2024-11-03T18:19:38Z

llvm/lib/Analysis/ValueTracking.cpp

+    return;
+  CtxIOut = PHI->getIncomingBlock(*U)->getTerminator();
+  Value *V;
+  // If the Use is a select of this phi, compute analysis on other arm to be


be or break ?

As discussed on llvm#114689 (review) and following, there is no principled reason why the phi of select case should have a different recursion limit than the general case. There may still be fan-out, and there may still be indirect recursion. Revert that part of llvm#113707.

As discussed on #114689 (review) and following, there is no principled reason why the phi of select case should have a different recursion limit than the general case. There may still be fan-out, and there may still be indirect recursion. Revert that part of #113707.

As discussed on llvm#114689 (review) and following, there is no principled reason why the phi of select case should have a different recursion limit than the general case. There may still be fan-out, and there may still be indirect recursion. Revert that part of llvm#113707.

…eKnownBits/isKnownNonZero; NFC

Finish porting llvm#114008 to `KnownBits` (Follow up to llvm#113707).

goldsteinn · 2025-01-11T00:15:47Z

Removed all Depth related adjustments.

goldsteinn · 2025-01-20T22:19:07Z

ping

nikic

LGTM

antoniofrighetto · 2025-01-25T17:46:09Z

llvm/lib/Analysis/ValueTracking.cpp


        // Change the context instruction to the "edge" that flows into the
        // phi. This is important because that is where the value is actually
        // "evaluated" even though it is used later somewhere else. (see also
        // D69571).
-        SimplifyQuery RecQ = Q.getWithoutCondContext();
-        RecQ.CxtI = P->getIncomingBlock(u)->getTerminator();
+        SimplifyQuery RecQ = Q.getWithoutCondContext().getWithInstruction(CxtI);


This is causing the snippet at #124275 to miscompile. Shouldn't we evaluate IncValue by keeping the context instruction fixed to the point of the PN, as the comment seems to suggest?

Looking into it. if I don't have a fix shortly I will revert.

~~And yes I think you are correct, it should be:~~

SimplifyQuery RecQ = Q.getWithoutCondContext().getWithInstruction(P->getIncomingBlock(u)->getTerminator());

I don't think the bug is with the context. The bug is with:

// Check for cases of duplicate successors. if ((TrueSucc == P->getParent()) != (FalseSucc == P->getParent())) { // If we're using the false successor, invert the predicate. if (FalseSucc == P->getParent()) Pred = CmpInst::getInversePredicate(Pred);

~~We assume that either TrueSucc == P->getParent() or FalseSucc == P->getParent(). Which isn't true anymore.~~ Err that the CtxI is leading into P.

I will have patch up shortly.

see: #124481

Fixes llvm#124275 Bug was introduced by llvm#114689 Now that computeKnownBits supports breaking out of recursive Phi nodes, `IncValue` can be an operand of a different Phi than `P`. This breaks the previous assumptions we had when using the possibly condition at `CxtI` to constrain `IncValue`.

…ts (#124481) - **[ValueTracking] Add test for issue 124275** - **[ValueTracking] Fix bug of using wrong condition for deducing KnownBits** Fixes #124275 Bug was introduced by #114689 Now that computeKnownBits supports breaking out of recursive Phi nodes, `IncValue` can be an operand of a different Phi than `P`. This breaks the previous assumptions we had when using the possibly condition at `CxtI` to constrain `IncValue`.

…ing KnownBits (#124481) - **[ValueTracking] Add test for issue 124275** - **[ValueTracking] Fix bug of using wrong condition for deducing KnownBits** Fixes llvm/llvm-project#124275 Bug was introduced by llvm/llvm-project#114689 Now that computeKnownBits supports breaking out of recursive Phi nodes, `IncValue` can be an operand of a different Phi than `P`. This breaks the previous assumptions we had when using the possibly condition at `CxtI` to constrain `IncValue`.

goldsteinn requested a review from nikic as a code owner November 3, 2024 00:19

goldsteinn requested review from dtcxzyw, hiraditya and davemgreen and removed request for nikic November 3, 2024 00:19

llvmbot added llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Nov 3, 2024

goldsteinn requested a review from nikic November 3, 2024 00:19

goldsteinn mentioned this pull request Nov 3, 2024

Task submission dtcxzyw/llvm-opt-benchmark#1312

Open

dtcxzyw mentioned this pull request Nov 3, 2024

pre-commit: PR114689 dtcxzyw/llvm-opt-benchmark#1607

Closed

goldsteinn force-pushed the goldsteinn/value-tracking-select-phi-handling-cont branch from 3dfe4f0 to 0c969e6 Compare November 3, 2024 00:39

dtcxzyw mentioned this pull request Nov 3, 2024

Fuzz 114689 dtcxzyw/llvm-fuzz-service#29

Closed

dtcxzyw reviewed Nov 3, 2024

View reviewed changes

goldsteinn force-pushed the goldsteinn/value-tracking-select-phi-handling-cont branch from 0c969e6 to 7294978 Compare November 3, 2024 07:51

goldsteinn changed the title ~~[ValueTracking] Handle recursive select/PHI in ComputeKnownBits/IsKnownNonZero~~ [ValueTracking] Handle recursive select/PHI in ComputeKnownBits Nov 3, 2024

tschuett reviewed Nov 3, 2024

View reviewed changes

nikic reviewed Nov 3, 2024

View reviewed changes

llvm/lib/Analysis/ValueTracking.cpp Outdated Show resolved Hide resolved

nikic reviewed Nov 3, 2024

View reviewed changes

nikic mentioned this pull request Nov 3, 2024

[ValueTracking] Compute known bits from recursive select/phi #113707

Merged

tschuett reviewed Nov 3, 2024

View reviewed changes

nikic mentioned this pull request Nov 5, 2024

[ValueTracking] Don't special case depth for phi of select #114996

Merged

goldsteinn added 2 commits January 10, 2025 18:02

[ValueTracking] Add tests for handling recursive select/phi in comput…

826cd52

…eKnownBits/isKnownNonZero; NFC

[ValueTracking] Handle recursive PHI's in computeKnownBits

256d0ee

Finish porting llvm#114008 to `KnownBits` (Follow up to llvm#113707).

goldsteinn force-pushed the goldsteinn/value-tracking-select-phi-handling-cont branch from 7fd4687 to 256d0ee Compare January 11, 2025 00:15

llvmbot added the llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes label Jan 11, 2025

nikic approved these changes Jan 22, 2025

View reviewed changes

goldsteinn merged commit a56ba1f into llvm:main Jan 22, 2025
9 checks passed

antoniofrighetto reviewed Jan 25, 2025

View reviewed changes

goldsteinn mentioned this pull request Jan 26, 2025

[ValueTracking] Fix bug of using wrong condition for deducing KnownBits #124481

Merged

[ValueTracking] Handle recursive select/PHI in ComputeKnownBits #114689

[ValueTracking] Handle recursive select/PHI in ComputeKnownBits #114689

Uh oh!

Conversation

goldsteinn commented Nov 3, 2024

Uh oh!

llvmbot commented Nov 3, 2024

Uh oh!

llvmbot commented Nov 3, 2024

Uh oh!

github-actions bot commented Nov 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

goldsteinn commented Nov 3, 2024

Uh oh!

nikic commented Nov 3, 2024

Uh oh!

goldsteinn commented Nov 3, 2024

Uh oh!

nikic commented Nov 3, 2024

Uh oh!

goldsteinn commented Nov 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goldsteinn commented Jan 11, 2025

Uh oh!

goldsteinn commented Jan 20, 2025

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goldsteinn Jan 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goldsteinn Jan 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Nov 3, 2024 •

edited

Loading

goldsteinn Jan 26, 2025 •

edited

Loading

goldsteinn Jan 26, 2025 •

edited

Loading