[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

Sat Apr 11 12:15:26 PDT 2020

wenlei added a comment.

In D77632#1975619 <https://reviews.llvm.org/D77632#1975619>, @mehdi_amini wrote:

> The existing TLI provides a very convenient way to define a VecLib without LLVM knowing about it ahead of time. This feature is important for any embedded use of LLVM as a library out-of-tree (I'll add a unit-test in-tree).
>  I don't think it is a big change to this patch to preserve the current ability but I wanted to check first (and in the meantime I reverted in temporarily in https://reviews.llvm.org/D77925 to avoid the feature regression).
>
> At the moment the place where you seem to use this knowledge is with the `enum VectorLibrary`  in the `TargetLibraryInfoImpl` class, and the `VecLibDescs` array which statically contains the known VecLib.
>  It seems to me that if we replace this enum with a string instead to identify the VecLib everything should still hold together and this would fit with minor changes to this path. The `VecLibDescs` could just be a `StringMap<VectorLibraryDescriptors>` in this case.
>
> That was a third-party (in my case the XLA compiler) can still register its own "XLA" VecLib and add all the descriptors.
>
> How does it sound?

Thanks for the explanation about the revert. The proposal of using a StringMap to maintain the openness sounds good to me. And agree with @tejohnson, if the openness is a feature, it should be covered in tests, otherwise it can feel somewhat like a loophole and prone to breakage, though I can see how it can be useful.. Hope this patch can be restored with tweaks soon (we have workloads with very visible vectorization that relies on this).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77632