-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix clang support #21
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some few comments. Maybe they're helpful.
ChangeLog: * README.md: Add a section about constexpr_wrapper.
Clang warns about and doesn't use the always_inline attribute on lambdas. Therefore only define the macro if __clang__ is not defined. ChangeLog: * vir/detail.h: Guard VIR_LAMBDA_ALWAYS_INLINE with not __clang__.
Clang and GCC disagree about whether the decltype of an NTTP is const or not. This made it work on GCC but not on Clang. Avoid the issue by using types as simd_policy options instead of NTTPs. ChangeLog: * vir/simd_execution.h (simd_policy): Refactor to type-based options instead of NTTPs.
ChangeLog: * vir/simd_execution.h: Replace a + b <= c with a + b - 1 < c.
ChangeLog: * testsuite/tests/transform.cc: #ifdef zip_view test for __clang_major__ <= 17. Remove unused lambda captures.
* We don't need warnings about self-assignment when testing that self-assignment works. * The GCC-compatible method of PCH inclusion triggers a -Wdeprecated warning, which isn't helpful. Turning it off.
Clang doesn't defined __GCC_IEC_559, so we need to look at __STDC_IEC_559__. This is already implemented as COMPLETE_IEC559_SUPPORT in test_values.h, which we can reuse in the tests. For completeness, don't define named constants when their use is #ifdef'ed out. ChangeLog: * testsuite/tests/fpclassify.cc: Guard IEC559 special values with COMPLETE_IEC559_SUPPORT. * testsuite/tests/frexp.cc: Likewise.
Clang < 17 cannot include <ranges> from GCC 13 in C++23 mode without breaking. ChangeLog: * vir/simd_execution.h: Guard against incompatible Clang and <ranges> header.
The problem didn't trigger with the libstdc++ stdx::simd because of the padding induced by alignment requirements, which then hit the sizeof condition. This is different for the array-based vir::stdx::simd implementation. ChangeLog: * vir/simdize.h: Move static_assert for power-of-2 number of elements into constexpr-if condition.
On test failure, the COMPARE and ULP_COMPARE functions use __PRETTY_FUNCTION__ to print the function where the failure happened. As an implementation detail the macros internally use a directly invoked lambda. If __PRETTY_FUNCTION__ is inside that lambda then the compiler may emit the name of the lambda instead of the calling function. To get to see the latter, pass __PRETTY_FUNCTION__ as function argument to the lambda. ChangeLog: * testsuite/tests/bits/verify.h (COMPARE, ULP_COMPARE): Move __PRETTY_FUNCTION__ from inside the lambda to the lambda arguments.
This way CI also tests these without depending on custom TESTFLAGS (or similar). ChangeLog: * Makefile: Add -Wall -Wextra -Wpedantic to the beginning of CXXFLAGS * vir/detail.h: Ignore -Wpedantic when uttering __int128. * vir/simd_execution.h: Remove stray semicolon.
Clang 17 miscompiles a vinsertps call in some tests. It's fairly impossible to debug where this comes from. Any help from the compiler via e.g. -Og or -fsanitize makes the bug go away. ChangeLog: * vir/detail.h (VIR_HAVE_WORKING_SHUFFLEVECTOR): Renamed from VIR_HAVE_BUILTIN_SHUFFLEVECTOR. False for Clang 17. * vir/simd_permute.h: Rename to VIR_HAVE_BUILTIN_SHUFFLEVECTOR. * vir/simdize.h: Likewise.
ChangeLog: * vir/constexpr_wrapper.h: Comparing arrays of different size is ill-formed; check for size and subscript value instead. * vir/test_constexpr_wrapper.cpp: Test all kinds of literals.
ChristianTackeGSI
approved these changes
Nov 2, 2023
Thank you Christian! Much appreciated! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.