[PATCH] D151658: [LV] Check if value was already not uniform for previous VF.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal accepted this revision.
Ayal added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4684
+        return false;
+    }
     if (!Legal->isUniformMemOp(*I))
----------------
fhahn wrote:
> Ayal wrote:
> > fhahn wrote:
> > > Ayal wrote:
> > > > This looks right.
> > > > Also holds for non-MemOpUse?
> > > > Furthermore, if value is uniform for PrevVF, suffice to only check if SCEVs of lanes 0 and VF-1 are equal.
> > > > Also holds for non-MemOpUse?
> > > 
> > > Yes, but the loop that uses `isUniformMemOpUse` also collects pointers where only lane 0 is demanded (for widened memory ops), so checking in the loop directly isn't possible without first dis-entangling this logic.
> > > 
> > > 
> > > > Furthermore, if value is uniform for PrevVF, suffice to only check if SCEVs of lanes 0 and VF-1 are equal.
> > > 
> > > Hmm, I think we would have to check if lane 0 matches the lanes PrevVF..VF-1? Might be good as follow-up, as it will require adjusting the API a bit I think to provide the additional info.
> > >> Also holds for non-MemOpUse?
> > 
> > > Yes, but the loop that uses isUniformMemOpUse also collects pointers where only lane 0 is demanded (for widened memory ops), so checking in the loop directly isn't possible without first dis-entangling this logic.
> > 
> > Agreed, uniformness should be disentangled from first-lane-used-ness.
> > 
> > >> Furthermore, if value is uniform for PrevVF, suffice to only check if SCEVs of lanes 0 and VF-1 are equal.
> > 
> > > Hmm, I think we would have to check if lane 0 matches the lanes PrevVF..VF-1? Might be good as follow-up, as it will require adjusting the API a bit I think to provide the additional info.
> > 
> > Stitching piecewise uniformity - if a value is uniform across PrevVF then its SCEVs for lanes {0,1,..,PrevVF-1} are equal to each other, and so are those of lanes/offsets {PrevVF,PrevVF+1...,VF-1}, right? If so then suffice the select and compare one sample from each for the value to be uniform across VF.
> > 
> > In any case these can be dealt with in separate follow-up patches.
> Sounds good, so should this initial patch go in as is for now?
Sure, just noting more room for improving how we go about analyzing uniformity/divergence.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151658/new/

https://reviews.llvm.org/D151658