[PATCH] D136251: [LoopVectorize] Use available masked vector functions when required

Tue Feb 28 07:30:23 PST 2023

huntergr marked an inline comment as done.
huntergr added inline comments.

================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:285
+    // otherwise we look for one that matches the supplied VF.
+    auto Mappings = VFDatabase::getMappings(CI);
+    for (VFInfo Info : Mappings)
----------------
david-arm wrote:
> I wonder if it's useful to have a `VFDatabase::getMaskedMappings` variant or something like that? That way we can avoid walking through all the mappings if there aren't going to be any masked variants anyway. I was thinking about something like this:
> 
>   auto Mappings = VFDatabase::getMaskedMappings(CI);
>   if (Mappings.emtpy())
>     return false;
>   for (VFInfo Info : Mappings)
>     if (!VF || Info.Shape.VF == *VF)
>       return true;
>   return false;
> 
> Do you think it's worth it?
getMaskedMappings would just end up iterating over all the variants anyway -- this code doesn't really do any caching, since it's a static method rather than an instance method. So every time we ask for variants we re-decode the mangled names to figure out which functions we should be looking at.

I think LoopVectorize and VectorUtils could be much smarter about this, but that's something for another patch.

================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8341
                                                    VFRange &Range,
-                                                   VPlanPtr &Plan) const {
+                                                   VPlanPtr &Plan) {
   bool IsPredicated = LoopVectorizationPlanner::getDecisionAndClampRange(
----------------
david-arm wrote:
> Just out of interest why do we need to remove `const`?
'createBlockInMask' is not a const function and takes a VPlan as an argument, so this can't be const either.

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

https://reviews.llvm.org/D136251