Skip to content

[HIP] Add offload PGO tiled matmul E2E test - #366

Open
yxsamliu wants to merge 1 commit into
llvm:mainfrom
yxsamliu:amd/dev/yaxunl/pgo-tiled-matmul-test
Open

[HIP] Add offload PGO tiled matmul E2E test#366
yxsamliu wants to merge 1 commit into
llvm:mainfrom
yxsamliu:amd/dev/yaxunl/pgo-tiled-matmul-test

Conversation

@yxsamliu

Copy link
Copy Markdown
Contributor

[HIP] Add offload PGO tiled matmul E2E test

Add a tiled matrix multiply kernel that demonstrates the offload PGO
workflow on AMDGPU. The kernel uses a large per-thread sub-tile
(configurable via -DTH_M and -DTH_N) with LDS-based cooperative tile
loading, creating natural register pressure that exceeds the VGPR
budget and causes spills. Boundary tile handling creates biased
branches that PGO can optimize by guiding the register allocator to
reduce spills on the hot path.

Sub-tile sizes are tunable per architecture to induce spills on GPUs
with different register file sizes.

Two tests are registered:

  • pgo-tiled-matmul: correctness test (compile + run + verify)
  • pgo-tiled-matmul-pipeline: full PGO pipeline test
    (baseline -> instrument -> collect -> merge -> PGO build -> compare)

The pipeline test verifies that the full -fprofile-generate /
-fprofile-use workflow completes successfully and reports the
performance difference for information.

@yxsamliu
yxsamliu requested review from jmmartinez and jplehr March 10, 2026 14:03
Comment thread External/HIP/pgo-tiled-matmul.hip Outdated
Comment thread External/HIP/workload/pgo/test_pgo_matmul.sh.in Outdated

@jmmartinez jmmartinez 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.

Looks good to me but I'll JP have the final word.

@jplehr jplehr 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.

LG
Just comments about the GPU arch of the bots.

Reporting the actual PGO speed-up just for information purpose is good.

Comment thread External/HIP/pgo-tiled-matmul.hip Outdated
Comment thread External/HIP/workload/pgo/test_pgo_matmul.sh.in Outdated
@yxsamliu
yxsamliu force-pushed the amd/dev/yaxunl/pgo-tiled-matmul-test branch 2 times, most recently from 1edfa31 to f6050ee Compare June 10, 2026 13:34
@yxsamliu

Copy link
Copy Markdown
Contributor Author

Gentle ping. The llvm-zorg PR needed by this test has landed now: llvm/llvm-zorg#868

All review comments here have been addressed and resolved. Could you take another look when you get a chance?

@yxsamliu
yxsamliu force-pushed the amd/dev/yaxunl/pgo-tiled-matmul-test branch from f6050ee to cf9c709 Compare July 25, 2026 14:08
Add a tiled matrix multiply kernel that demonstrates the offload PGO
workflow on AMDGPU. The kernel uses a large per-thread sub-tile
(configurable via -DTH_M and -DTH_N) with LDS-based cooperative tile
loading, creating natural register pressure that exceeds the VGPR
budget and causes spills. Boundary tile handling creates biased
branches that PGO can optimize by guiding the register allocator to
reduce spills on the hot path.

Sub-tile sizes are tunable per architecture to induce spills on GPUs
with different register file sizes.

Two tests are registered:
- pgo-tiled-matmul: correctness test (compile + run + verify)
- pgo-tiled-matmul-pipeline: full PGO pipeline test
  (baseline -> instrument -> collect -> merge -> PGO build -> compare)

The pipeline test verifies that the full -fprofile-generate /
-fprofile-use workflow completes successfully and reports the
performance difference for information.
@yxsamliu
yxsamliu force-pushed the amd/dev/yaxunl/pgo-tiled-matmul-test branch from cf9c709 to 90cd42b Compare July 25, 2026 17:27
@yxsamliu

Copy link
Copy Markdown
Contributor Author

@jmmartinez I updated the test so the PGO pipeline only runs when both required profile libraries are available. I also fixed the script so compiler failures are not hidden. Could you please take another look?

@jmmartinez jmmartinez 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.

The new CMake changes look good to me.

Comment on lines +233 to +234
_hip_profile_rocm_count GREATER 0 AND
_hip_device_profile_count GREATER 0)

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.

Nitpick: In CMake, does an empty list evaluate to false in this context ?

That could allow us to avoid the LENGHT and just check for emptiness of _hip_profile_rocm_count and _hip_device_profile_count

@jplehr jplehr 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.

Are there requirements on the installed ROCm version for this to work?

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.

3 participants