Compatibility: Only use sycl::half if SYCL_CTS_ENABLE_HALF_TESTS is set
#872
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
bader
reviewed
Feb 17, 2024
| } else if (std::is_same_v<std::remove_cv_t<CTS_TYPE>, double>) { | ||
| } | ||
| #endif | ||
| if (std::is_same_v<std::remove_cv_t<CTS_TYPE>, double>) { |
There was a problem hiding this comment.
Suggested change
| if (std::is_same_v<std::remove_cv_t<CTS_TYPE>, double>) { | |
| if constexpr (std::is_same_v<std::remove_cv_t<CTS_TYPE>, double>) { |
bader
reviewed
Feb 17, 2024
| sycl::half bitselect(sycl::half a, sycl::half b, sycl::half c); | ||
| #endif |
There was a problem hiding this comment.
Probably we should re-group these declarations to
// float type overloads for all math functions.
...
#if SYCL_CTS_ENABLE_DOUBLE_TESTS
// double type overloads for all math functions.
...
#endif
#if SYCL_CTS_ENABLE_HALF_TESTS
// half type overloads for all math functions.
...
#endif
bader
reviewed
Feb 17, 2024
| @@ -51,8 +51,12 @@ using has_atomic_support = contains<T, int, unsigned int, long, unsigned long, | |||
| */ | |||
| template <typename T> | |||
| using is_sycl_floating_point = | |||
There was a problem hiding this comment.
Suggested change
| using is_sycl_floating_point = | |
| using is_sycl_scalar_floating_point = |
From what I see, they are filtered out automatically: Lines 162 to 164 in 42e9081 |
|
g++-13 implements already https://en.cppreference.com/w/cpp/types/floating-point from C++23. @fknorr Go for it! :-) |
|
@fknorr Some merge from upstream? |
|
I was not aware of the _Float16 support in newer GCC - this PR might not be required anymore for SimSYCL, I can check after merging #870 . |
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.
This PR improves compatibility with implementations that do not expose the
sycl::halftype (notably, SimSYCL built with GCC).There might be additional usages I missed, in tests that SimSYCL is currently unable to compile for other reasons.
Question to the reviewers: Is it necessary to exclude some translation units in CMake when the file names are
*fp16.cppor are these handled automatically?