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

Mon Apr 13 09:07:06 PDT 2020

tejohnson added a comment.

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

> In D77632#1976231 <https://reviews.llvm.org/D77632#1976231>, @wenlei wrote:
>
> > 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
>
>
> The thing is that LLVM does not have much C++ unittests in general, so most of the "features" like this one that LLVM offers as a library are just an artifact of being "designed as a library" and being mindful about the layering.
>  From this point of view this patch is changing the design of a component that was modular/pluggable into a closed system.

The interfaces being relied on were in the underlying Impl class, I think if that is expected to be pluggable and stable it really needs unit testing to reflect that usage.

> I'm perfectly fine with trying to add a unit-test, I just don't know yet where it would fit in the LLVM testing though.

Presumably the testing should be in llvm/unittests/Analysis/TargetLibraryInfoTest.cpp, which already exists but only tests the LibFuncs (builtins) interfaces.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77632