[llvm] dd48a9b - [ValueTracking] Handle conflicts when computing knownbits of PHI nodes in deadcode; PR65022

[Noah Goldstein via llvm-commits]

Author: Noah Goldstein
Date: 2023-09-01T02:11:50-05:00
New Revision: dd48a9b0561cc65b8597d618f8b286682866c66d

URL: https://github.com/llvm/llvm-project/commit/dd48a9b0561cc65b8597d618f8b286682866c66d
DIFF: https://github.com/llvm/llvm-project/commit/dd48a9b0561cc65b8597d618f8b286682866c66d.diff

LOG: [ValueTracking] Handle conflicts when computing knownbits of PHI nodes in deadcode; PR65022

Bug was introduced in: https://reviews.llvm.org/D157807

The prior logic assumed that the information from the knownbits
returned from analyzing the `icmp` and its operands in the context
basicblock would be consistent.

This is not necessarily the case if we are analyzing deadcode.

For example with `(icmp sgt (select cond, 0, 1), -1)`. If we take
knownbits for the `select` using knownbits from the operator, we will
know the signbit is zero. If we are analyzing a not-taken from based
on the `icmp` (deadcode), we will also "know" that the `select` must
be negative (signbit is one). This will result in a conflict in
knownbits.

The fix is to just give up on analying the phi-node if its deadcode. We 1) don't want to waste time continuing to analyze it and 2) will be removing it (and its dependencies) later.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D158960

Added: 
    llvm/test/Analysis/ValueTracking/knownbits-phi-deadcode.ll

Modified: 
    llvm/lib/Analysis/ValueTracking.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 0ea47f1dffc3ca..801814e0c18a83 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -1480,7 +1480,17 @@ static void computeKnownBitsFromOperator(const Operator *I,
                 Pred = CmpInst::getInversePredicate(Pred);
               // Get the knownbits implied by the incoming phi condition.
               auto CR = ConstantRange::makeExactICmpRegion(Pred, *RHSC);
-              Known2 = Known2.unionWith(CR.toKnownBits());
+              KnownBits KnownUnion = Known2.unionWith(CR.toKnownBits());
+              // We can have conflicts here if we are analyzing deadcode (its
+              // impossible for us reach this BB based the icmp).
+              if (KnownUnion.hasConflict()) {
+                // No reason to continue analyzing in a known dead region, so
+                // just resetAll and break. This will cause us to also exit the
+                // outer loop.
+                Known.resetAll();
+                break;
+              }
+              Known2 = KnownUnion;
             }
           }
         }

diff  --git a/llvm/test/Analysis/ValueTracking/knownbits-phi-deadcode.ll b/llvm/test/Analysis/ValueTracking/knownbits-phi-deadcode.ll
new file mode 100644
index 00000000000000..6ccb87471fcb08
--- /dev/null
+++ b/llvm/test/Analysis/ValueTracking/knownbits-phi-deadcode.ll
@@ -0,0 +1,57 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
+; RUN: opt -passes=loop-deletion -S < %s | FileCheck %s
+; This reduced testcase from pr65022. We are only testing that is doesn't crash.
+
+define void @f(i32 %spec.select) {
+; CHECK-LABEL: define void @f
+; CHECK-SAME: (i32 [[SPEC_SELECT:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[L_OUTER:%.*]]
+; CHECK:       L.outer:
+; CHECK-NEXT:    br label [[IF_END:%.*]]
+; CHECK:       if.end:
+; CHECK-NEXT:    [[DOTB:%.*]] = load i1, ptr null, align 1
+; CHECK-NEXT:    [[TMP0:%.*]] = select i1 [[DOTB]], i32 0, i32 1
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i32 [[TMP0]], -1
+; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN3:%.*]], label [[WHILE_COND_SPLIT_LOOP_EXIT1:%.*]]
+; CHECK:       if.then3:
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       while.cond.split.loop.exit1:
+; CHECK-NEXT:    [[DOTLCSSA:%.*]] = phi i32 [ [[TMP0]], [[IF_END]] ]
+; CHECK-NEXT:    [[NOT_LE:%.*]] = xor i32 [[DOTLCSSA]], 1
+; CHECK-NEXT:    [[TOBOOL7_NOT:%.*]] = icmp eq i32 [[NOT_LE]], 0
+; CHECK-NEXT:    [[SPEC_SELECT3:%.*]] = select i1 [[TOBOOL7_NOT]], i32 0, i32 [[SPEC_SELECT]]
+; CHECK-NEXT:    [[TOBOOL10_NOT:%.*]] = icmp eq i32 [[SPEC_SELECT3]], 0
+; CHECK-NEXT:    br i1 [[TOBOOL10_NOT]], label [[FOR_COND_PREHEADER:%.*]], label [[L_OUTER]]
+; CHECK:       for.cond.preheader:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %L.outer
+
+L.outer:                                          ; preds = %while.cond.split.loop.exit1, %entry
+  br label %if.end
+
+if.end:                                           ; preds = %if.then3, %L.outer
+  %.b = load i1, ptr null, align 1
+  %0 = select i1 %.b, i32 0, i32 1
+  %cmp = icmp sgt i32 %0, -1
+  br i1 %cmp, label %if.then3, label %while.cond.split.loop.exit1
+
+if.then3:                                         ; preds = %if.end
+  br label %if.end
+
+while.cond.split.loop.exit1:                      ; preds = %if.end
+  %.lcssa = phi i32 [ %0, %if.end ]
+  %not.le = xor i32 %.lcssa, 1
+  %tobool7.not = icmp eq i32 %not.le, 0
+  %spec.select3 = select i1 %tobool7.not, i32 0, i32 %spec.select
+  %tobool10.not = icmp eq i32 %spec.select3, 0
+  br i1 %tobool10.not, label %for.cond.preheader, label %L.outer
+
+for.cond.preheader:                               ; preds = %while.cond.split.loop.exit1
+  ret void
+}
+
+; uselistorder directives
+uselistorder i32 0, { 1, 0, 2, 3 }