[PATCH] D53876: Preserve loop metadata when splitting exit blocks

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:84
+    MDNode *OuterLoopMD = nullptr;
+    if (LI->isLoopHeader(BB)) {
+      OuterLoopMD = LI->getLoopFor(BB)->getLoopID();
----------------
`{ }` not needed here.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:86
+      OuterLoopMD = LI->getLoopFor(BB)->getLoopID();
+      // getLoopID() also verifies that all latches have the same metadata
+    }
----------------
Dot in the end of sentence.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:96
+    if (NewExitBB && OuterLoopMD &&
+        LI->getLoopFor(NewExitBB) == LI->getLoopFor(BB)) {
+      // Every pred of the new block should be a latch for the outer loop,
----------------
Is it possible that their loops don't match? If no, please turn it into an assert. If yes, please add a test on this situation.


================
Comment at: test/Transforms/LoopSimplify/preserve-llvm-loop-metadata2.ll:34
+  %sub = add nsw i64 %j.0, -2
+  %arrayidx = getelementptr inbounds double, double* %a, i64 %sub
+  %0 = load double, double* %arrayidx, align 8, !tbaa !4
----------------
Can the test be reduced to make it clear that it's only supposed to test metadata and not fp computations? I think it can be just reduced to having just blocks with counters.

Maybe also makes sense to rename blocks to smth like `outer.loop`, `inner.loop`.


Repository:
  rL LLVM

https://reviews.llvm.org/D53876