[PATCH] D9151: Loop Versioning for LICM

[Ashutosh Nema via llvm-commits]

ashutosh.nema added inline comments.

================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:302
@@ +301,3 @@
+  // Loop should have a trip count, if not return false.
+  const SCEV *ExitCount = SE->getBackedgeTakenCount(CurLoop);
+  if (ExitCount == SE->getCouldNotCompute()) {
----------------
hfinkel wrote:
> Why are you checking this?
Loop trip count is required for runtime check generation, as the bound checks are based on this.

================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:386
@@ +385,3 @@
+  // Check memory threshold, should be in limit.
+  if (PointerSet.size() > RuntimeMemoryCheckThreshold) {
+    DEBUG(dbgs() << "    Loop body has pointers more then defined threshold\n");
----------------
hfinkel wrote:
> Shouldn't you use LAI->getNumRuntimePointerChecks() here instead of PointerSet.size()?
‘LAI->getNumRuntimePointerChecks()’ returns number of runtime checks but here we are checking maximum possible pointers in runtime check.

================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:632
@@ +631,3 @@
+char LoopVersioningLICM::ID = 0;
+INITIALIZE_PASS_BEGIN(LoopVersioningLICM, "do-loop-versioning-licm",
+                      "Loop Versioning For LICM", false, false)
----------------
hfinkel wrote:
> do-loop-versioning-licm -> loop-versioning-licm
> 
> (I often just use DEBUG_TYPE for this; there's no reason for them to be out-of-sync)
Tried making it 'loop-versioning-licm' but later found command line error mentioning option registered more than once.
So, made DEBUG_TYPE as “do-loop-versioning-licm”. Hoping this should be OK.



Repository:
  rL LLVM

http://reviews.llvm.org/D9151