[PATCH] D125574: [BasicBlockUtils] Do not move loop metadata if outer loop header.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:1162
+    Loop *IL = LI->getLoopFor(OldLatch);
+    // It's still possible that OldLatch is the latch of another inner loop, in
+    // which case we do not move the metadata.
----------------
So the interesting case is when the original latch is shared between multiple loops. At the moment, we move the metadata to the new latch of the outer loop. With this patch we keep it at the latch of the inneer loop, which won't be the latch of the outer loop any longer.

I am not sure if either option is more correct/better than the other.

Should we duplicate the metadata and add it to the new latches of both loops? This would be in line with the original IR, where the metadata applied to both loops.


================
Comment at: llvm/test/Transforms/LoopSimplify/update_latch_md2.ll:27
 loop.latch:
   br i1 %p, label %loop.latch, label %loop.header, !llvm.loop !0
 
----------------
Ah, so the the `llvm.loop` metadata here applies to 2 loops. `loop.latch is shared between 2 loops. 


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

https://reviews.llvm.org/D125574