Skip to content

[rocjitsu] Align CDNA5 BF16 WMMA accumulation - #13837

Merged
kuhar merged 1 commit into
developfrom
users/kuhar/cdna5-wmma-accumulation
Oct 20, 2026
Merged

kuhar merged 1 commit into
developfrom
users/kuhar/cdna5-wmma-accumulation

Conversation

@kuhar

@kuhar kuhar commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Triton's test_scaled_dot[128-64-128-True-False-False-e2m1-e4m3-4-16-1] fails on simulated gfx1250 at (111, 35): the scaled-dot kernel returns 0.0126953125, while the hipBLASLt reference returns 0.01171875. Large cancelling BF16 products discard the small incoming C value in the reference's dense WMMA instruction.

Accumulate the 32 BF16 products before adding FP32 C in both v_wmma_f32_16x16x32_bf16 and v_wmma_bf16f32_16x16x32_bf16, in scalar and SIMD execution. Retain the mixed form's final BF16 conversion. This fixes the Triton numerical failure and gives the two forms a consistent accumulation policy; it does not establish the actual accumulation order on physical CDNA5 hardware.

Extend the decoded cancellation regression to both forms, covering C modifiers, inline constants, and overlapping register tuples. Update the overflow regression: a dot that overflows before C is added remains infinite. The grouping is a modeling choice, not a general improvement in mathematical accuracy.

Related report: ROCm/rocm-systems#13082

Validation:

  • The expanded decoded regression fails for the BF16-output form on the previous local commit and passes with the paired change.
  • The original Triton reproducer passes with identical captured inputs, an unchanged kernel binary, and unchanged tolerances; all 8,192 output words agree.
  • 163 focused C++ tests pass in an AVX-512 build, including forced-scalar/SIMD comparisons, narrowing, NaN propagation, overflow, register overlap, execution-plugin and ABI checks.
  • Decoded cancellation probes confirm agreement between the two forms for both tested cancellation orders, in default and forced-scalar execution.
  • Changed-file pre-commit checks pass.

Issue: #13082

@kuhar
kuhar requested review from a team and atgutier as code owners October 20, 2026 18:59
Copilot AI balanced review requested due to automatic review settings October 20, 2026 18:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@therock-pr-bot

therock-pr-bot Bot commented Oct 20, 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.

Accumulate the 32 BF16 products before adding FP32 C in both the F32
and BF16F32 16x16x32 forms. Keep the mixed form's final BF16 conversion.
Use the same grouping in scalar and SIMD execution.

This preserves the small incoming C value when large products cancel,
fixing the reported Triton test_scaled_dot numerical failure. Extend
the decoded regression to both forms, including modifiers, inline
constants and overlapping register tuples. Update the overflow case
for the dot being rounded before C is added.

The grouping is a consistent simulator policy. It does not establish
the accumulation order used by physical CDNA5 hardware.
@kuhar
kuhar force-pushed the users/kuhar/cdna5-wmma-accumulation branch from 4a6e312 to 2114d2a Compare October 20, 2026 20:06
@kuhar
kuhar merged commit 473a025 into develop Oct 20, 2026
71 checks passed
@kuhar
kuhar deleted the users/kuhar/cdna5-wmma-accumulation branch October 20, 2026 21:09
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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