Skip to content

fix(clr): keep hipLaunchKernelEx cluster dimensions in stream capture - #14228

Merged
iassiour merged 2 commits into
developfrom
users/iassiour/capture-cluster-dims
Oct 28, 2026
Merged

iassiour merged 2 commits into
developfrom
users/iassiour/capture-cluster-dims

Conversation

@iassiour

@iassiour iassiour commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

hipLaunchKernelEx with hipLaunchAttributeClusterDimension loses its cluster dimensions when the launch is stream-captured. hipLaunchKernelExC passes the decoded cluster dimensions to hipLaunchKernel_common, but the capture hook there only forwarded the plain hipLaunchKernel arguments. The kernel node then fell back to the kernel's __cluster_dims__ (or no cluster), and the graph replayed with the wrong cluster shape. hipDrvLaunchKernelEx capture is not affected.

Technical Details

  • capturehipLaunchKernel is replaced by capturehipLaunchKernelCommon, which takes the cluster dimensions from hipLaunchKernel_common.
  • Explicit cluster dimensions are set on the kernel node. Otherwise {0,0,0} is passed as before, so the node still resolves __cluster_dims__ or a runtime cluster attribute.
  • The dynamic data prefetch hint is still dropped from the captured node, as before.
  • New hipClusterStreamCapture.cc in ClusterTest. Both tests check the captured node's hipLaunchAttributeClusterDimension and the cluster shape seen by the replayed blocks:
    • Unit_hipClusterLaunch_StreamCapture_LaunchAttribute: hipLaunchKernelEx with and without a {2,1,1} cluster attribute.
    • Unit_hipClusterLaunch_StreamCapture_CompilerDirective: a __cluster_dims__(2,1,1) kernel launched via hipLaunchKernelGGL, hipLaunchKernelEx without attributes, and hipLaunchKernelEx with a matching attribute.

Issue Tracking

JIRA ID: AIRUNTIME-3085

Test Plan

  • Build CLR and ClusterTest.
  • Run Unit_hipClusterLaunch_StreamCapture_* on a device with cluster launch support, develop vs this PR.

Test Result

  • CLR and ClusterTest build cleanly with the new tests.

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/clr/hipamd/src/hip_graph.cpp, projects/clr/hipamd/src/hip_graph_capture.hpp, projects/clr/hipamd/src/hip_module.cpp, projects/hip-tests/catch/unit/cluster/hipClusterStreamCapture.cc; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 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

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

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.

🔵 Needs a closer look

No concrete defects were identified, but capture-and-replay behavior still needs validation on cluster-capable hardware before final human approval.

No open issues 🎉

What changed in this PR

This PR fixes HIP stream capture so hipLaunchKernelEx preserves explicit cluster dimensions in captured kernel nodes while retaining existing fallback behavior when dimensions are omitted.

Changes:

  • Forward decoded cluster dimensions through the common launch capture hook.
  • Preserve kernel-defined cluster requirements and continue dropping the dynamic data prefetch hint.
  • Add and register regression tests checking captured node attributes and replayed cluster shapes.
File Description
projects/​hip-tests/​catch/​unit/​cluster/​hipClusterStreamCapture.cc Tests explicit, omitted, and compiler-defined cluster dimensions during capture and replay.
projects/​hip-tests/​catch/​unit/​cluster/​CMakeLists.txt Adds the new test source to the build.
projects/​hip-tests/​catch/​config/​configs/​unit/​cluster.yaml Registers both stream-capture test cases.
projects/​clr/​hipamd/​src/​hip_module.cpp Forwards cluster arguments to the common capture hook.
projects/​clr/​hipamd/​src/​hip_graph.cpp Passes explicit dimensions or the fallback sentinel to kernel-node creation.
projects/​clr/​hipamd/​src/​hip_graph_capture.hpp Declares the updated capture hook and prefetch configuration type.

Review effort: Balanced

@iassiour
iassiour force-pushed the users/iassiour/capture-cluster-dims branch from e2d3157 to c355437 Compare October 26, 2026 12:35
@iassiour
iassiour marked this pull request as ready for review October 26, 2026 12:35
@iassiour
iassiour requested review from a team as code owners October 26, 2026 12:35

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

🤖 AI-assisted pre-review (for @mangupta)

@iassiour
iassiour force-pushed the users/iassiour/capture-cluster-dims branch from c355437 to d133b00 Compare October 27, 2026 00:10
iassiour and others added 2 commits October 27, 2026 00:55
hipLaunchKernelExC decodes hipLaunchAttributeClusterDimension and passes the
cluster dimensions to hipLaunchKernel_common, but the capture hook there only
forwarded the plain hipLaunchKernel arguments. A captured launch therefore lost
its cluster dimensions: the kernel node fell back to the kernel's own
requirement, or to no cluster, and replayed with the wrong cluster shape.
hipDrvLaunchKernelEx capture was not affected.

Replace capturehipLaunchKernel with capturehipLaunchKernelCommon, which takes
the decoded cluster dimensions and passes them to the kernel node when they
were given explicitly. When they were not, it passes {0,0,0} as before, so the
node still resolves __cluster_dims__ or a runtime cluster attribute. The
dynamic data prefetch hint is still dropped from the captured node, as before.

Add ClusterTest cases that capture hipLaunchKernelEx with and without a cluster
attribute, and launches of a __cluster_dims__ kernel without cluster
dimensions, and check both the captured node's cluster attribute and the
cluster shape seen by the replayed blocks.

Co-Authored-By: Claude <noreply@anthropic.com>
…ure tests

Co-Authored-By: Claude <noreply@anthropic.com>
@iassiour
iassiour force-pushed the users/iassiour/capture-cluster-dims branch from 999f1e4 to c8c5fb6 Compare October 27, 2026 00:55
@iassiour
iassiour requested a balanced review from Copilot October 27, 2026 00:58

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.

🔵 Needs a closer look

No blocking defects were identified, but graph replay still needs validation on cluster-capable hardware beyond the reported successful builds.

No open issues 🎉

Review effort: Balanced

@mangupta

Copy link
Copy Markdown
Contributor

🤖 AI-assisted pre-review (for @mangupta)

Re-reviewed the updated head (c8c5fb6af8a4) — no new findings at or above the posting threshold.

@iassiour
iassiour merged commit d529b86 into develop Oct 28, 2026
446 of 456 checks passed
@iassiour
iassiour deleted the users/iassiour/capture-cluster-dims branch October 28, 2026 18:01
@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.

4 participants