[PATCH] D100121: [LV] Let selectVectorizationFactor reason directly on VectorizationFactor.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationPlanner.h:203
+  // Returns true if this VectorizationFactor is more profitable than \p rhs.
+  bool isMoreProfitableThan(const VectorizationFactor &rhs) const {
+    InstructionCost::CostType ThisCost = *Cost.getValue();
----------------
sdesmalen wrote:
> dmgreen wrote:
> > rhs -> RHS
> > 
> > If the backend knew that the SVE vector length was 256, as opposed 128, how would it best communicate that information to here?
> Doing it here without context would be a bit difficult, as it needs information from CostModel/TargetTransformInfo in order to know which one to favour. Hence my reason to initially create this method in the LV CostModel (see the previous revision https://reviews.llvm.org/D100121?id=336138), so it would mean either passing that information separately, or moving it to the other class again.
Yeah, I think that makes sense. If we have a good reason to put it into the cost model (and @bmahjour doesn't object), then keeping it in the cost model so it could have access to TTI etc would make sense to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100121