[PATCH] [SLSR] handle candidate form &B[i * S]

[Jingyue Wu]

================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:214
@@ +213,3 @@
+
+  if (GlobalVariable *GV = dyn_cast<GlobalVariable>(GEP->getPointerOperand())) {
+    BaseGV = GV;
----------------
hfinkel wrote:
> Don't need the { } here.
Done.

================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:366
@@ +365,3 @@
+      ConstantInt *RHS = nullptr;
+      // TODO: handle shl. e.g., we could treat (S << 2) as (S * 4).
+      if (match(Idx, m_Mul(m_Value(LHS), m_ConstantInt(RHS)))) {
----------------
hfinkel wrote:
> jingyue wrote:
> > hfinkel wrote:
> > > Can't you use SCEV to do that here? If you do, then you get the shift case for free. If the problem here is the loss of the information on the NSW, please comment on that.
> > > 
> > NSW is one problem. The other problem, as I replied earlier, is that this may complicate the rewriting phase because the rewriting would have to translate SCEVs back to IR instructions. For example, suppose `S = a + b` and the immediate basis of `X = S * 4` is `Y = S * 3`. The current code simply rewrites `X` as `Y + S` without tracing into how S is computed. However, if we use SCEV, we would represent X as SCEV `(a + b) * 4` and Y as SCEV `(a + b) * 3`, and rewrite Y as SCEV `X + (a + b)`. After that, we would need to translate SCEV `X + (a + b)` back into IR instructions `X + (a + b)` and further into `X + S`. While I think this is doable, given the relatively simple pattern matching this pass currently has, I don't feel it's worthwhile leveraging SCEVExpander or such to rewrite SCEV into IR instructions. 
> Okay, please add a detailed comment about this in the code.
Done. 

================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:444
@@ +443,3 @@
+      } else {
+        // C = inttoptr(ptrtoint(Basis) + Bump)
+        Reduced = Builder.CreatePtrToInt(Basis.Ins, IntPtrTy);
----------------
hfinkel wrote:
> No, please don't do that. Cast the pointer to an i8* (in the appropriate address space), and use a GEP.
> 
Done. Also change BumpWithGEP to BumpWithUglyGEP because we use GEPs in both cases. 

================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:93
@@ +92,3 @@
+          Basis(nullptr) {}
+    Type CandidateType;
+    const SCEV *Base;
----------------
sanjoy wrote:
> Very minor:  I'd call this `Kind` to differentiate from `llvm::Type`.
Done. 

================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:327
@@ +326,3 @@
+  if (MaxValue.getZExtValue() >= ElementSize) {
+    // I = B + sext((Idx * ElementSize) *s S)
+    ConstantInt *ScaledIdx = ConstantInt::get(
----------------
jingyue wrote:
> sanjoy wrote:
> > Unless I'm missing some larger context; for this equivalence to hold, you'll have to prove that `Idx * S * ElementSize` does not sign-overflow.  For instance, consider:
> > 
> > `Idx->getType()` == `i8`, pointers are 16 bit, `Idx` == `4`, `S` == `4` and `ElementSize` == `24`.  `sext(Idx *s S) *s ElementSize` is `i16 384` while `sext((Idx * ElementSize) *s S)` is `-128`.
> Thanks for pointing this out! You are right. I'll fix it. 
Done. We now transform
```
sext(Idx *s S) *s ElementSize
```
to
```
(sext(Idx) * ElementSize) * S
```

================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:444
@@ +443,3 @@
+        // C = inttoptr(ptrtoint(Basis) + Bump)
+        Reduced = Builder.CreatePtrToInt(Basis.Ins, IntPtrTy);
+        Reduced = Builder.CreateAdd(Reduced, Bump, "", false, true);
----------------
sanjoy wrote:
> Have you considered doing a `bitcast` to `i8*`, gep on that `i8*` and then back instead?  That will preserve pointer-ness throughout.
Done. 

================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:451
@@ +450,3 @@
+  default:
+    assert(false && "CandidateType is invalid");
+  };
----------------
sanjoy wrote:
> Maybe use `llvm_unreachable` here?
Done.

http://reviews.llvm.org/D7459

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