-
Notifications
You must be signed in to change notification settings - Fork 407
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
Enable SYCL atomic tests #3742
Enable SYCL atomic tests #3742
Conversation
425cf80
to
754ab97
Compare
@@ -45,6 +45,8 @@ | |||
#include <TestAtomicOperations.hpp> | |||
|
|||
namespace Test { | |||
// FIXME_SYCL atomic_fetch_oper for large types to be implemented | |||
#ifndef KOKKOS_ENABLE_SYCL |
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.
Why do you remove it from the list if everything is being commented out?
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.
So that I have a better description for the FIXME.
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.
If you feel really strongly about it I can remove it but I prefer to have tests in core/unit/CMakeLists.txt
that I have not looked at and found a better description.
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.
I prefer tests which completely not work to be removed in cmake
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.
You can have the comment in the CMakeLists.txt right next to the unit test you remove from the list
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.
OK. Changed.
Retest this please. |
Depends on #3741.