Re: [PATCH] D17268: [LAA] Function 'isStridedPtr' returns additional result “Loop *Lp” via function argument and add appropriate checks out of the 'isStridedPtr'.

[Adam Nemet via llvm-commits]

anemet requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4589-4594
@@ -4588,3 +4588,8 @@
     Value *Ptr = LI ? LI->getPointerOperand() : SI->getPointerOperand();
-    int Stride = isStridedPtr(PSE, Ptr, TheLoop, Strides);
-
+    Loop *Lp = nullptr; 
+    int Stride = isStridedPtr(PSE, Ptr, Lp, Strides);
+    
+    // Consider stride to be zero in case of Ptr not strided over TheLoop
+    if (Lp != TheLoop)
+      Stride = 0;
+ 
----------------
Rather than complicating the interface of isStridedPtr, we should just check ScalarEvoluiton::isLoopInvariant on the SCEV of Ptr here.

================
Comment at: test/Analysis/LoopAccessAnalysis/stride-vectorization.ll:1
@@ +1,2 @@
+; RUN: opt -loop-vectorize  -S < %s | FileCheck %s
+; CHECK-LABEL: Test
----------------
Please don't put vectorization-only tests under LAA.  Those should go under Transforms/LoopVectorize.

You *should* have a test under LAA but it should then check the result of the analysis.  See some examples in the directory.

There are ways to combine a single test to get tested both for vectorization and LAA.  There should be examples in the LAA directory.


================
Comment at: test/Analysis/LoopAccessAnalysis/stride-vectorization.ll:3
@@ +2,3 @@
+; CHECK-LABEL: Test
+; CHECK: %vector.body
+
----------------
Ayal wrote:
> better check for a vector type?
+1


http://reviews.llvm.org/D17268