[PATCH] D53927: [AArch64] Enable trigonometry libm vectorized functions via SLEEF

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added a comment.

Hi @steleman , thank you for working on this!

Before doing a detailed review, I have a couple of comments and requests.

1. Please add full context to the patch (`git show -U999999`).
2. You are using Vector Function ABI mangled names, which means you ware assuming to link agains `libsleefgnuabi.so` instead of `libsleef.so`. For this reason, I think it is worth to replace the `-fveclib=SLEEF` option with `-fveclib=SLEEFGNUABI`. This request applies to all places where you  have used `SLEEF`.
3. Given that SLEEF has some degree of target independence, I would recommend using the TargetTriple `if` that selects the list of functions into a switch statement that sets the architectural extension token in the mangled names. On the long term, the list of available vector functions should be generated by setting such token instead of hard coding all names for all targets. Notice that I am not saying that you should add all targets, I am just saying that you should prepare the ground for extending this, as I think that `-fveclib=SLEEFGNUABI` should work the same for all the extensions supported by SLEEF.
4. On the longer term, I want to make you aware of the fact that there is plan to replace the TLI implementation of -fveclib with an OpenMP based one (to decouple backend and frontend). We discussed this at the last LLVM dev meeting, I will circulate an RFC soon. See https://llvm.org/devmtg/2018-10/talk-abstracts.html#bof7.
5. Please change your commit messages header, the function you are adding are not just trigonometric functions. May I suggest "[AArch64] Enable libm vectorized functions via SLEEFGNUABI"?

Kind regards,

Francesco


Repository:
  rL LLVM

https://reviews.llvm.org/D53927