[PATCH] D143864: [VPlan] Replace AlsoPack field with shouldPack() method (NFC).
Ayal Zaks via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Feb 19 13:05:36 PST 2023
Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.
Indeed better compute such information on demand than cache it! Looks good to me, adding a couple of nits.
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9504
// Insert scalar instance packing it into a vector.
- if (AlsoPack && State.VF.isVector()) {
+ if (shouldPack() && State.VF.isVector()) {
// If we're constructing lane 0, initialize to start from poison.
----------------
nit: may be slightly faster if swapped to check State.VF.isVector() first.
================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1540
+
+ /// Returns true if the recipe is used by a widened instruction. In this case,
+ /// the scalar values are also packed in a vector.
----------------
nit: used by a widened instruction indirectly - via an intervening PredInstPhi? Until the insert-element's are represented explicitly.
================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:930
+ return any_of(PredR->users(),
+ [](const VPUser *U) { return !isa<VPReplicateRecipe>(U); });
+ return false;
----------------
This is indeed the current behavior, but better have it check if !U.usesScalars() instead, possibly as a follow-up?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143864/new/
https://reviews.llvm.org/D143864
More information about the llvm-commits
mailing list