From 408158e9c65c6fe46a30a0750e7c8affe4a5d69e Mon Sep 17 00:00:00 2001
From: Sanjay Patel <spatel@rotateright.com>
Date: Mon, 14 Aug 2017 15:13:46 +0000
Subject: [PATCH] [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
---
 lib/Transforms/Scalar/BDCE.cpp                 | 17 +++++++----------
 test/Transforms/BDCE/invalidate-assumptions.ll | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/lib/Transforms/Scalar/BDCE.cpp b/lib/Transforms/Scalar/BDCE.cpp
index 5677ac91d6f..2e5618686ec 100644
--- 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 094c5ec6bd9..d165d74be86 100644
--- 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
+}
+
-- 
2.40.0