[PATCH] D31281: [LoopUnroll] Remap references in peeled iteration

[Michael Kuperstein via Phabricator via llvm-commits]

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

LGTM, but please fix the test before committing.



================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:419
+
+    // Rewrite the cloned instruction operands to use the values created when
+    // the clone is created.
----------------
sepavloff wrote:
> mkuper wrote:
> > Something here is wrong. There's already a call to remapInstructionsInBlocks in line 443 (original) that does exactly this, so everything ought to be updated correctly. If it isn't, then something's going wrong between here and there, and I'm not sure what it is.
> Indeed, you are right. But the remapping is made too late, it must be done prior to dominator tree calculation, which in turn must be done prior to call to `SplitBlock`.
Ok, this makes more sense. So what happened here isn't that we generated bad branches, it's that we produced a malformed dominator tree?
Anyway, thanks for catching this!


================
Comment at: test/Transforms/LoopUnroll/peel-loop.ll:99
+; Check if loop composed of several BB is peeled correctly.
+declare void @funcb()
+define void @funca(i8* readnone %b, i8* readnone %e) local_unnamed_addr {
----------------
Please make the test actually test its output (in particular, that we actually peeled the loop).
I think you can just run the update script on it.


https://reviews.llvm.org/D31281