Skip to content

feat(rocdbgapi): introduce clusters of workgroups - #7992

Closed
aktemur wants to merge 9 commits into
developfrom
users/aktemur/cluster
Closed

aktemur wants to merge 9 commits into
developfrom
users/aktemur/cluster

Conversation

@aktemur

@aktemur aktemur commented Jun 30, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Add a new level of hierarchy, "clusters", to the layering of wave -> workgroup -> cluster -> dispatch.

Technical Details

Define clusters as a new layer, similar to how workgroups are defined.

JIRA ID: AIROCGDB-77

Test Plan

To be done covered by GDB tests.

Test Result

Not yet available.

@aktemur
aktemur requested a review from a team as a code owner June 30, 2026 05:37
@therock-pr-bot

therock-pr-bot Bot commented Jun 30, 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/rocdbgapi/src/architecture.cpp, projects/rocdbgapi/src/architecture.h, projects/rocdbgapi/src/cluster.cpp, projects/rocdbgapi/src/cluster.h, projects/rocdbgapi/src/dispatch.cpp (+11 more); 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

therock-pr-bot Bot commented Jun 30, 2026 •

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ Unit Test

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

Comment thread projects/rocdbgapi/src/architecture.cpp Resolved
@aktemur aktemur changed the title rocdbgapi: introduce clusters of workgroups feat(rocdbgapi): introduce clusters of workgroups Jun 30, 2026

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

A first quick pass, this looks promising already, thanks!

In the commit msg for "rocdbgapi: create clusters of workgroups"

"We have to consider take the cluster id" either "consider" or "take" is too much.

Comment thread projects/rocdbgapi/src/architecture.cpp Resolved
Comment thread projects/rocdbgapi/src/architecture.h Resolved
Comment thread projects/rocdbgapi/src/cluster.h Resolved
Comment thread projects/rocdbgapi/src/logging.cpp Resolved
@aktemur

aktemur commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

The update above has two major differences wrt to the previous revision:

  1. Use first-of-cluster/last-of-cluster to determine if a wave is in a cluster.
  2. Define an API to query cluster sizes (similar to workgroup sizes and grid sizes).

Comment thread projects/rocdbgapi/src/architecture.cpp Resolved
Comment thread projects/rocdbgapi/src/cluster.h Resolved
Comment thread projects/rocdbgapi/src/cluster.h Resolved
Comment thread projects/rocdbgapi/src/workgroup.cpp Resolved
@aktemur
aktemur force-pushed the users/aktemur/cluster branch from 5c4c116 to 6c615bc Compare July 7, 2026 13:29
@aktemur

aktemur commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

I addressed some of Lancelot's comments. There are still a few opens that need to be clarified.

@aktemur
aktemur force-pushed the users/aktemur/cluster branch 2 times, most recently from 8fd0911 to e9ea232 Compare July 16, 2026 07:14
@aktemur
aktemur requested a review from lancesix July 16, 2026 07:17

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

(review for patch 3)

Comment thread projects/rocdbgapi/src/architecture.cpp Resolved
Comment thread projects/rocdbgapi/src/architecture.cpp Resolved
Comment thread projects/rocdbgapi/src/architecture.cpp Resolved

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

(patch 4)

Comment thread projects/rocdbgapi/src/cluster.h Resolved
Comment thread projects/rocdbgapi/src/architecture.cpp Resolved
Comment thread projects/rocdbgapi/src/architecture.cpp Resolved
Comment thread projects/rocdbgapi/src/process.cpp Resolved
Comment thread projects/rocdbgapi/src/queue.cpp Resolved
Comment thread projects/rocdbgapi/CHANGELOG.md Resolved
@aktemur
aktemur force-pushed the users/aktemur/cluster branch from e9ea232 to 5ccf7d9 Compare July 16, 2026 12:16
@aktemur aktemur assigned aktemur and unassigned lancesix Jul 16, 2026
@aktemur

aktemur commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

The latest push is only a rebase.

@aktemur
aktemur force-pushed the users/aktemur/cluster branch from 5ccf7d9 to d171615 Compare July 16, 2026 13:36
@aktemur

aktemur commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

The update above addresses the comments received so far.

@aktemur
aktemur force-pushed the users/aktemur/cluster branch from 972adda to 62cfcaf Compare August 3, 2026 11:08
@aktemur aktemur assigned lancesix and unassigned aktemur Aug 3, 2026

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

