-
Notifications
You must be signed in to change notification settings - Fork 281
c.parallel: enable dynamic policies in unique_by_key. #6087
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
base: main
Are you sure you want to change the base?
Conversation
😬 CI Workflow Results🟥 Finished in 6h 28m: Pass: 98%/185 | Total: 3d 12h | Max: 4h 09m | Hits: 94%/190718See results here. |
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, we should keep that test instead of deleting it and just increase the size for now.
|
||
operation_t op = make_operation( | ||
"op", | ||
"struct large_key_pair { int a; char c[100]; };\n" |
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.
Important: instead of deleting this, let us keep it but increase the size of the dtype. Use char c[500]
here instead. The reason the build succeeds after your changes is that the selected policy uses a smaller block size with less items per thread, which allows us to use a type of this size without running out of shared memory.
VSmem is still not supported, if you search for VSMemPerBlock
in unique_by_key.cu
you can see we return 0.
This issue is tracked here #3790
struct large_key_pair | ||
{ | ||
int a; | ||
char c[100]; |
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 will also want to update the size here
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.
CUB part loooks good
Description
Note: the "big keys shouldn't compile" case started compiling because we now actually do the proper math, presumably that's fine? I'd like some confirmation on that one.
Resolves #4362
Checklist