-
Notifications
You must be signed in to change notification settings - Fork 99
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
Remove volatile qualifiers in reducer join(), init(), and operator+= methods #1382
Conversation
7788416
to
a8fa21b
Compare
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
It probably makes sense to wait on running the PR tester on this until after the Kokkos fixes are on |
With revisions to the Kokkos PR, this shouldn't be strictly necessary, but it will still be desirable cleanup |
For now, Kokkos Kernels remains backwards compatible with the release branch for Kokkos 3.6. This PR would drop that compatibility. The code here is fine, so feel free to move this forward whenever KK is ready to drop compatibility or breaks it elsewhere. |
a8fa21b
to
d8b6b2d
Compare
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
The |
5ad0e64
to
94d8575
Compare
It would be nice to get this auto-tested and integrated before the 3.7 release branch |
@PhilMiller yeah, we need to fix some other issues first related to the new guards to prevent impl files from being included. |
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
bdcd6a8
to
b813659
Compare
kokkos/kokkos#5215 will generate a bunch of deprecation warnings in KK until this gets merged. |
@PhilMiller did you merge a change in Kokkos develop that makes it generate the warning? |
Also please apply clang-format to you PR so that it passes our check, we are using clang-format 8.0.0 |
That PR isn't merged yet, but it's expected to be before 3.7 is released. Applying clang-format will result in a very noisy PR, because I have made some targeted changes to files that aren't already formatted. I think it's preferable to review this as is, and then make a separate PR to reformat those files (and potentially others) in full. |
b813659
to
2f60e26
Compare
Based on #1464 to treat the reformatting separately. |
Reformat example/fenl files changed in #1382
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.
Looks good, thanks for the clean-up
Status Flag 'Pre-Test Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED by label AT: PRE-TEST INSPECTED! Autotester is Removing Label; This inspection will remain valid until a new commit to source branch is performed. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9_GCC720_Light_Tpls_GCC720_GCC740
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10_Tpls_CUDA10_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720_Light_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_INTEL18
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_CLANG1001
Jenkins Parameters
Using Repos:
Pull Request Author: PhilMiller |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9_GCC720_Light_Tpls_GCC720_GCC740
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10_Tpls_CUDA10_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720_Light_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_INTEL18
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_CLANG1001
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ lucbv ]! |
Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge |
3 similar comments
Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge |
Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge |
Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge |
Following kokkos/kokkos#4077 et seq, none of these methods need to take
volatile
-qualified parameters any longerThis should all be good on Kokkos
develop
and releases from 3.7/4.0 onwards, but this will break compatibility with Kokkos 3.6, in case that matters.