Skip to content
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

Make "cooperativity" be part of the launch configuration #258

Closed
eyalroz opened this issue Aug 20, 2021 · 7 comments
Closed

Make "cooperativity" be part of the launch configuration #258

eyalroz opened this issue Aug 20, 2021 · 7 comments

Comments

@eyalroz
Copy link
Owner

eyalroz commented Aug 20, 2021

At the moment, the user must specify, when launching a kernel, whether thread blocks should be allowed to cooperate or not. This is a bit cumbersome, and also increases the number of kernel launch functions and methods, which is already not so small.

On the other hand, our wrapped kernel class does indicate whether thread blocks should be allowed to cooperate, and putting that flag into a launch config may remove the ability to hard-code that requirement.

Yet again on the first hand - why should we not have the ability to hard-code requirements for other aspects of the launch, into a kernel_t? Dynamic shared memory size requirements, and even block and grid size requirements? A kernel can certainly have those.

I would lean in favor of having less functionality in kernel_t and making the move, since this library is just for wrapping, not for beefier abstractions.

Why do you think @codecircuit , @raplonu, @quxflux et al. ?

@codecircuit
Copy link
Contributor

As far as I understand your point you want to discuss how much 'kernel launch configuration' should be saved in class kernel_t. Is that correct?

I agree with your point of having less functionality in class kernel_t and e.g. move the thread block cooperation flag into the kernel launch config, instead of having it as a member variable in kernel_t. Due to issue #236, I think it is not completely safe to use the kernel_t class when launching a CUDA kernel and I personally avoid that completely. Nevertheless, I think the kernel_t class is handy, to e.g. check kernel attributes and GPU occupancy. As far as I see it, the member variable thread_block_cooperation of class kernel_t is only used in the ctor and getter function. Thus, the member functions of class kernel_t should not be affected if this member variable is removed.

@eyalroz
Copy link
Owner Author

eyalroz commented Aug 23, 2021

As far as I understand your point you want to discuss how much 'kernel launch configuration' should be saved in class kernel_t. Is that correct?

No... I'm wondering whether to add bool block_cooperation to launch_configuration_t, and removing it from kernel_t...

@codecircuit
Copy link
Contributor

To me this sounds reasonable.

@eyalroz
Copy link
Owner Author

eyalroz commented Aug 27, 2021

@codecircuit : Reasonable, and better than what we have currently?

@eyalroz eyalroz changed the title Should "cooperativity" be made part of the launch configuration? Make "cooperativity" be part of the launch configuration Aug 28, 2021
eyalroz added a commit that referenced this issue Aug 28, 2021
…on. Several `launch()` functions and methods removed.
@eyalroz
Copy link
Owner Author

eyalroz commented Aug 28, 2021

@codecircuit : Done.

@codecircuit
Copy link
Contributor

@codecircuit : Reasonable, and better than what we have currently?

Yes, the commit looks good 👍.

@eyalroz
Copy link
Owner Author

eyalroz commented Aug 30, 2021

@codecircuit : I've deleted 63 more lines than I've added, so it has to be good :-P

Reminding you to possibly have a look at the driver-wrappers branch.

eyalroz added a commit that referenced this issue Nov 7, 2021
…on. Several `launch()` functions and methods removed.
eyalroz added a commit that referenced this issue Nov 13, 2021
…on. Several `launch()` functions and methods removed.
eyalroz added a commit that referenced this issue Dec 4, 2021
…on. Several `launch()` functions and methods removed.
eyalroz added a commit that referenced this issue Dec 10, 2021
…on. Several `launch()` functions and methods removed.
eyalroz added a commit that referenced this issue Jan 14, 2022
…on. Several `launch()` functions and methods removed.
@eyalroz eyalroz closed this as completed Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants