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

[Jingyue Wu]

Thanks Hal and Sanjoy for the review. I will fix all other comments.


================
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:
> 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. 

================
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(
----------------
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.

http://reviews.llvm.org/D7459

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