[PATCH] D37702: [LV] Clamp the VF to the trip count

[Anna Thomas via Phabricator via llvm-commits]

anna marked 2 inline comments as done.
anna added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:6179
   // If we optimize the program for size, avoid creating the tail loop.
-  unsigned TC = PSE.getSE()->getSmallConstantTripCount(TheLoop);
   DEBUG(dbgs() << "LV: Found trip count: " << TC << '\n');
 
----------------
Ayal wrote:
> Probably good to hoist this DEBUG line along with computing TC earlier.
When `OptForSize` is true (i.e. at this point in the code), we know for sure that a small trip count value returns a constant trip count or value of 1, if we didn't know the trip count.

I left this here rather than move up because there maybe *more* spurious cases where we do not have a *small* trip count, and the DEBUG statement wouldn't be useful.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:6258
     SmallVector<unsigned, 8> VFs;
     unsigned NewMaxVectorSize = WidestRegister / SmallestType;
     for (unsigned VS = MaxVectorSize; VS <= NewMaxVectorSize; VS *= 2)
----------------
Ayal wrote:
> While we're at it, in this or a follow-up patch:
>   - Suggest to also clamp NewMaxVectorSize above to TC.
>   - VS below should start from MaxVectorSize*2 instead of MaxVectorSize; it's pointless and wasteful to calculateRegisterUsage for MaxVectorSize, as the latter will be returned by default anyway.
>   - Optimize calculateRegisterUsage() for the case where VFs is empty.
> 
> Doing this will also take care of your early exit above.
To me, it seems rather than clamping the `NewMaxVectorSize`, explicitly showing the early return is clearer. 
The second and third suggestions I'll do in a follow-up patch. 

Also, an additional point to follow up: I think we can early return in even earlier case -  when VF is set to 1 because the target has no vector registers. 


https://reviews.llvm.org/D37702