[llvm] r367173 - [InstSimplify] remove quadratic time looping (PR42771)

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Sat Jul 27 07:05:51 2019
New Revision: 367173

URL: http://llvm.org/viewvc/llvm-project?rev=367173&view=rev
Log:
[InstSimplify] remove quadratic time looping (PR42771)

The test case from:
https://bugs.llvm.org/show_bug.cgi?id=42771
...shows a ~30x slowdown caused by the awkward loop iteration (rL207302) that is
seemingly done just to avoid invalidating the instruction iterator. We can instead
delay instruction deletion until we reach the end of the block (or we could delay
until we reach the end of all blocks).

There's a test diff here for a degenerate case with llvm.assume that is not
meaningful in itself, but serves to verify this change in logic.

This change probably doesn't result in much overall compile-time improvement
because we call '-instsimplify' as a standalone pass only once in the standard
-O2 opt pipeline currently.

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

Modified:
    llvm/trunk/lib/Transforms/Scalar/InstSimplifyPass.cpp
    llvm/trunk/test/Transforms/InstSimplify/assume.ll

Modified: llvm/trunk/lib/Transforms/Scalar/InstSimplifyPass.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/InstSimplifyPass.cpp?rev=367173&r1=367172&r2=367173&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/InstSimplifyPass.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/InstSimplifyPass.cpp Sat Jul 27 07:05:51 2019
@@ -33,37 +33,33 @@ static bool runImpl(Function &F, const S
   bool Changed = false;
 
   do {
-    for (BasicBlock *BB : depth_first(&F.getEntryBlock())) {
-      // Here be subtlety: the iterator must be incremented before the loop
-      // body (not sure why), so a range-for loop won't work here.
-      for (BasicBlock::iterator BI = BB->begin(), BE = BB->end(); BI != BE;) {
-        Instruction *I = &*BI++;
-        // The first time through the loop ToSimplify is empty and we try to
-        // simplify all instructions.  On later iterations ToSimplify is not
+    for (BasicBlock &BB : F) {
+      SmallVector<Instruction *, 8> DeadInstsInBB;
+      for (Instruction &I : BB) {
+        // The first time through the loop, ToSimplify is empty and we try to
+        // simplify all instructions. On later iterations, ToSimplify is not
         // empty and we only bother simplifying instructions that are in it.
-        if (!ToSimplify->empty() && !ToSimplify->count(I))
+        if (!ToSimplify->empty() && !ToSimplify->count(&I))
           continue;
 
-        // Don't waste time simplifying unused instructions.
-        if (!I->use_empty()) {
-          if (Value *V = SimplifyInstruction(I, SQ, ORE)) {
+        // Don't waste time simplifying dead/unused instructions.
+        if (isInstructionTriviallyDead(&I)) {
+          DeadInstsInBB.push_back(&I);
+        } else if (!I.use_empty()) {
+          if (Value *V = SimplifyInstruction(&I, SQ, ORE)) {
             // Mark all uses for resimplification next time round the loop.
-            for (User *U : I->users())
+            for (User *U : I.users())
               Next->insert(cast<Instruction>(U));
-            I->replaceAllUsesWith(V);
+            I.replaceAllUsesWith(V);
             ++NumSimplified;
             Changed = true;
+            // A call can get simplified, but it may not be trivially dead.
+            if (isInstructionTriviallyDead(&I))
+              DeadInstsInBB.push_back(&I);
           }
         }
-        if (RecursivelyDeleteTriviallyDeadInstructions(I, SQ.TLI)) {
-          // RecursivelyDeleteTriviallyDeadInstruction can remove more than one
-          // instruction, so simply incrementing the iterator does not work.
-          // When instructions get deleted re-iterate instead.
-          BI = BB->begin();
-          BE = BB->end();
-          Changed = true;
-        }
       }
+      RecursivelyDeleteTriviallyDeadInstructions(DeadInstsInBB, SQ.TLI);
     }
 
     // Place the list of instructions to simplify on the next loop iteration

Modified: llvm/trunk/test/Transforms/InstSimplify/assume.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstSimplify/assume.ll?rev=367173&r1=367172&r2=367173&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstSimplify/assume.ll (original)
+++ llvm/trunk/test/Transforms/InstSimplify/assume.ll Sat Jul 27 07:05:51 2019
@@ -33,15 +33,15 @@ define i64 @PR31809() !dbg !7 {
 }
 
 ; Similar to above: there's no way to know which assumption is truthful,
-; so just don't crash. The second icmp+assume gets processed later, so that
-; determines the return value.
+; so just don't crash.
 
 define i8 @conflicting_assumptions(i8 %x) !dbg !10 {
 ; CHECK-LABEL: @conflicting_assumptions(
+; CHECK-NEXT:    [[ADD:%.*]] = add i8 [[X:%.*]], 1, !dbg !10
 ; CHECK-NEXT:    call void @llvm.assume(i1 false)
-; CHECK-NEXT:    [[COND2:%.*]] = icmp eq i8 %x, 4
+; CHECK-NEXT:    [[COND2:%.*]] = icmp eq i8 [[X]], 4
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[COND2]])
-; CHECK-NEXT:    ret i8 5
+; CHECK-NEXT:    ret i8 [[ADD]]
 ;
   %add = add i8 %x, 1, !dbg !11
   %cond1 = icmp eq i8 %x, 3