[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

@nikic Is this OK or do you want it split?



================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:661
+      }
+    }
   }
----------------
atmnpatel wrote:
> nikic wrote:
> > These fixes look unrelated. Is it possible to test them separately?
> So my understanding is that the actual line that fixes the compile time error is 651, that is, just having that line fixes the compile time error. I would assume its because before I didn't tell the dominator tree to remove the edge connecting the preheader and header, and not having that cascade, GVN was unable to iterate properly in some cases over the (now) dead blocks because it wasn't updated in LLVM's internal structures. The actual error was from the iteration in GVN::assignValNumForDeadCode() where it would try to iterate through a block that partially existed but didn't really.
> 
> The lines 652-660 that update MemorySSA I added because in the other more general case above, we seem to update MemorySSA right after updating the Dominator Tree.
Nit: Move DTU into the conditional


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86844