[PATCH] D65336: [InstSimplify] remove quadratic time looping (PR42771)

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked 2 inline comments as done.
spatel added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/InstSimplifyPass.cpp:45-48
         // Don't waste time simplifying unused instructions.
-        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())
-              Next->insert(cast<Instruction>(U));
-            I->replaceAllUsesWith(V);
-            ++NumSimplified;
-            Changed = true;
-          }
-        }
-        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();
+        if (isInstructionTriviallyDead(&I)) {
+          DeadInstsInBB.push_back(&I);
+        } else if (Value *V = SimplifyInstruction(&I, SQ, ORE)) {
----------------
foad wrote:
> spatel wrote:
> > foad wrote:
> > > Why did you change the logic here? Now we might try to simplify an instruction that has no uses but is not trivially dead, which at least makes the comment on L45 slightly wrong.
> > See comment at line 55 - just because an instruction (a call at least) has no uses does not mean it can not be simplified. I think we should use isInstructionTriviallyDead() as our point of truth here rather than mixing that with a use check.
> > 
> > I'll update the comment at line 45 if you agree.
> > just because an instruction (a call at least) has no uses does not mean it can not be simplified
> 
> As I understand it, there is no point simplifying an instruction that has no uses. "Simplifying" does not change the instruction at all, it just returns a pointer to another (simpler) Value that users of the original instruction could use instead. If there are no users, there is no point doing this.
Ok, that sounds good. So let's only try to simplify if an instruction is not trivially dead *and* it has uses. We can only delete the trivially dead, so we only add those to the dead instructions list.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65336/new/

https://reviews.llvm.org/D65336