[PATCH] D67572: [VectorUtils] Introduce the Vector Function Database (VFDatabase).

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added a comment.

Hi @uabelho

In D67572#1779762 <https://reviews.llvm.org/D67572#1779762>, @uabelho wrote:

> Hi @fpetrogalli ,
>
> A question regarding this patch.
>  For my out-of-tree target vectorization of intrinsics added for my target seems to have stopped working with this patch.


Ops... sorry!

> Is there something/what do I have to do to make the vectorizer understand my intrinsics are vectorizable?

Yes, you need to add an attribute in the IR that maps the (scalar) attribute to its vector counterpart.

The attribute is called `vector-function-abi-variant`.

You can add it by using the following method in `llvm/include/llvm/Transforms/Utils/ModuleUtils.h`:

  namespace VFABI {
  /// Overwrite the Vector Function ABI variants attribute with the names provide
  /// in \p VariantMappings.
  void setVectorVariantNames(CallInst *CI,
                             const SmallVector<std::string, 8> &VariantMappings);
  } // End VFABI namespace

The VariantMappins are strings that need to be generated according to some Vector Function ABI (VFABI). If your target doesn't have such ABI, you can use the LLVM internal mangling.

For example, say that your attribute is `double @llvm.funky.intrinsic (double)`, and you need to map it to an unmasked vector function with a vectorization factor of two, say `custom_vector_function`, the string that you need to add in the attribute is `__ZGV_LLVM_N2v_llvm.funky.intrinsic(custom_vector_function)`.

The name mangling rules are admittedly not well documented for the internal mangling, but other than for the ISA token (which is `_LLVM_`), they correspond to the ones of x86 and AArch64, which are the same (you can browse the latter here: https://github.com/ARM-software/software-standards/blob/master/abi/vfabia64/vfabia64.rst#vector-function-name-mangling)

I will definitely add some docs to explain more in detail the mangling rules, but for the moment you can look at the tests in `llvm/unittests/Analysis/VectorFunctionABITest.cpp` to get a sense of the meaning of the different tokens in the mangled name, especially the use of the `<parameters>` in `_ZGV<isa><mask><vlen><parameters>_<scalarname>[(<redirection>)]`.

> Looking at this code in LoopVectorizationLegality.cpp:
> 
>   // We handle calls that:
>   //   * Are debug info intrinsics.
>   //   * Have a mapping to an IR intrinsic.
>   //   * Have a vector version available.
>   auto *CI = dyn_cast<CallInst>(&I);
>   if (CI && !getVectorIntrinsicIDForCall(CI, TLI) &&
>       !isa<DbgInfoIntrinsic>(CI) &&
>       !(CI->getCalledFunction() && TLI &&
>         !VFDatabase::getMappings(*CI).empty())) {
>    
> 
> VFDatabase::getMappings(*CI).empty() is indeed true for my intrisic, and if I dig further, I take this return in
> 
>   static void getVFABIMappings(const CallInst &CI,
>                                SmallVectorImpl<VFInfo> &Mappings) {
>     const StringRef ScalarName = CI.getCalledFunction()->getName();
>     const StringRef S =
>         CI.getAttribute(AttributeList::FunctionIndex, VFABI::MappingsAttrName)
>             .getValueAsString();
>     if (S.empty())
>       return;
>    
> 
> Is there some existing commit where in-tree targets have been modified already to work with the new VFDatabase?

Unless I have been missing something, all targets in the in-tree version are using VFDatabase now. The patch in this revision is what introduced the change.

> Thanks!

I hope you find this useful. Let me know if you need more help with this, I am generally available on IRC and discord too.

Francesco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67572