Skip to content

feat(rocdbgapi): introduce clusters of workgroups (alternative) - #10241

Merged
aktemur merged 9 commits into
developfrom
users/aktemur/cluster-111
Sep 29, 2026
Merged

aktemur merged 9 commits into
developfrom
users/aktemur/cluster-111

Conversation

@aktemur

@aktemur aktemur commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

This is the same as PR #7992, except that it implements the "there is always a cluster" model.

@aktemur
aktemur requested a review from a team as a code owner August 17, 2026 17:08
@aktemur aktemur changed the title feat(rocdbgapi): introduce clusters of workgroups feat(rocdbgapi): introduce clusters of workgroups (alternative) Aug 17, 2026
@therock-pr-bot

therock-pr-bot Bot commented Aug 17, 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/logging.cpp (+10 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?

Comment thread projects/rocdbgapi/src/architecture.cpp Resolved
Comment thread projects/rocdbgapi/src/architecture.h Resolved
Comment thread projects/rocdbgapi/src/workgroup.cpp Resolved
@aktemur
aktemur force-pushed the users/aktemur/cluster-111 branch from ff2a049 to 7c8fbf5 Compare September 21, 2026 12:54
@aktemur

aktemur commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

The force-push above is a rebase.

@aktemur
aktemur force-pushed the users/aktemur/cluster-111 branch from 7c8fbf5 to 0bd0824 Compare September 22, 2026 10:02
@aktemur

aktemur commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

The force-push above updated the branch for the next review round.

@aktemur aktemur assigned lancesix and unassigned aktemur Sep 22, 2026
Comment thread projects/rocdbgapi/src/exportmap.in Resolved
Comment thread projects/rocdbgapi/src/queue.cpp Resolved
Comment thread projects/rocdbgapi/src/queue.cpp Resolved
@aktemur
aktemur force-pushed the users/aktemur/cluster-111 branch from 0bd0824 to d4b4c31 Compare September 23, 2026 16:49
@aktemur

aktemur commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

The force-push above is a rebase. As part of that, the PR now bumps the version to 0.82.

@aktemur
aktemur force-pushed the users/aktemur/cluster-111 branch from d4b4c31 to 705a012 Compare September 23, 2026 17:08

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

With latest changes, things look good to me, thanks.

Be sure to retest on the final target before merging.

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.
@aktemur
aktemur force-pushed the users/aktemur/cluster-111 branch from 705a012 to 939673f Compare September 24, 2026 15:11
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.

If a wave/workgroup is not in cluster mode, we nevertheless
conceptually consider the workgroup to belong to a single-workgroup
cluster and return cluster ids and sizes accordingly.

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 a dispatch is
not necessarily set in cluster mode.  We nevertheless create a
single-workgroup cluster even in that case, so that conceptually a
workgroup is always in a cluster.

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

Bug: AIROCGDB-77
@aktemur
aktemur force-pushed the users/aktemur/cluster-111 branch from 939673f to 2e780e1 Compare September 24, 2026 16:28
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 queries to obtain cluster support, mode, and sizes.

Clusters are supported at the architecture level.

Clusters are enabled/disabled and their size information is available
at the dispatch level.

Regarding 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
…uster

Add a workgroup query and a wave query to get the workgroup coordinates
within the cluster the workgroup belongs to.
@aktemur

aktemur commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

In nwg_in_cluster, the values obtained from ttmp6 are "minus 1". Hence, I made the following change in the last update:

-  nwg[0] = utils::bit_extract (ttmp6, 12, 15);
-  nwg[1] = utils::bit_extract (ttmp6, 16, 19);
-  nwg[2] = utils::bit_extract (ttmp6, 20, 23);
+  nwg[0] = utils::bit_extract (ttmp6, 12, 15) + 1;
+  nwg[1] = utils::bit_extract (ttmp6, 16, 19) + 1;
+  nwg[2] = utils::bit_extract (ttmp6, 20, 23) + 1;

@aktemur
aktemur force-pushed the users/aktemur/cluster-111 branch from 2e780e1 to b8b2cd8 Compare September 29, 2026 12:40
@aktemur

aktemur commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

I noticed I forgot the update the version in CMakeLists.txt. The last update makes this change:

+ project(amd-dbgapi VERSION 0.81.0 LANGUAGES CXX DESCRIPTION "ROCm Debugger API")
- project(amd-dbgapi VERSION 0.82.0 LANGUAGES CXX DESCRIPTION "ROCm Debugger API")

@aktemur

aktemur commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

Tested on gfx1250. I'll merge when the CI run finishes.

@aktemur
aktemur merged commit 6b97c66 into develop Sep 29, 2026
82 of 83 checks passed
@aktemur
aktemur deleted the users/aktemur/cluster-111 branch September 29, 2026 15:20
@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.

2 participants