[PATCH] D136251: [LoopVectorize] Use available masked vector functions when required
David Sherwood via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 24 07:01:36 PDT 2023
david-arm 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)
----------------
huntergr wrote:
> 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.
Sure, I was just thinking we could avoid walking over the mappings twice like we're doing at the moment. With your patch as it stands we call `VFDatabase::getMappings`, which walks over all the mappings and creates a set of them, which we then iterate over yet again here. However, if you had a function such as `VFDatabase::getMaskedMappings` you can at least optimise for the (common) case where there are no masked variants, i.e.
```auto Mappings = VFDatabase::getMaskedMappings(CI);
if (Mappings.emtpy())
return false;```
That only requires walking over the mappings once, and if there are masked mappings it also makes the second list walk shorter because you've already filtered out the non-masked variants.
Anyway, I wouldn't hold the patch up for this - I just thought it seemed like it might reduce computational effort.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136251/new/
https://reviews.llvm.org/D136251
More information about the llvm-commits
mailing list