[llvm] r310842 - [BDCE] reduce scope of an assert (PR34179)

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Mon Aug 14 08:13:46 2017
New Revision: 310842

URL: http://llvm.org/viewvc/llvm-project?rev=310842&view=rev
Log:
[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


Modified:
    llvm/trunk/lib/Transforms/Scalar/BDCE.cpp
    llvm/trunk/test/Transforms/BDCE/invalidate-assumptions.ll

Modified: llvm/trunk/lib/Transforms/Scalar/BDCE.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/BDCE.cpp?rev=310842&r1=310841&r2=310842&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/BDCE.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/BDCE.cpp Mon Aug 14 08:13:46 2017
@@ -40,20 +40,15 @@ STATISTIC(NumSimplified, "Number of inst
 /// 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(Inst
     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);
     }
   }

Modified: llvm/trunk/test/Transforms/BDCE/invalidate-assumptions.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/BDCE/invalidate-assumptions.ll?rev=310842&r1=310841&r2=310842&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/BDCE/invalidate-assumptions.ll (original)
+++ llvm/trunk/test/Transforms/BDCE/invalidate-assumptions.ll Mon Aug 14 08:13:46 2017
@@ -84,3 +84,17 @@ define i1 @poison_on_call_user_is_ok(i1
   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
+}
+