[PATCH] D77925: Revert "[TLI] Per-function fveclib for math library used for vectorization"

Thu Aug 27 23:41:56 PDT 2020

wenlei added a comment.

In D77925#2236519 <https://reviews.llvm.org/D77925#2236519>, @tejohnson wrote:

> In D77925#2229484 <https://reviews.llvm.org/D77925#2229484>, @mehdi_amini wrote:
>
>> Overall that would likely work for XLA. Something I'd like to mention though in response to:
>>
>>> The veclib type is also tied to the accepted values for -fveclib, which is a list of supported lib,
>>
>> `-fveclib` is a Clang thing, it shouldn't limit what LLVM does. Of course LLVM needs to support Clang, but does not have to inherit the limitation of map 1:1 to Clang UI.
>> In particular as a library, it isn't clear why we would make the choice to write LLVM VecLib support this way.

Fair point. I was trying to differentiate the accepted veclib from any other custom lib. I guess it's somewhat like namespace, even though built-in ones are different from user defined ones, the underlying support doesn't have to differentiate them.

> Is there any benefit to keeping a closed list like this in LLVM? If not (and presumably clang is checking for valid values of -fveclib), then I think I agree with @mehdi_amini. Unless there is an efficiency reason for doing it via an enum. It's been awhile since I looked through this code in detail...

I think performance should be fine, the attributes on functions are in string form already. TLI compatibility check will involve a string compare, but a short string compare shouldn't be disastrous. I was mainly trying to let LLVM match clang's supported list.

Will get back to this hopefully next week.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77925