Skip to content

Enable fix from PR #11963 for gfx950 - #12187

Merged
amd-ysanchez merged 8 commits into
developfrom
mx_overhead_fix_gfx950
Sep 18, 2026
Merged

amd-ysanchez merged 8 commits into
developfrom
mx_overhead_fix_gfx950

Conversation

@amd-ysanchez

@amd-ysanchez amd-ysanchez commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

JIRA ID : AIHPBLAS-4756

Motivation

This change adds regression coverage for the MX swizzle cache behavior introduced and refined around PR #11963.

The key issue is that MX scale reuse is not uniform across architectures:

  • on gfx950, the host-side preswizzled MX scale can be solution-dependent because the swizzled layout is tied to the selected matrix-instruction shape
  • on gfx1250, the swizzled MX scale layout is derived from problem geometry, so unchanged descriptors can safely reuse previously generated swizzled device data

That difference means stale reuse bugs are easy to introduce if the host cache state is not kept truthful. In this path, correctness depends on the interaction between canonical host buffers, preswizzled device buffers, per-solution reinitialization, and the m_mxSwizzledDescriptor cache tag. This PR adds direct regression coverage for those rules and makes the control flow easier to reason about.

This work builds upon PR #11963

Technical Details

This PR strengthens both the implementation clarity and the regression net around MX scale swizzle reuse in TensileLite.

In tensilelite/client/src/DataInitialization.cpp, the existing MX initialization and upload flow is now explicitly described at the decision points that matter for correctness:

  • The initializeMXData() path distinguishes between gfx950 and gfx1250 reuse semantics.

    • For gfx1250, if the MX data and scale descriptors are unchanged, the generated swizzled scale bytes are fully determined by problem geometry, so the existing gpuInput.valid swizzled buffer remains reusable.
    • For gfx950, descriptor equality alone is not enough because the preswizzled layout also depends on the selected solution’s matrix-instruction shape. Reuse is therefore only safe when the cached swizzled descriptor still matches the current scale descriptor and the preswizzled state has been explicitly revalidated.
  • The m_mxSwizzledDescriptor cache is treated as truth metadata, not just an optimization hint.

    • When the path falls back to canonical scale upload, the descriptor cache entry is erased before upload so later calls cannot incorrectly assume that gpuInput.valid still contains a valid swizzled layout.
    • When a new swizzled MX scale is generated and copied into gpuInput.valid, the descriptor cache is repopulated with the exact scale descriptor so future reuse checks can skip unnecessary host-side swizzling.
    • When fallback host-side swizzle generation is required, stale cache metadata is cleared before regenerating device state.
  • The copySwizzledToGPUBuffer() path now has regression coverage for the core branch behavior:

    • if a kernel requests canonical MX scales (problemType.mxScaleFormat == 0), the code uploads cpuInput.valid and invalidates any cached swizzled descriptor for that tensor
    • if a preswizzled scale was already produced into gpuInput.valid, the device buffer is reused directly
    • if the cache records that gpuInput.valid already contains the correct swizzled bytes for the current descriptor, host-side re-swizzling is skipped
    • otherwise the code falls back to rebuilding the swizzled layout from canonical host data

In tensilelite/tests/DataInit_test.cpp, this PR adds runtime-targeted coverage for the architecture split:

  • Gfx950RequiresPerSolutionRecomputeForSwizzledMX

    • constructs a real ClientProblemFactory + DataInitialization fixture using valid client-style arguments
    • verifies that on gfx950, referenceNeedsPerSolutionRecompute(...) is false for a canonical solution (mxScaleFormat == 0) and true for a swizzled solution (mxScaleFormat == 1)
    • this directly covers the solution-dependent host preswizzle behavior added for gfx950
  • Gfx1250ReusesPreswizzledMXAcrossSolutions

    • uses the same production fixture path
    • verifies that on gfx1250, a swizzled MX solution does not require per-solution recompute, which preserves the original descriptor-based reuse behavior
    • this ensures the gfx1250 optimization remains tested even though the original behavior previously had no dedicated regression coverage

The reasoning for these branches and cache transitions is also captured inline in source comments at the changed control-flow points, so future edits do not have to reconstruct the design intent from behavior alone.

