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

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

> The patch isn't entirely NFC because it also fixes an issue in selectEpilogueVectorizationFactor, where the cost passed to ProfitableVFs is no longer the total cost, but rather the cost per lane which is also what is used to determine the VF for the vector body loop.

I'm a bit confused by this...before we saved the per-lane cost in `ProfitableVFs` and compared that with the `Result` variable in ` selectEpilogueVectorizationFactor`. The `Result` variable gets initialized as `Disabled` and then gets refined as we go through the `ProfitableVFs`, getting assigned a value from `ProfitableVFs`. That means that the `Result` variable could only get assigned per-lane costs. Since we are now comparing per-lane costs as well, I don't see this change being non-NFC.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1607
+  /// that of B.
+  bool isMoreProfitable(const VectorizationFactor &A,
+                        const VectorizationFactor &B) const;
----------------
I think this should become a member of the `VectorizationFactor` struct....then code like `A.isMoreProfitableThan(B)` reads better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100121