[PATCH] [LoopDist/LoopVer] Move LoopVersioning to a new module, NFC

Adam Nemet anemet at apple.com
Tue Jun 23 11:09:46 PDT 2015


================
Comment at: lib/Transforms/Utils/LoopVersioning.cpp:41
@@ +40,3 @@
+  BasicBlock *MemCheckBB = VersionedLoop->getLoopPreheader();
+  std::tie(FirstCheckInst, MemRuntimeCheck) =
+      LAI.addRuntimeCheck(MemCheckBB->getTerminator(), PtrToPartition);
----------------
ashutosh.nema wrote:
> Loop pre header may be null, probably you need to check that.
> If you are assuming a preheader then you can put a assert here.
Sounds good.  I'll put it in the constructor next to the other assert.

================
Comment at: lib/Transforms/Utils/LoopVersioning.cpp:60
@@ +59,3 @@
+      cloneLoopWithPreheader(PH, MemCheckBB, VersionedLoop, VMap, ".lver.orig",
+                             LI, DT, NonVersionedLoopBlocks);
+  remapInstructionsInLoop(NonVersionedLoopBlocks, VMap);
----------------
ashutosh.nema wrote:
> This function does not care about exit blocks, you don't want to clone exit blocks ?
It does not, hence the name ;).  Here we may want to create exist blocks and then clone them to preserve SimplifyLoop.

cloneLoopWithPreheader is used in the LoopDistribution as well and in LoopDistribution there is no point of cloning the exit blocks because the preheader of the next distributed loop becomes the current loop's exit block (satisfying SimplifyLoop).

================
Comment at: lib/Transforms/Utils/LoopVersioning.cpp:75
@@ +74,3 @@
+
+void LoopVersioning::addPHINodes(
+    const SmallVectorImpl<Instruction *> &DefsUsedOutside) {
----------------
ashutosh.nema wrote:
> Add some comments what this function does
There is some before the decl but Hal also requested more comments there, so more to follow.

Thanks for your review.

Adam

http://reviews.llvm.org/D10577

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list