Superseded by D156478.

Thank you Craig.
Can someone with commit access please land the patch for me? Thanks!

The glibc Bugzilla ticket probably makes the best reading so far: https://sourceware.org/bugzilla/show_bug.cgi?id=20024

Here is the discussion that lead up to the glibc / libmvec commit linked above: https://marc.info/?t=146472287500003

In D116879#3823875, @RKSimon wrote:

I'm struggling to find much documentation for libmvec other than the VectorABI.txt you mentioned - do you happen to know where we can review a reference header or source code please? I've tried searching by there's plenty of libs out there with similar names....

I'm still not sure the vector-of-pointers pattern what we want to support tbh

The failing LLVM.Transforms/LoopVectorize/AArch64::scalable-call.ll test makes me wonder what the exact semantics of the vector-function-abi-variant call attribute is in the presence of pointer arguments.

Reset parent to reflect reviewer comment there (removing duplicate call && test).

Re-uploading once again. arc diff --preview creates a preview that looks correct, but just arc diff is somehow mixing in the other patch I currently have open on top of this one.

Re-uploading patch after follow-on patch got mixed in

Add tests using #pragma clang loop vectorize(...).

• Remove duplicate checks

To finally get this out of the way without having the necessary analysis available, I am retreating a bit and just allow forced auto-vectorization via #pragma clang loop vectorize(assume_safety).

It's been a while since I last looked at this. However there don't seem to be many users of -fveclib=libmvec in general as demonstrated by the fact that it has been broken since before I've submitted this patch.
Before returning to allowing sincos() vectorization, I've thus submitted a separate patch to reenable vectorization of all other functions declared in VecFuncs.def already: D134732.

• at newlines at end of file where missing

• increase nest count
• add nounroll sincos() tests
• make sincos test fail reliably even in single precision
• protect against wrongly computed reference results by printing a few values

• compute reference result in long double

• run tests in both single and double precision

• run libmvec tests for each supported ISA
• don't depend on immintrin.h

• handle 32 bit pointer size

• check for AVX2, not SSE2

Add check that libmvec is available for linking and at runtime
and rename Libmvec/ directory to Veclib/ to indicate the test is
not specific to libmvec and might be enabled for other vector
libraries as well.

Are you talking about automatically choosing a vector library (-fveclib)?

Even if we run the test unconditionally, some logic will still be required to select the right flags to enable auto-vectorization.

I am not sure if it should be Libmvec or X86 specific though. There are other vector libraries on different platforms that provide vector versions of sincos. I don't think we should exclude those from this testing here.

my previous update seems to have eaten the original changes.
Let's see if I can get this right.

I have just submitted the test for the testsuite. Once that is merged, this change should also be fine to go in.

fix mixed up comments

This was supposed to update https://reviews.llvm.org/D120977 instead of creating a new submission.

And a variant of the code above shows that even the transformation to the vvv variant isn't safe in all cases.
I am looking into adding variations of the code above into the test-suite ahead of enabling the vectorizing transformation, to be sure the transformation is not applied when unsafe, and that the behavior of the underlying vector library matches my interpretation of the VectorAPI.
I don't see any existing tests around the transformations already performed.
Also, I see lots of regression tests to prevent potential performance regressions, ensuring the vectorizing transformation is not missed. But I don't see any tests currently to guard against correctness issues, ensuring the transformation is not applied in unsafe cases.

I have beefed up my testcase to demonstrate why I had to choose the _ZGVdN4vvv_sincos() variant for correctness, even though _ZGVdN4vl8l8_sincos() would be desirable from a performance perspective:
We have no control over what pointers the user is passing in in different loop iterations.

Thank you.
I do not have commit access anyway, so can someone please commit once review is sufficient.

A potential issue is that working sincos() support in libmvec is "only" a bit over 5 years old.
E.g. a user on Centos 7 compiling code with a vectorizable call to sincos() and compiling with -fveclib=libmvec and -O2 or higher will now run into an undefined reference to the vectorized sincos() function and will either have to deactivate auto-vectorization or update their libmvec.

I haven't figured out how to link to https://bugs.llvm.org/show_bug.cgi?id=51530 so that it would automatically get closed on merging. I suppose this isn't possible anymore with bugzilla being frozen?

In line with existing behavior, I have not added vector function definitions for AVX-512.

Of the two reproducers linked in https://bugs.llvm.org/show_bug.cgi?id=51530, the sincos_simd.cpp now auto-vectorizes when compiling with clang++ -fveclib=libmvec -O2 -march=core-avx2 sincos_simd.cpp, even when the #pragma omp simd annotation is left out in the source.

Adding reviewers who commented on D88154.

This is my first activity on reviews.llvm.org, so I might just be lacking suitable permissions - I still don't see any way to close.
Thanks!
Tim

I tried to mark this patch as a duplicate of the merged https://reviews.llvm.org/D32888 and ideally close it as such, but could not figure out how.