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

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

In https://reviews.llvm.org/D47139#1109102, @Ka-Ka wrote:

> I end up with infinite loops when trying the lazy approach. As I'm not really familiar with the the algorithm used, I instead made this minimal change to fix the problem I saw in pr37539.


I see.  However, the current patch is also risky in the same way (in that it looks like it may lead to infinite loops), but I don't know the algorithm here either.  Unless the current patch is clearly not going to introduce infinite loops in your understanding, I'd suggest taking a closer look at making the lazy deletion work, or have some other scheme of avoiding the reset-to-beginning behavior.



================
Comment at: lib/Transforms/Scalar/NaryReassociate.cpp:251
+            // beginning, reset the iterator and start over from the beginning
+            I = BB->begin();
+            continue;
----------------
This bit me is worrying me somewhat -- can this lead to an infinite loop (where we reassociate from A to B then back)?


Repository:
  rL LLVM

https://reviews.llvm.org/D47139