Skip to content

feat(rocprim): tune new device scan params on gfx942 - #13743

Closed
NguyenNhuDi wants to merge 2 commits into
users/NguyenNhuDi/device_transform_kernel_tunerfrom
users/jmahovsk/port-pr12642-rocprim-scan-tuning
Closed

NguyenNhuDi wants to merge 2 commits into
users/NguyenNhuDi/device_transform_kernel_tunerfrom
users/jmahovsk/port-pr12642-rocprim-scan-tuning

Conversation

@NguyenNhuDi

Copy link
Copy Markdown
Contributor

Motivation

#11702 did add optimizations, but did not include them to the tuning mechanisms. This PR addresses that and tunes said algorithms for MI300X.

This PR should not be merged before #11702. After the aforementioned PR is merged, commit 345fe33 should be dropped from this PR.

JIRA ID : EXSWSTRHPC-330
JIRA ID : EXSWSTRHPC-331

Technical Details

345fe33: #11702 (should be dropped)
96b6d8f: adds the new parameters to the search space
36cee5f: allows tuning with mismatching default parameters
3fd8430: the resulting configs

Test Plan

Tests pass.
Performance improved.

Test Result

CI passes.
Performance verified on MI300X node.

Submission Checklist

…Issue #207883) (#12266)

## Motivation

<!-- Explain the purpose of this PR and the goals it aims to achieve.
-->

This PR fixes a race condition in SYEVJ GPU kernel. The problem showed
up on gfx11 architecture.
The solution is to add calls to __syncthreads() to avoid race condition
hazards in accessing arrays cosine_res[] and sines_diag[] in LDS shared
memory.

This will address 

JIRA ROCM-31219
https://amd-hub.atlassian.net/browse/ROCM-31219



## Technical Details

<!-- Explain the changes along with any relevant GitHub links. -->

The problem was initially identified by github user zpengmei in PR 9135
(now closed)

#9135



The issue was initially thought to be related to "[AMDGPU] Miscompiled
post-barrier LDS reload in a large-workgroup HIP kernel on gfx11 (silent
wrong results) #207883)

llvm/llvm-project#207883

Anshil Gandhi (angandhi@amd.com) has identified the source of the
problem of race condition where synchronizations are required.

This PR adds the calls to __synchthreads() in the GPU kernel for SYEVJ.


## Test Plan

<!-- Explain any relevant testing done to verify this PR. -->
Pass google tests for all routines that use the Jacobi method on all
architectures, including gfx11.

## Test Result

<!-- Briefly summarize test outcomes. -->

All tests passed.

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests.

---------

Co-authored-by: Mark Gates <mark.gates@amd.com>
@NguyenNhuDi
NguyenNhuDi added this pull request to stack #13742 October 15, 2026 16:44
@NguyenNhuDi
NguyenNhuDi marked this pull request as ready for review October 15, 2026 16:45
@NguyenNhuDi
NguyenNhuDi requested a review from a team as a code owner October 15, 2026 16:45
@NguyenNhuDi NguyenNhuDi changed the title gfx942 tuning for lookback scan feat(rocprim): tune new device scan params on gfx942 Oct 15, 2026
@NguyenNhuDi
NguyenNhuDi force-pushed the users/jmahovsk/port-pr12642-rocprim-scan-tuning branch from 4532031 to 6141bcf Compare October 15, 2026 16:48
@NguyenNhuDi
NguyenNhuDi requested review from a team as code owners October 15, 2026 16:48
@therock-pr-bot

therock-pr-bot Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@NguyenNhuDi
NguyenNhuDi removed this pull request from stack #13742 October 15, 2026 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants