• 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.