[PATCH] D49516: [LoadStoreVectorizer] Use getMinusScev() to compute the distance between two pointers.

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

Adding optional items noticed in post commit review.



================
Comment at: llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:339
   const SCEV *C = SE.getConstant(BaseDelta);
   const SCEV *X = SE.getAddExpr(PtrSCEVA, C);
   if (X == PtrSCEVB)
----------------
It really looks like the new check completely subsumes this check.  Have you tried simplify the code to remove this case?


================
Comment at: llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:351
+
   // Sometimes even this doesn't work, because SCEV can't always see through
   // patterns that look like (gep (ext (add (shl X, C1), C2))). Try checking
----------------
This block may also be subsumed by the subtract.  If not, this would make a good bug against SCEV because it should be able to handle most any reasonable case here.


================
Comment at: llvm/trunk/test/Transforms/LoadStoreVectorizer/AMDGPU/complex-index.ll:7
+
+declare double @llvm.fmuladd.f64(double, double, double)
+
----------------
You really should be able to construct a target independent test for this.  You don't need to exercise the actual vectorization, just the analysis phase.  I'm not sure the code is structured to make this easy, but if it isn't, it really should be.  (To be clear, this is not a must have, just a strong nice to have.)


Repository:
  rL LLVM

https://reviews.llvm.org/D49516