Direct, immediate, and local fix to UpSampleNearest2D fwd to work around the UINT32_MAX limit of HIP runtime#3082
Open
glen-amd wants to merge 5 commits intorelease/2.10from
Open
Direct, immediate, and local fix to UpSampleNearest2D fwd to work around the UINT32_MAX limit of HIP runtime#3082glen-amd wants to merge 5 commits intorelease/2.10from
glen-amd wants to merge 5 commits intorelease/2.10from
Conversation
…nd the UINT32_MAX limit of HIP runtime
|
Jenkins build for c1fbfd13502a1870937f7e8f4995ff2827a738ae commit finished as FAILURE |
jeffdaily
reviewed
Mar 18, 2026
Comment on lines
+322
to
+333
| #if 0 | ||
| // TODO: The change below needs to work with | ||
| // a grid-strided loop in `vectorized_elementwise_kernel`. | ||
| // Simiar to: https://github.com/pytorch/pytorch/pull/169474 | ||
| #ifdef USE_ROCM | ||
| // Clamp the grid to ensure total threads (grid * num_threads) | ||
| // does not exceed the uint32_t limit of the HSA AQL packet. | ||
| // Use 4294967295 (UINT32_MAX) as the ceiling. | ||
| int64_t max_safe_grid = 4294967295LL / num_threads(); | ||
| grid = std::min(grid, max_safe_grid); | ||
| #endif | ||
| #endif |
Collaborator
There was a problem hiding this comment.
Why did you add an #if 0 block? For commentary?
Author
There was a problem hiding this comment.
- It's not required for the direct fix to the specific error we were working on. But it's identified as a potential (similar) issue while debugging the issue in question.
- As the
TODOcomment indicates, to be fully functional, this change would need some corresponding changes to the other function. - So, I commented out this change (using
#if 0) for now.
Collaborator
There was a problem hiding this comment.
Please remove from PR.
We can add it back when necessary.
Otherwise. Looks good to me
|
Jenkins build for 2a0a2b229e7e30577bedd61535f9d1343a667244 commit finished as FAILURE |
PR Review SummaryJira: ROCM-2926 ProblemHIP's HSA AQL dispatch packet stores Code Quality Assessment
Open Issues / Action Items
VerdictThe fix is technically sound — the root cause analysis is correct, the grid-stride loop pattern is the right approach, and the CUDA/ROCm split is clean. However, the PR is not ready to merge because:
|
|
Jenkins build for 7d3a91f696975197c73afe49349e2f980f553672 commit finished as FAILURE Detected error during Pytorch building: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please check the inline, detailed comments in the changed files.
Motivation
Technical Details
Test Plan
Test Result
Submission Checklist