[PATCH] D10477: Expand loop reroll to handle loop with multiple induction variables and negative increment -part 1/3

[James Molloy]

jmolloy added a comment.

Hi,

This is looking a lot better, thanks!

James


================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:372
@@ +371,3 @@
+                     TargetLibraryInfo *TLI,
+                     DenseMap<Instruction *, int64_t> &IncrMap)
+          : Parent(Parent), L(L), SE(SE), AA(AA), TLI(TLI), IV(IV),
----------------
This should be const, to notify a user that it is not captured or modified.

================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:437
@@ -432,2 +436,3 @@
       UsesTy Uses;
+      DenseMap<Instruction *, int64_t> &IVToIncMap;
     };
----------------
Please add a docstring, like all other variables.

================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:664
@@ -660,1 +663,3 @@
+
+  if (BO && BO->getOpcode() != Instruction::Add)
     return false;
----------------
I think this is still relating to adding GEP support, is it not?

================
Comment at: test/Transforms/LoopReroll/negative.ll:16
@@ +15,3 @@
+;CHECK-LABEL: while.body:
+;CHECK-NEXT:    %indvar = phi i32 [ %indvar.next, %while.body ], [ 0, %while.body.lr.ph ]
+;CHECK-NEXT:    %sum4.015 = phi i64 [ 0, %while.body.lr.ph ], [ %add, %while.body ]
----------------
This test will fail in release builds because registers will lose their names. Please either make it not rely on named variables or add a

    ; REQUIRES: asserts

to the top (please also check the line above is actually correct, I've just pasted it from memory!)


Repository:
  rL LLVM

http://reviews.llvm.org/D10477