I think we are almost there.

Comment thread projects/rocdbgapi/src/dispatch.cpp Resolved
@aktemur
aktemur force-pushed the users/aktemur/cluster branch from 62cfcaf to 60892d9 Compare August 7, 2026 20:46
@aktemur

aktemur commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@lancesix, the last force-push updates the CLUSTER_SIZES query. Please check.

@aktemur
aktemur force-pushed the users/aktemur/cluster branch from 60892d9 to 42d3a6e Compare August 12, 2026 14:09
@aktemur

aktemur commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

The force-push above introduces queries to get workgroup ids within the cluster.

@aktemur
aktemur force-pushed the users/aktemur/cluster branch from 42d3a6e to 18c37dd Compare August 17, 2026 11:19
In two places, we get a workgroup's ids by calling `group_ids ()`,
which returns an optional tuple, and if the optional is non-empty, we
call `group_ids ()` again.  Avoid the duplicated call.
A workgroup is alive as long as it has a wave.  Set the epoch mark
unconditionally, whenever the mark of any wave of the workgroup is
being set.

In practice, this should not change the epoch value of a workgroup.
This change is a refactoring to obtain clearer and simpler code.
Starting with gfx12-5, workgroups may belong to clusters.  Add methods
to read cluster ids from a CWSR record.

The cluster and group ids are read from TTMP registers.  The meaning
of the registers depend on whether the wave is in cluster mode.  If
not in a cluster, group ids are obtained from TTMP7 and TTMP9 as in
previous gfx12 architecture.  If in a cluster, what we read from TTMP7
and TTMP9 are cluster ids.  The group ids in this case are read from
TTMP6.

To figure out if the wave is in cluster mode, we keep track of the
"first_of_cluster" and "last_of_cluster" bits in the compute relaunch
wave register.  All CWSR records created for waves between the first
and the last of cluster (inclusive) are marked to be in a cluster.

Bug: AIROCGDB-77
Add a first definition for clusters, which are groups of workgroups.

Bug: AIROCGDB-77
Starting with gfx12-5, workgroups may be grouped into clusters.  When
creating workgroups, take this into account.  Note that being part of
a cluster is not mandatory.  In that case, the cluster id would be
std::nullopt.  We nevertheless create clusters and associate
workgroups to them, to avoid having to handle non-cluster mode
separately.  In cluster mode, a cluster id would be a valid 3D
coordinate.  In this case, a workgroup's group id is not necessarily
unique.  We have to take also the cluster id into account to identify
workgroups uniquely.

This patch adds clusters into the wave -> workgroup -> cluster ->
dispatch hierarchy.  API queries are added in the next patch.

Bug: AIROCGDB-77
Define new enums in the API to query the cluster coordinate of a wave
and a workgroup, and other cluster-related information.

Bug: AIROCGDB-77
Add API query to obtain cluster sizes.  To be in consistence with the
existing queries that return workgroup sizes and grid sizes
(i.e. AMD_DBGAPI_DISPATCH_INFO_WORKGROUP_SIZES and
AMD_DBGAPI_DISPATCH_INFO_GRID_SIZES), the new query
(i.e. AMD_DBGAPI_DISPATCH_INFO_CLUSTER_SIZES) returns the sizes in
units of work-items.

Bug: AIROCGDB-77
@aktemur
aktemur force-pushed the users/aktemur/cluster branch from 18c37dd to 95fde49 Compare August 17, 2026 11:50
@aktemur

aktemur commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

The latest push above is only a rebase.

…uster

Add a workgroup query and a wave query to get the workgroup coordinates
within the cluster the workgroup belongs to.
@aktemur
aktemur force-pushed the users/aktemur/cluster branch from 95fde49 to 5a105b3 Compare August 17, 2026 11:54
@aktemur

aktemur commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

The push above removes two more "... in units of ..." sentences.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

  • Add new commits
  • Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

@github-actions github-actions Bot added the Stale PR has no activity for 25+ days label Sep 12, 2026
@aktemur

aktemur commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

This PR should remain open as a reference to #10241.

@aktemur aktemur removed the Stale PR has no activity for 25+ days label Sep 14, 2026
@aktemur

aktemur commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

The PR that replaces this PR has been merged: #10241
Closing.

@aktemur aktemur closed this Sep 29, 2026
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.

3 participants