Test Plan

  1. Rebuild the TensileLite C++ test binary with tests enabled:

    • cmake --preset tensilelite -S .. -B my-custom-build -DTENSILELITE_BUILD_TESTING=ON -DHIPBLASLT_ENABLE_YAML=OFF
    • cmake --build my-custom-build --parallel --target tensilelite-tests
  2. Run the focused MX runtime regression tests:

    • ./my-custom-build/tensilelite/tests/tensilelite-tests --gtest_filter='DataInitializationReferenceRecompute.*'
  3. Re-run the targeted Tensile config tests covering the MX YAML surfaces:

    • python -m pytest Tensile/Tests/common/test_config.py -q -k 'sk_mx32f4_quick.yaml or subtile_mxfp8_tail_loop.yaml or mxfp4_mxfp4_fp32_tn_act.yaml or sk_mx32f4_force_dp_only.yaml or sk_dynamic_mxfp4.yaml or subtile_mxfp8_tail_loop_smoke.yaml or sk_mxfp4_atomic_variants.yaml or sk_mxfp4_problem_sizes.yaml or mx32f4_tn.yaml or sk_mxfp4_debug_modes.yaml or sk_mxfp4_tree_reduction.yaml or sk_mxfp4_comprehensive.yaml or sk_mxfp4_full_coverage.yaml or sk_mxfp4_basic_variants.yaml or sk_mxf4gemm_pap.yaml or subtile_mxfp4.yaml or mxfp4_mxfp4_bf16_tn_act.yaml'

Test Result

  • tensilelite-tests built successfully from the custom build tree with:

    • TENSILELITE_BUILD_TESTING=ON
    • HIPBLASLT_ENABLE_YAML=OFF
  • Focused MX runtime gtests:

    • DataInitializationReferenceRecompute.Gfx950RequiresPerSolutionRecomputeForSwizzledMX passed on gfx950
  • Targeted Tensile config tests:

    • 17 passed, 398 deselected, 15 warnings
    • warnings were PytestUnknownMarkWarning for existing custom markers and did not indicate behavioral failures

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Sep 16, 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.

@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 22 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...aslt/tensilelite/client/src/DataInitialization.cpp 0.00% 22 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #12187      +/-   ##
===========================================
- Coverage    70.82%   70.40%   -0.42%     
===========================================
  Files         2845     2812      -33     
  Lines       465988   462691    -3297     
  Branches     68628    68146     -482     
===========================================
- Hits        330026   325750    -4276     
- Misses      112214   113224    +1010     
+ Partials     23748    23717      -31     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 48.83% <0.00%> (+2.42%) ⬆️
TensileLite-Unit 76.22% <ø> (ø)
hipBLAS 90.62% <ø> (ø) Carriedforward from ba23120
hipBLASLt 35.28% <ø> (ø) Carriedforward from ba23120
hipCUB 82.68% <ø> (ø) Carriedforward from ba23120
hipDNN 87.01% <ø> (+0.03%) ⬆️ Carriedforward from ba23120
hipFFT 43.07% <ø> (-0.18%) ⬇️ Carriedforward from ba23120
hipRAND 76.12% <ø> (ø) Carriedforward from ba23120
hipSOLVER 68.92% <ø> (-0.04%) ⬇️ Carriedforward from ba23120
hipSPARSE 86.99% <ø> (ø) Carriedforward from ba23120
rocBLAS 48.31% <ø> (ø) Carriedforward from ba23120
rocFFT 47.42% <ø> (-4.17%) ⬇️ Carriedforward from ba23120
rocRAND 57.42% <ø> (ø) Carriedforward from ba23120
rocSOLVER 76.83% <ø> (ø) Carriedforward from ba23120
rocSPARSE 74.61% <ø> (ø) Carriedforward from ba23120
rocThrust 91.60% <ø> (-1.55%) ⬇️ Carriedforward from ba23120

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...aslt/tensilelite/client/src/DataInitialization.cpp 7.46% <0.00%> (+7.46%) ⬆️

... and 422 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@smalekta
smalekta self-requested a review September 17, 2026 12:58
@amd-ysanchez
amd-ysanchez enabled auto-merge (squash) September 17, 2026 16:26
@amd-ysanchez
amd-ysanchez merged commit 8df4f70 into develop Sep 18, 2026
180 of 186 checks passed
@amd-ysanchez
amd-ysanchez deleted the mx_overhead_fix_gfx950 branch September 18, 2026 13:31
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.

2 participants