[PATCH] D105473: [LV] Ignore candidate VFs with invalid costs.

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Thanks. This looks good to me minus one nit about having selectUserVectorizationFactor return the success/failure directly.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1641-1642
+  /// such instructions are captured in \p Invalid.
+  bool hasInvalidCosts(ElementCount VF,
+                       SmallVectorImpl<Instruction *> *Invalid = nullptr);
+
----------------
sdesmalen wrote:
> dmgreen wrote:
> > There's only one use of hasInvalidCosts, and it doesn't use Invalid. Is it worth just using expectedCost directly?
> Could do, but that means making `expectedCost` public instead of private. I wasn't sure if there was a specific reason these cost-queries were private.
Oh I see. I hadn't noticed that expectedCost was private.

In that case can we make selectUserVectorizationFactor return a bool indicating whether it can select that user factor? So a true/false on success depending on the illegal cost. That should be able to use any private methods, and it makes sense to me for the method to return false if it cannot successfully pick that UserVF.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105473