[PATCH] D25276: [LoopVectorizer] Interleaved-mem-accesses analysis and getPtrStride

[Matthew Simpson via llvm-commits]

mssimpso added a comment.

If I understand things correctly now, we're trying to optimize the case where we have a full interleave group where the stride may wrap. So yes I see that we may want ShouldCheckWrap=false, at least initially. I don't think there is any harm in Assume=true for the initial call, though, because we later in Legality (collectLoopUniforms) call isConsecutivePtr for the pointer operand of every memory instruction (with Assume=true). If we don't use Assume=true for a pointer in the interleaved accesses anaysis, we will do so later in the uniformity analysis, so I think we want to do the same thing in both places.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5648-5650
+      int64_t Stride =
+          getPtrStride(PSE, Ptr, TheLoop, Strides,
+                       /*Assume=*/false, /*ShouldCheckWrap=*/false);
----------------
I think this would be a little cleaner if we added a may wrap flag to StrideDescriptor. We would check the wrapping condition initially here and save that in the StrideDescriptor (and in the interleave groups as we build them). Then for removing groups, we could simply check that it is not full and may wrap.


================
Comment at: llvm/test/Transforms/LoopVectorize/interleaved-accesses-2.ll:16
+; with proper threshold checks.
+; XFAIL: *
+
----------------
I would prefer that we not XFAIL a new test. Unless Silviu disagrees with letting Assume=true, I think it would be better to check the actual behavior while keeping the FIXME.

Would we be missing a test though? If Assume=true here, I don't think we will be checking the case that we actually remove a group that is may wrap?


https://reviews.llvm.org/D25276