[PATCH] D47139: [NaryReassociate] Detect deleted instr with WeakTrackingVH

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: lib/Transforms/Scalar/NaryReassociate.cpp:251
+            // beginning, reset the iterator and start over from the beginning
+            I = BB->begin();
+            continue;
----------------
Ka-Ka wrote:
> sanjoy wrote:
> > This bit me is worrying me somewhat -- can this lead to an infinite loop (where we reassociate from A to B then back)?
> I agree that restarting the iteration from BB->begin() is kind of ugly, but it will not lead to a infinite loop. The reason why is that RecursivelyDeleteTriviallyDeadInstructions() is starting deleting from 'I' that is a part of BB (as it looked from the beginning). We end up in this if clause if we, when recursively deleting, end up deleting 'NewI' (the instruction that should replace 'I'). However, as at least 'I' and 'NewI' was deleted, the basic block BB have always shrunk when restarting the iteration from BB-begin().
> 
SGTM. 


Repository:
  rL LLVM

https://reviews.llvm.org/D47139