Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions

[steleman]

Summary

This changeset adds the required Builtins for enabling AArch64 vectorization of libm trigonometry functions via SLEEF: http://sleef.org/.

• A new argument is added to -fveclib=<X>: SLEEF.
• A number of Builtins that were previously unimplemented are now enabled.

This changeset depends on https://reviews.llvm.org/D53927.

List of SLEEF vectorized trigonometry functions:

acos
asin
atan
atanh
cos
cosh
exp
exp2
exp10
lgamma
log10
log2
log
sin
sinh
sqrt
tan
tanh
tgamma

[steleman]

• changed the -fveclib=<X> argument value to 'sleefgnuabi'.
• added atan2 and pow.
• spreadsheet with comparison between libm and sleef is here:

https://docs.google.com/spreadsheets/d/1lcpESCnuzEoTl_XHBqE9FLL0tXJB_tZGR8yciCx1yjg/edit?usp=sharing

• comprehensive test case in C with timings is here:

https://drive.google.com/open?id=1PGKRUdL29_ANoYebOo3Q59syhKp_mNSj

[steleman]

Updated version of this changeset/patch, as per Renato's latest comments from D53927 (https://reviews.llvm.org/D53927).

New benchmark comparing SLEEF performance when built with GCC 8.2 vs. clang ToT from 12/15/2018:

https://docs.google.com/spreadsheets/d/1QgdJjgCTFA9I2_5MvJU0uR4JT4knoW5xu-ujxszRbd0/edit?usp=sharing

[rengolin]

Adding clang/omp developers for proper review. Please feel free to add more.

FYI, there are four main ongoing discussions on the LLVM thread (D53927):

1. There is collusion between clang pragmas (clang vectorise), OpenMP pragmas (omp simd) and newly proposed veclib pragmas, which IMHO is too much. We need more OpenMP people looking at them. My proposal is to not add new ones and use OpenMP SIMD without creating library dependencies with OMP, but the interpretation of other pragmas in the same source needs sorting.

2. How we're going to handle the naming scheme, currently proposed to have a local math.h and mask the SLEEF includes with macros. My proposal is to have a special header that the compiler only includes upon -mveclibabi instead. Library includes end up as -lsleef in the linker command line.

3. Who controls the header file. The proposal is Clang control the headers for all vector libraries (all in math.h with guards), which means the evolution of Clang and each library is now tied. My proposal is for the library to have their own headers in a standard place (/usr/include/arch/something') and get Clang to agree on the right place, so that it can include the header from $default_path/$library_name.h`.

4. The option was called fveclib but GCC calls it mveclibabi. There doesn't seem to be any behavioural differences, so I think we should follow the same name. We may be forced to create an alias, because this is not new in LLVM (already existed for SVML). This can go on a separate patch. But it's good to be aware of that. Making it an alias, we are fixing the implementation to be at least compatible with GCC, which is a good thing.

cheers,
--renato

lib/Index/CommentToXML.cpp

230 ↗

(On Diff #178477)

Spurious change and unrelated to the patch. Please revert this part.

[steleman]

Removed spurious patch for an unrelated change.

[steleman]

Ping!

Yes, I know, everyone was away for the holidays. :-)

Could someone please take a look at this. We'd like to make sure everything is OK on this side before D53927 can proceed.

Yes, it's a bit of a circular dependency in that sense.

This changeset doesn't cover any of the problems with OpenMP and conflicting pragmas and semantics described above by Renato. This is just the clang side of straightforward mapping of SLEEF vectorized function calls.

[hfinkel]

LGTM

Please, in the future, make sure you post full-context patches.

[steleman]

This is a small - but signifcant - update to the original changeset.

What's new in this update:

${top_srcdir}/lib/Frontend/CompilerInvocation.cpp:

When -fveclib=SLEEF was passed on compile-line, neither clang nor LLVM emit any warnings if SLEEF is not supported on the Target architecture. SLEEF is currently supported only on AArch64. So, for other ISA's, -fveclib=SLEEF would be quietly ignored, without any hints.

This new version of the changeset adds a diagnostic if -fveclib=SLEEF is called on an ISA that doesn't (yet) support SLEEF.

[rengolin]

Since the previous version was approved already, I'm "requesting changes" so that we can look at it again, together with D53927 to make sure it's still the right way to go.

[rengolin]

@hfinkel There are changes in this patch, as well as its LLVM counterpart D53927. Please, re-review.

[rengolin]

Funny thing is, SVML is also only supported, AFAIK, for Intel. I agree that we should emit errors, but we should also emit a similar error on SVML.

I know it's not entirely relevant to this patch, but we should keep the behaviour consistent on all veclibs.

Also, can you add a test for the new error messages, please?

The new changes look good to me and don't substantially deviate from the previous, approved, patch, so LGTM after addressing the comments.

Thanks!

lib/Frontend/CompilerInvocation.cpp

678

This is not really invalid value for the flag, it's invalid architecture for the value.

I think there's already a string for that somewhere in Clang.

[steleman]

Hi Renato,

Thank you very much for the comments.

I will create a new changeset incorporating your comments, and then re-submit.

I will also add a Diagnostic for SVML and X86/X86_64.

[steleman]

Addressed comments from Renato:

• Diagnostic for unsupported argument to -fveclib=<N> is now:

error: unsupported option '<X>' for target '<Y>'

• Added check and diagnostic for SVML on non-Intel ISA's.

[rengolin]

Thanks, LGTM!