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

[silviu.baranga@arm.com via llvm-commits]

sbaranga added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5872
+        continue;
+      Value *Ptr = getPointerOperand(Member);
+      int Stride = getPtrStride(PSE, Ptr, TheLoop, Strides, /*Assume=*/false,
----------------
dorit wrote:
> dorit wrote:
> > sbaranga wrote:
> > > I think there is another case here when we don't need to check the wrapping: if the group has the first and last pointers, and we know they both don't wrap.
> > > 
> > > We should be able to apply this for versioning later and have at most two no-wrap checks per interleaved group.
> > So, trying to summarize:
> > 1) Initial check with Assume=true, ShouldCheckWrap=false;
> > 2) Revisit only groups with gaps as follows: 
> > - if first and last elements are accessed: skip further checking.
> > - otherwise – check the first and last pointers of the group with Assume=false (for now), ShouldCheckWrap=true;
> > 
> > Silviu, is that what you meant?
> > 
> 3 and 4 above were meant to be sub-bullets of item 2...:
> 
> 1.Initial check with Assume=true, ShouldCheckWrap=false;
> 2.Revisit only groups with gaps as follows:
>    - if first and last elements are accessed: skip further checking.
>    - otherwise – check the first and last pointers of the group with Assume=false (for now), ShouldCheckWrap=true;
> 
I agree with point 1)
For point 2) I meant something slightly different:
  - if first and last elements are accessed *and* we know they don't wrap, skip further checking (because it implies that all the pointers in the group don't wrap).
  - if the group is full, skip further checking
  - otherwise (as you said) check the first and last pointers of the group with Assume=false (for now), ShouldCheckWrap=true;

I think optimizing this is a bit open-ended, but we could definitely go further (feel free to omit this for this commit):

If we are peeling the loop and we know that the first pointer doesn't wrap then we can deduce that all pointers in the group don't wrap.

This means that we can forcefully peel the loop in order to only have to check the first pointer for no-wrap. When 
we'll change to use Assume=true,ShouldCheckWrap=true we'll only need at most one runtime check per interleaved group (I think this is probably optimal).


================
Comment at: llvm/test/Transforms/LoopVectorize/interleaved-accesses-2.ll:16
+; with proper threshold checks.
+; XFAIL: *
+
----------------
dorit wrote:
> sbaranga wrote:
> > mssimpso wrote:
> > > 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?
> > I think having Assume=true for the initial check should be fine.
> Right; 
> 
> Silviu, the example you gave earlier for a strided access that requires Assume=true: How would you turn it into a test that doesn't fail  vectorization on "SCEV could not compute the loop exit count"...? 
I've tried the following function, and you do have to have assume=true for it in order to form interleaved groups

 void func(unsigned * restrict a, unsigned * restrict b, unsigned char x, unsigned char y) {
     int i = 0;
     for (unsigned char index = x; i < y; index +=2, ++i)
       b[i] = a[index] * 2;
   
   }


https://reviews.llvm.org/D25276