[BDCE] reduce scope of an assert (PR34179)

The assert was added with r310779 and is usually correct,
but as the test shows, not always. The 'volatile' on the
load is needed to expose the faulty path because without
it, DemandedBits would return that the load is just dead
rather than not demanded, and so we wouldn't hit the
bogus assert.

Also, since the lambda is just a single-line now, get rid
of it and inline the DB.isAllOnesValue() calls.

This should fix (prevent execution of a faulty assert):
https://bugs.llvm.org/show_bug.cgi?id=34179

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@310842 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/Scalar/BDCE.cpp b/lib/Transforms/Scalar/BDCE.cpp
index 5677ac91d6f0fb5d8037753b6eb9694c96018598..2e5618686ec21d4fee07826ca82db0a78504d517 100644 (file)
--- a/lib/Transforms/Scalar/BDCE.cpp
+++ b/lib/Transforms/Scalar/BDCE.cpp
@@ -40,20 +40,15 @@ STATISTIC(NumSimplified, "Number of instructions trivialized (dead bits)");
 /// instruction may need to be cleared of assumptions that can no longer be
 /// guaranteed correct.
 static void clearAssumptionsOfUsers(Instruction *I, DemandedBits &DB) {
-  // Any value we're trivializing should be an integer value, and moreover,
-  // any conversion between an integer value and a non-integer value should
-  // demand all of the bits. This will cause us to stop looking down the
-  // use/def chain, so we should only see integer-typed instructions here.
-  auto isExternallyVisible = [](Instruction *I, DemandedBits &DB) {
-    assert(I->getType()->isIntegerTy() && "Trivializing a non-integer value?");
-    return DB.getDemandedBits(I).isAllOnesValue();
-  };
+  assert(I->getType()->isIntegerTy() && "Trivializing a non-integer value?");
 
   // Initialize the worklist with eligible direct users.
   SmallVector<Instruction *, 16> WorkList;
   for (User *JU : I->users()) {
+    // If all bits of a user are demanded, then we know that nothing below that
+    // in the def-use chain needs to be changed.
     auto *J = dyn_cast<Instruction>(JU);
-    if (J && !isExternallyVisible(J, DB))
+    if (J && !DB.getDemandedBits(J).isAllOnesValue())
       WorkList.push_back(J);
   }
 
@@ -72,8 +67,10 @@ static void clearAssumptionsOfUsers(Instruction *I, DemandedBits &DB) {
     Visited.insert(J);
 
     for (User *KU : J->users()) {
+      // If all bits of a user are demanded, then we know that nothing below
+      // that in the def-use chain needs to be changed.
       auto *K = dyn_cast<Instruction>(KU);
-      if (K && !Visited.count(K) && !isExternallyVisible(K, DB))
+      if (K && !Visited.count(K) && !DB.getDemandedBits(K).isAllOnesValue())
         WorkList.push_back(K);
     }
   }

diff --git a/test/Transforms/BDCE/invalidate-assumptions.ll b/test/Transforms/BDCE/invalidate-assumptions.ll
index 094c5ec6bd950842987c579b3a431774aae0929e..d165d74be86d697ed2f396f9c5eeaac77d7cfead 100644 (file)
--- a/test/Transforms/BDCE/invalidate-assumptions.ll
+++ b/test/Transforms/BDCE/invalidate-assumptions.ll
@@ -84,3 +84,17 @@ define i1 @poison_on_call_user_is_ok(i1 %b, i8 %x) {
   ret i1 %mul
 }
 
+
+; We were asserting that all users of a trivialized integer-type instruction were
+; also integer-typed, but that's too strong. The alloca has a pointer-type result.
+
+define void @PR34179(i32* %a) {
+; CHECK-LABEL: @PR34179(
+; CHECK-NEXT:    [[T0:%.*]] = load volatile i32, i32* %a
+; CHECK-NEXT:    ret void
+;
+  %t0 = load volatile i32, i32* %a
+  %vla = alloca i32, i32 %t0
+  ret void
+}
+