Skip to content

add MXFP8 pre-swizzling for gfx1250 GEMM (#568)#605

Open
matthiasdiener wants to merge 18 commits into
devfrom
mdiener/mxfp8-swizzle-gfx1250
Open

add MXFP8 pre-swizzling for gfx1250 GEMM (#568)#605
matthiasdiener wants to merge 18 commits into
devfrom
mdiener/mxfp8-swizzle-gfx1250

Conversation

@matthiasdiener
Copy link
Copy Markdown
Contributor

@matthiasdiener matthiasdiener commented Jun 1, 2026

Description

Cherry-picked from #568 (same code)

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

Please list the changes introduced in this PR:

  • Change A
  • Change B

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@matthiasdiener matthiasdiener self-assigned this Jun 1, 2026
@matthiasdiener matthiasdiener added the ci-level 1 CI test level 1 label Jun 1, 2026
@matthiasdiener matthiasdiener requested a review from alextmagro June 1, 2026 18:37
@matthiasdiener matthiasdiener marked this pull request as ready for review June 1, 2026 18:37
@matthiasdiener
Copy link
Copy Markdown
Contributor Author

Manually tested on gfx1250, should be ready to go from my perspective.

GTEST_SKIP() << "MXFP8 is not supported in current config";
}

// hipBLASLt on gfx950 produces incorrect results for certain small MXFP8
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there ticket for that?

if (is_nvfp4_scaling(config.scaling_mode)) {
if (is_nvfp4_scaling(config.scaling_mode)
#ifdef USE_ROCM
|| (config.scaling_mode == JAXX_Scaling_Mode::MXFP8_1D_SCALING
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think there should be corresponding update fort workspace size calculation: scale sizes should be added to it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-level 1 CI test level 1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants