-
Notifications
You must be signed in to change notification settings - Fork 477
[Tile] Disable STD builtins in tile mode #11021
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
Changes from all commits
19f9d82
f3cfa44
c7cefe2
4acbd2e
6eee818
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,8 +10,7 @@ | |
| // REQUIRES: enable-tile || force-tile | ||
|
|
||
| // We cannot suppress execution checks in cuda::std::construct_at | ||
| // UNSUPPORTED: clang-14 | ||
| // UNSUPPORTED: enable-tile && !c++17 | ||
| // UNSUPPORTED: clang-14 && !c++17 | ||
| // UNSUPPORTED: force-tile && !c++17 | ||
|
|
||
| #include <cuda/std/cassert> | ||
|
|
@@ -21,8 +20,9 @@ | |
| #include "host_device_types.h" | ||
| #include "test_macros.h" | ||
|
|
||
| TEST_TILE_FUNC void test() | ||
| __tile__ void test() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: My understanding is this PR was meant to enable existing Thrust/CUB/libcu++ tests to build with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Those are the tests that verify that we can use the vocabulary types when tile mode is enabled. With us disabling tile support those tests do not work anymore and there is also not This slipped through because we only had the tile runs in nightly.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The alternative would have been to either delete the tests, which is bad for internal testing or completely ifdefing out everything which is also not nice and a lot of churn
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we just mark them as
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really want to avoid the chance of forgetting to reenable them |
||
| { | ||
| #if _CCCL_TILE_COMPILATION() | ||
| using expected = cuda::std::expected<void, tile_only_type>; | ||
| { // default construction | ||
| expected default_constructed{}; | ||
|
|
@@ -160,6 +160,7 @@ TEST_TILE_FUNC void test() | |
| assert(lhs.has_value()); | ||
| assert(rhs.error() == 1337); | ||
| } | ||
| #endif // _CCCL_TILE_COMPILATION() | ||
| } | ||
|
|
||
| __tile_global__ void test_kernel() | ||
|
|
||
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.
question: Why is the switch to
cuda::std::algorithms needed throughout this PR?