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

[hfinkel@anl.gov via llvm-commits]

hfinkel added inline comments.

================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:485
@@ +484,3 @@
+    if (const SCEVConstant *IncSCEV = dyn_cast<SCEVConstant>(SCEVExpr)) {
+      const PointerType *PTy = dyn_cast<PointerType>(IV->getType());
+      Type *ElTy = PTy->getElementType();
----------------
dyn_cast -> cast (you've already tested IV->getType()->isPointerTy()).

================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:513
@@ +512,3 @@
+        break;
+    }
+  }
----------------
Don't you need an:

  } else {
    break;
  }

here. If the MulSCEV operands are a constant and some other expression, you don't always want to return the constant). Also, do you want to look through extensions and/or truncations here?


================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:1340
@@ +1339,3 @@
+
+  const SCEVAddRecExpr *H = nullptr;
+  const SCEV *SizeOfExpr = nullptr;
----------------
I realize it was like this before, but we should name this something more explanatory than H.

================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:1342
@@ +1341,3 @@
+  const SCEV *SizeOfExpr = nullptr;
+  const SCEV *OneExpr =
+      SE->getConstant(RealIVSCEV->getType(), Negative ? -1 : 1);
----------------
This is not really just 'One', how about we name it IncExpr, or IncrementExpr, etc.?

================
Comment at: test/Transforms/LoopReroll/ptrindvar.ll:42
@@ +41,2 @@
+}
+
----------------
We should also have a test case where the pointer operand is being decremented.



Repository:
  rL LLVM

http://reviews.llvm.org/D13151