[PATCH] D45191: [LoopReroll] Rewrite induction variable rewriting.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

I had a high-level look and left some minor comments inline. With respect to the overflow, the original code seems to have the same problem, so this patch does not make things worse.

IIUC the induction variables must be wide enough to represent the scaled number of iterations in the original code, but an overflow can happen because we use EQ with the scaled trip count and the new induction variables start at 0. (e.g. if we have something like `for (int64_t i = -100; i < INT64_MAX; i += 10)`). I am probably missing something, but maybe we could avoid the overflow by doing the re-writing of the induction variables/exit conditions closer to the original range for the induction variables?



================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:397
       /// replacement.
       /// @param IterCount The maximum iteration count of L.
+      void replace(const SCEV *LIBETC);
----------------
Stale doxygen comment. Could you document LIBETC?


================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:1409
+
+  // Copute the start and increment for each BaseInst before we start erasing
+  // instructions.
----------------
Typo, Copute -> Compute


================
Comment at: test/Transforms/LoopReroll/basic.ll:86
 ; CHECK: %indvar.next = add i64 %indvar, 1
-; CHECK: %exitcond = icmp eq i64 %indvar, 1499
+; CHECK: %exitcond = icmp eq i32 %0, 1499
 ; CHECK: br i1 %exitcond, label %for.end, label %for.body
----------------
I think we should also check the instruction producing `%0`, which should truncate `%indvar` in this and other changed tests.


Repository:
  rL LLVM

https://reviews.llvm.org/D45191