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

[Stefan Teleman via Phabricator via llvm-commits]

steleman added a comment.

In D53927#1307070 <https://reviews.llvm.org/D53927#1307070>, @chandlerc wrote:

>




> FWIW, I'm sorry to push on this, but I think it would be better to have review from folks more familiar w/ generic LLVM optimizations and with SLEEF to review this. The obvious candidate, IMO, would be Hal. If Hal is unavailable, we could have a number of other people that would be very effective at reviewing (Tim, others...) with deep and londstanding experience in this place.
> 
> Given that this is a pretty significant change for AArch64 at least, and potentially for LLVM as a whole, so I think it is reasonable to expect a bit more wide review before it gets landed.

To make things perfectly clear:

- This changeset doesn't do anything that wasn't already in LLVM. -fveclib=SVML support was added by Intel prior to this changeset.
- There is nothing here that qualifies as //this is a pretty significant change for AArch64 at least, and potentially for LLVM as a whole//, as -fveclib=<X> is not the default in clang.
- The only difference is in the veclib library: SLEEF instead of SVML. And in the vector function names, obviously.
- @shibatch - a.k.a. the author of SLEEF - is subscribed to this changeset, and has commented on it already.

> Essentially, I'd suggest being a bit more reluctant to approve this kind of patch.

I'd like a better understanding of what this really means.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53927