[Mirror] [5.1.1] 0 SYCL: Fix Windows build issues - #11
Open
csiefer2 wants to merge 11 commits into
Open
Conversation
Signed-off-by: Damien L-G <dalg24@gmail.com>
Signed-off-by: Daniel Arndt <arndtd@ornl.gov>
```
/path/to/kokkos/core/src/Kokkos_Macros.hpp:36:6: warning: 'KOKKOS_VERSION' is not defined, evaluates to 0 [-Wundef]
36 | #if !KOKKOS_VERSION_EQUAL(KOKKOS_VERSION_MAJOR, KOKKOS_VERSION_MINOR, \
| ^
/path/to/kokkos/core/src/Kokkos_Macros.hpp:34:4: note: expanded from macro 'KOKKOS_VERSION_EQUAL'
34 | (KOKKOS_VERSION == ((MAJOR)*10000 + (MINOR)*100 + (PATCH)))
| ^
/path/to/kokkos/core/src/Kokkos_Macros.hpp:36:27: warning: 'KOKKOS_VERSION_MAJOR' is not defined, evaluates to 0 [-Wundef]
36 | #if !KOKKOS_VERSION_EQUAL(KOKKOS_VERSION_MAJOR, KOKKOS_VERSION_MINOR, \
| ^
/path/to/kokkos/core/src/Kokkos_Macros.hpp:36:49: warning: 'KOKKOS_VERSION_MINOR' is not defined, evaluates to 0 [-Wundef]
36 | #if !KOKKOS_VERSION_EQUAL(KOKKOS_VERSION_MAJOR, KOKKOS_VERSION_MINOR, \
| ^
/path/to/kokkos/core/src/Kokkos_Macros.hpp:37:27: warning: 'KOKKOS_VERSION_PATCH' is not defined, evaluates to 0 [-Wundef]
37 | KOKKOS_VERSION_PATCH)
| ^
```
Co-Authored-By: Daniel Arndt <arndtd@ornl.gov>
Signed-off-by: Damien L-G <dalg24@gmail.com>
[5.1.0] Fix -Wundef warnings about KOKKOS_VERSION* not being defined
[5.1.0] Identify homebrew llvm as KOKKOS_COMPILER_CLANG
Signed-off-by: Seyong Lee <lees2@ornl.gov> Signed-off-by: Christian Trott <crtrott@sandia.gov> Co-authored-by: Daniel Arndt <arndtd@ornl.gov> Co-authored-by: Jakob Bludau <104908666+JBludau@users.noreply.github.com> Co-authored-by: Damien L-G <dalg24+github@gmail.com> Co-authored-by: Nathan Ellingwood <ndellin@sandia.gov> Co-authored-by: Trévis Morvany <63788850+tretre91@users.noreply.github.com> Co-authored-by: Christian Trott <crtrott@sandia.gov>
…8967) * Fix subview construction between managed and unmanaged Views * Add test Signed-off-by: Daniel Arndt <arndtd@ornl.gov>
Signed-off-by: Damien L-G <dalg24@gmail.com>
…ubview_constructor [5.1.0] Fix subview construction between managed and unmanaged Views
Signed-off-by: Damien L-G <dalg24@gmail.com>
* SYCL: Fix Windows build issues --------- Signed-off-by: Daniel Arndt <arndtd@ornl.gov> Signed-off-by: Damien L-G <dalg24+github@gmail.com> Co-authored-by: Damien L-G <dalg24+github@gmail.com> (cherry picked from commit 59c0fcc)
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates Kokkos to version 5.1.1 and includes a comprehensive changelog for the 5.1.0 release. Key code changes involve refined compiler detection for Apple Clang, explicit type casting for SYCL mathematical functions, and the addition of unit tests for unmanaged subview construction. Review feedback identified several typos and PR link mismatches within the changelog. Additionally, there is a concern regarding the SYCL abort implementation on Windows, which currently uses a printf instead of a terminating call, potentially allowing execution to continue after a failure.
| * Enable MPI detection with PALS [\#8895](https://github.com/kokkos/kokkos/pull/8895) | ||
| * Add simd memory permute functions [\#8775](https://github.com/kokkos/kokkos/pull/8775) | ||
| * Performance improvements using `MDRangePolicy` with `CUDA`, `HIP` and `SYCL` [\#8638](https://github.com/kokkos/kokkos/pull/8638), [\#8731](https://github.com/kokkos/kokkos/pull/8731) | ||
| * Add `Kokkos::norm`for `Kokkos::complex`- similar to `std::norm` [\#8627](https://github.com/kokkos/kokkos/pull/8927) |
There was a problem hiding this comment.
There are several typos and a link mismatch in this line:
normforshould benorm for.complex-should becomplex -.- The link text
[\#8627]does not match the PR number in the URLpull/8927. Based on the feature, it should likely point to PR Jenkins: Print commands being executed kokkos/kokkos#8627.
Suggested change
| * Add `Kokkos::norm`for `Kokkos::complex`- similar to `std::norm` [\#8627](https://github.com/kokkos/kokkos/pull/8927) | |
| * Add Kokkos::norm for Kokkos::complex - similar to std::norm [\#8627](https://github.com/kokkos/kokkos/pull/8627) |
| * Performance improvements using `MDRangePolicy` with `CUDA`, `HIP` and `SYCL` [\#8638](https://github.com/kokkos/kokkos/pull/8638), [\#8731](https://github.com/kokkos/kokkos/pull/8731) | ||
| * Add `Kokkos::norm`for `Kokkos::complex`- similar to `std::norm` [\#8627](https://github.com/kokkos/kokkos/pull/8927) | ||
| * Use neon and sve SIMD instructions if `nvcc` supports them [\#8667](https://github.com/kokkos/kokkos/pull/8667) | ||
| * Expand math support: complete the implementation of all remaining math functions and increase half-type support [\#8595](https://github.com/kokkos/kokkos/pull/8789) [\#8858](https://github.com/kokkos/kokkos/pull/8858) [\#8873](https://github.com/kokkos/kokkos/pull/8873) [\#8712](https://github.com/kokkos/kokkos/pull/8712) [\#8827](https://github.com/kokkos/kokkos/pull/8827) [\#8819](https://github.com/kokkos/kokkos/pull/8819) [\#8719](https://github.com/kokkos/kokkos/pull/8719) [\#8863](https://github.com/kokkos/kokkos/pull/8863) [\#8862](https://github.com/kokkos/kokkos/pull/8862) [\#8778](https://github.com/kokkos/kokkos/pull/8778) [\#8891](https://github.com/kokkos/kokkos/pull/8891) |
There was a problem hiding this comment.
The link text [\#8595] does not match the PR number in the URL pull/8789. Both PRs are related to math support, but the mismatch is confusing.
| * Use intrinsics when calling min and max on simd vectors of integral types [\#8899](https://github.com/kokkos/kokkos/pull/8899) | ||
| * Adds missing `constexpr` specifiers on `conj()`, and for the `real()` and `imag()` non-member functions taking complex numbers [\#8928](https://github.com/kokkos/kokkos/pull/8928) | ||
| * Ensure that execution space instances fence on finalize [\#8626](https://github.com/kokkos/kokkos/pull/8626) | ||
| * Update `team_fan_{in|out}` member functions of `ThreadsExecTeamMember` not to call host-only fuctions on the device [\#8730](https://github.com/kokkos/kokkos/pull/8730) |
There was a problem hiding this comment.
Typo: fuctions should be functions.
Suggested change
| * Update `team_fan_{in|out}` member functions of `ThreadsExecTeamMember` not to call host-only fuctions on the device [\#8730](https://github.com/kokkos/kokkos/pull/8730) | |
| * Update team_fan_{in|out} member functions of ThreadsExecTeamMember not to call host-only functions on the device [\#8730](https://github.com/kokkos/kokkos/pull/8730) |
| inline void sycl_abort(char const* msg) { | ||
| #ifdef NDEBUG | ||
| // FIXME_WINDOWS __assert_fail isn't defined | ||
| #if defined(NDEBUG) || defined(_MSC_VER) |
There was a problem hiding this comment.
Adding _MSC_VER to this check disables the abort behavior on Windows even in Debug builds, as it will only execute the printf and then return. If __assert_fail is not available on Windows, consider using an alternative that actually stops execution, such as std::abort() for host code.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Automated mirror of upstream PR kokkos#9109 Cherry-pick of kokkos#9043 onto
release-candidate-5.1.1.Original PR: kokkos#9043