[PATCH] D27919: [Loop Vectorizer] Interleave vs Gather - in some cases Gather is better.

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper accepted this revision.
mkuper added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:2120
+    if (VF == 1 || Uniforms.count(VF))
+      return;
+    setCostBasedWideningDecision(VF);
----------------
delena wrote:
> mkuper wrote:
> > delena wrote:
> > > mkuper wrote:
> > > > I'm still not sure I understand why this gets called twice for a user-provided VF. Could you explain again?
> > > We call this function from multiple places. From calculateRegisterUsage(), selectVectorizationFactor() - user-defined-VF and from expectedCost(). 
> > > I just prevent recalculation.
> > I understand, I'm just wondering whether we really need to do that.
> > Anyway, it's not a new problem, we don't have to solve it here.
> In the current version, before my changes,  we calculate Uniforms and Scalars once and do this in Legality. In this patch, I moved Uniforms and Scalars from Legality to the Cost Model and calculate them per VF. I did not find a single place to put the call, I'm calling the collectUniformsAndScalars() from multiple places. Doing that, I want to prevent the data recalculation.
> 
> I suppose that finding a right single place for calling collectUniformsAndScalars() is possible, but it will require additional movements in selectVectorizationFactor(). I think it can be done in a separate patch.
Ah, ok, got it. Sure, sounds good as a follow-up.


================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:7069
+        Cost = InterleaveCost;
+      } else if (GatherScatterCost < ScalarizationCost) {
+        Decision = CM_GatherScatter;
----------------
delena wrote:
> mkuper wrote:
> > So, in case the costs are equal, you prefer scalarization to interleaving, and interleaving to scatter/gather?
> > (In theory it shouldn't matter what happens when the costs are equal, just making sure this I understand.)
> Matthew asked to change:
> >>>"I think we're fairly consistent in other places where we compare costs, that we prefer the scalar version if there's no benefit for vectorization. So if the scalarization cost is <= the other costs, I think we should go with that."
Ah, ok, missed that. Could you please add this as an explicit comment?


Repository:
  rL LLVM

https://reviews.llvm.org/D27919