-
Notifications
You must be signed in to change notification settings - Fork 979
[QDP] Fix invalid CUDA kernel launch when num_samples exceeds grid dimension limit #968
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
[QDP] Fix invalid CUDA kernel launch when num_samples exceeds grid dimension limit #968
Conversation
|
cc @rich7420, @ryankert01 |
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.
using 65,535, you might be using a limit inherited from the Fermi architecture (pre-2012).
On any modern GPU (Compute Capability 3.0 or higher, which is virtually everything in use today), the 1D grid limit for the X-dimension is significantly higher. (at 2^31-1)
Which may or may not be touch before out-of-memory. (but it's good to add a check tho)
|
@ryankert01 thx for suggestion! What if we use |
76021d0 to
1f60d55
Compare
ryankert01
left a comment
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.
Thanks for the update! Some comments
rich7420
left a comment
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.
@viiccwen thanks for the patch!
I think we could add tests for num_samples==0/sample_len==0.
…d sample length for float32 and float64
rich7420
left a comment
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.
LGTM
|
@ryankert01 do you want to take another look? |
|
@viiccwen lg, ty for the contribution |
Purpose of PR
Fixes a bug in
launch_l2_norm_batch(f64) where attempting to process more than limited samples would result in an invalid CUDA kernel launch. The fix adds early validation to return an error whennum_samplesexceeds the CUDA 1D grid dimension limit.Related Issues or PRs
closes #967
Changes Made
Breaking Changes
Checklist