Skip to content

[AMD][GFX950] Add MI355 support and fix some rocm related issues #2025

Closed
zhangnju wants to merge 5 commits intotile-ai:mainfrom
zhangnju:main
Closed

[AMD][GFX950] Add MI355 support and fix some rocm related issues #2025
zhangnju wants to merge 5 commits intotile-ai:mainfrom
zhangnju:main

Conversation

@zhangnju
Copy link
Copy Markdown
Collaborator

@zhangnju zhangnju commented Apr 9, 2026

HI

This PR added MI355 support in the rocm docker file, and fix some issues when running the examples on MI355:

  1. MFMA ldmatrix does not support pipelined (3D) shared buffer indexing:
    issue: in some tests, we may meet the error: Buffer A_shared is 3-dimensional, cannot be indexed with the 2-dimensional indices provided.
    root cause : In MatrixCoreIntrinEmitter.ldmatrix_a/b, when a shared buffer becomes 3D after pipeline transformation (e.g., [num_stages, block_M, block_K]), the old codes only extracts base indices from the last two dimensions (region[-2].min, region[-1].min), ignoring the leading stage dimension.

  2. HIP codegen uses function call syntax instead of template syntax for rasterization2DRow
    issue: hipcc reports no matching function for call to 'rasterization2DRow'
    root cause: CUDA codegen generates tl::rasterization2DRow<10>() (template instantiation syntax), while HIP codegen incorrectly generates tl::rasterization2DRow(10) (function call syntax). Since this is a template <int panel_width> function, it cannot accept a runtime argument.

  3. HIP codegen takes address of temporary object in float32x8 broadcast
    issue: hipcc reports taking the address of a temporary object of type 'float2'
    root cause: CUDA allows taking the address of a temporary object returned by make_float2(...), but HIP is stricter and disallows this.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added support for additional ROCm GPU architecture.
  • Improvements

    • Optimized HIP code generation and matrix core operation code generation for improved performance.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

👋 Hi! Thank you for contributing to the TileLang project.

Please remember to run pre-commit run --all-files in the root directory of the project to ensure your changes are properly linted and formatted. This will help ensure your contribution passes the format check.

We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 9, 2026

📝 Walkthrough

Walkthrough

The PR updates ROCm GPU architecture configurations, refines HIP code generation for swizzle patterns and float2 reinterpretation using templated instantiation and lambda-based union constructs, and extends matrix core intrinsics to handle leading/pipeline-stage buffer dimensions through refined indexing logic.

Changes

Cohort / File(s) Summary
ROCm GPU Architecture Configuration
docker/Dockerfile.rocm
Added gfx950 GPU architecture to PYTORCH_ROCM_ARCH environment variable for expanded ROCm target support.
HIP Code Generation
src/target/codegen_hip.cc
Modified threadblock swizzle pattern to use templated instantiation syntax (tl::<func_name><panel_size>() instead of function call); refactored float2-to-unsigned-long-long reinterpretation using an immediately-invoked lambda with internal union to avoid temporary address-taking.
Matrix Core Intrinsics
tilelang/intrinsics/mfma_macro_generator.py
Extended ldmatrix_a and ldmatrix_b methods to extract leading dimensions and incorporate them into shared-buffer indexing via tuple concatenation, enabling support for multi-dimensional buffers with pipeline stages.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • LeiWang1999
  • Gongen-Ali

Poem

🐰 A GPU's new gfx950 arrives with cheer,
Templates twirl where functions once were near,
Lambdas dance with unions, avoiding that address grab,
Leading dimensions align in buffers' tab—
Matrices march forth through pipeline stages fast! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: adding MI355 (GFX950) support to the ROCm setup and addressing multiple rocm-related issues found during testing.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
tilelang/intrinsics/mfma_macro_generator.py (2)

331-331: Nitpick: EN DASH in comment.

The comment uses an EN DASH () instead of a regular hyphen (-). This is cosmetic but could cause issues with tools expecting ASCII.

-        # Leading dimensions (e.g. pipeline stage axis) – empty for 2-D buffers
+        # Leading dimensions (e.g. pipeline stage axis) - empty for 2-D buffers

(Same on line 375)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tilelang/intrinsics/mfma_macro_generator.py` at line 331, Replace the
non-ASCII EN DASH with a regular hyphen in the inline comment string "Leading
dimensions (e.g. pipeline stage axis) – empty for 2-D buffers" and the similar
comment around line 375 in tilelang/intrinsics/mfma_macro_generator.py; search
for that exact comment text in the file (likely within the generate_mfma_macro
or related function) and change "–" to "-" to use an ASCII hyphen.

331-354: LGTM – correctly handles leading dimensions for pipelined buffers.

The fix properly extracts leading dimension indices (e.g., pipeline stage) and prepends them to the 2D indexing. For standard 2D buffers, A_other is empty, preserving the original behavior.

Optional style improvement per RUF005: consider using iterable unpacking for slightly cleaner syntax:

♻️ Optional: use iterable unpacking
-                        A_local_buf[i * k_pack * local_size_a + local_id] = A_buf[tuple(A_other) + (A_base0 + l + row, A_base1 + r + col)]
+                        A_local_buf[i * k_pack * local_size_a + local_id] = A_buf[(*A_other, A_base0 + l + row, A_base1 + r + col)]

(Same pattern for line 354)

,

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tilelang/intrinsics/mfma_macro_generator.py` around lines 331 - 354, Replace
the manual slice for leading dims with iterable unpacking to make intent
clearer: unpack A_region.region into leading_regions, _, _ (e.g.
leading_regions, _, _ = A_region.region) and then set A_other = [r.min for r in
leading_regions]; this keeps the same semantics used by _warp_ldmatrix_a while
using clearer syntax referencing A_region.region and A_other.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tilelang/intrinsics/mfma_macro_generator.py`:
- Line 331: Replace the non-ASCII EN DASH with a regular hyphen in the inline
comment string "Leading dimensions (e.g. pipeline stage axis) – empty for 2-D
buffers" and the similar comment around line 375 in
tilelang/intrinsics/mfma_macro_generator.py; search for that exact comment text
in the file (likely within the generate_mfma_macro or related function) and
change "–" to "-" to use an ASCII hyphen.
- Around line 331-354: Replace the manual slice for leading dims with iterable
unpacking to make intent clearer: unpack A_region.region into leading_regions,
_, _ (e.g. leading_regions, _, _ = A_region.region) and then set A_other =
[r.min for r in leading_regions]; this keeps the same semantics used by
_warp_ldmatrix_a while using clearer syntax referencing A_region.region and
A_other.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dc4a5876-db26-4fb3-b4f0-82b06f757d93

📥 Commits

Reviewing files that changed from the base of the PR and between 86e37b7 and 4503bfa.

📒 Files selected for processing (3)
  • docker/Dockerfile.rocm
  • src/target/codegen_hip.cc
  • tilelang/intrinsics/mfma_macro_generator.py

@zhangnju zhangnju closed this by deleting the head repository Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant