[PATCH] D57382: [LV] Add planForInterleaving helper, to create VPlan if they are missing.

Thu Jan 31 11:36:10 PST 2019

dcaballe added inline comments.

================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6158
   if (MaxVF == 1)
     return NoVectorization;

----------------
fhahn wrote:
> dcaballe wrote:
> > I wondered if we could make `LoopVectorizationPlanner::plan` more interleave group aware, since the code above also builds a vplan for VF=1 and and returns NoVectorization. Maybe we don't have to modify `computeMaxVF` but just pass `InterleaveGroup` flag to `LoopVectorizationPlanner::plan` and make sure a vplan is built when the flag is true. We are already passing `UserVF` so it would be more of the same. In that way we wouldn't need a independent handling for interleaving cases. I wouldn't worried about compile time since the code executed for `InterleaveGroup` would be mostly the code that you have in the new `planForInterleaving`, right?. What do you think?
> The interleave count is currently determined by `CM.selectInterleaveCount` (or the user provided one, as in the test case) and depends on the vectorization factor. It might make sense to determine the interleave count in `plan` as well, and return a pair of VF and interleave count. What do you think?
> 
> (Please note that the interleave count I am talking about is not related to (Memory) InterleaveGroups)
Sorry, I didn't mean interleave "group" but interleave "loop" in my previous message (including the flag, line 7212) :).
Yes, IMO what you suggest is the the right think to do. VPlan is meant to be used for vectorization and interleaving. We mostly focused on the first one but we should catch up on the second. 

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

https://reviews.llvm.org/D57382