* [PATCH 2/2] gdb, amd-dbgapi-target: split wave_coordinates::to_string
2026-07-03 12:50 [PATCH 1/2] gdb, amd-dbgapi-target: use '%u' instead of '%d' in wave_coordinates::to_string Tankut Baris Aktemur
@ 2026-07-03 12:50 ` Tankut Baris Aktemur
2026-07-03 13:16 ` Simon Marchi
2026-07-03 13:12 ` [PATCH 1/2] gdb, amd-dbgapi-target: use '%u' instead of '%d' in wave_coordinates::to_string Simon Marchi
2026-07-03 13:42 ` [PATCH v2 1/2] gdb, amd-dbgapi-target: use pulongest more Tankut Baris Aktemur
2 siblings, 1 reply; 8+ messages in thread
From: Tankut Baris Aktemur @ 2026-07-03 12:50 UTC (permalink / raw)
To: gdb-patches; +Cc: lancelot.six
wave_coordinates::to_string() produces a string in the following format:
AMDGPU Wave a:q:d:w (x,y,z)/i
Split the method into smaller pieces:
a:q:d:w : hierarchy_str
(x,y,z) : workgroup_coord_str
(x,y,z)/i : dispatch_pos_str
This is a refactoring to allow reusing pieces. Currently the reuse
opportunity exists in the downstream debugger.
---
gdb/amd-dbgapi-target.c | 55 +++++++++++++++++++++++++++++++++--------
1 file changed, 45 insertions(+), 10 deletions(-)
diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
index 923b790664e..33512695fb5 100644
--- a/gdb/amd-dbgapi-target.c
+++ b/gdb/amd-dbgapi-target.c
@@ -147,6 +147,17 @@ struct wave_coordinates
: wave_id (wave_id)
{}
+ /* Return the string showing the agent -> queue -> dispatch -> wave
+ hierarchy. */
+ std::string hierarchy_str () const;
+
+ /* Return the workgroup coordinates as a string. */
+ std::string workgroup_coord_str () const;
+
+ /* Return the dispatch position string for the wave this
+ wave_coordinates is for. */
+ std::string dispatch_pos_str () const;
+
/* Return the target ID string for the wave this wave_coordinates is
for. */
std::string to_string () const;
@@ -387,13 +398,12 @@ get_amd_dbgapi_inferior_info (inferior *inferior)
static async_event_handler *amd_dbgapi_async_event_handler = nullptr;
std::string
-wave_coordinates::to_string () const
+wave_coordinates::hierarchy_str () const
{
- std::string str = "AMDGPU Wave";
-
- str += (agent_id != AMD_DBGAPI_AGENT_NONE
- ? string_printf (" %s", pulongest (agent_id.handle))
- : " ?");
+ std::string str
+ = (agent_id != AMD_DBGAPI_AGENT_NONE
+ ? string_printf (" %s", pulongest (agent_id.handle))
+ : "?");
str += (queue_id != AMD_DBGAPI_QUEUE_NONE
? string_printf (":%s", pulongest (queue_id.handle))
@@ -405,11 +415,25 @@ wave_coordinates::to_string () const
str += string_printf (":%s", pulongest (wave_id.handle));
- str += (group_ids[0] != UINT32_MAX
- ? string_printf (" (%u,%u,%u)", group_ids[0], group_ids[1],
- group_ids[2])
- : " (?,?,?)");
+ return str;
+}
+
+std::string
+wave_coordinates::workgroup_coord_str () const
+{
+ std::string str
+ = (group_ids[0] != UINT32_MAX
+ ? string_printf ("(%u,%u,%u)", group_ids[0], group_ids[1],
+ group_ids[2])
+ : "(?,?,?)");
+ return str;
+}
+
+std::string
+wave_coordinates::dispatch_pos_str () const
+{
+ std::string str = workgroup_coord_str ();
str += (wave_in_group != UINT32_MAX
? string_printf ("/%u", wave_in_group)
: "/?");
@@ -417,6 +441,17 @@ wave_coordinates::to_string () const
return str;
}
+std::string
+wave_coordinates::to_string () const
+{
+ std::string str = "AMDGPU Wave";
+
+ str += " " + hierarchy_str ();
+ str += " " + dispatch_pos_str ();
+
+ return str;
+}
+
/* Read in wave_info for WAVE_ID. */
void
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] gdb, amd-dbgapi-target: split wave_coordinates::to_string
2026-07-03 12:50 ` [PATCH 2/2] gdb, amd-dbgapi-target: split wave_coordinates::to_string Tankut Baris Aktemur
@ 2026-07-03 13:16 ` Simon Marchi
2026-07-06 10:45 ` Lancelot SIX
0 siblings, 1 reply; 8+ messages in thread
From: Simon Marchi @ 2026-07-03 13:16 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches; +Cc: lancelot.six
On 7/3/26 8:50 AM, Tankut Baris Aktemur wrote:
> wave_coordinates::to_string() produces a string in the following format:
>
> AMDGPU Wave a:q:d:w (x,y,z)/i
>
> Split the method into smaller pieces:
>
> a:q:d:w : hierarchy_str
> (x,y,z) : workgroup_coord_str
> (x,y,z)/i : dispatch_pos_str
>
> This is a refactoring to allow reusing pieces. Currently the reuse
> opportunity exists in the downstream debugger.
I think this is a nice change even for upstream, as it helps document
what the numbers mean. Assuming Lancelot is already fine with the
change:
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Just one formatting comment below.
> ---
> gdb/amd-dbgapi-target.c | 55 +++++++++++++++++++++++++++++++++--------
> 1 file changed, 45 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
> index 923b790664e..33512695fb5 100644
> --- a/gdb/amd-dbgapi-target.c
> +++ b/gdb/amd-dbgapi-target.c
> @@ -147,6 +147,17 @@ struct wave_coordinates
> : wave_id (wave_id)
> {}
>
> + /* Return the string showing the agent -> queue -> dispatch -> wave
> + hierarchy. */
> + std::string hierarchy_str () const;
> +
> + /* Return the workgroup coordinates as a string. */
> + std::string workgroup_coord_str () const;
> +
> + /* Return the dispatch position string for the wave this
> + wave_coordinates is for. */
> + std::string dispatch_pos_str () const;
> +
> /* Return the target ID string for the wave this wave_coordinates is
> for. */
> std::string to_string () const;
> @@ -387,13 +398,12 @@ get_amd_dbgapi_inferior_info (inferior *inferior)
> static async_event_handler *amd_dbgapi_async_event_handler = nullptr;
>
> std::string
> -wave_coordinates::to_string () const
> +wave_coordinates::hierarchy_str () const
> {
> - std::string str = "AMDGPU Wave";
> -
> - str += (agent_id != AMD_DBGAPI_AGENT_NONE
> - ? string_printf (" %s", pulongest (agent_id.handle))
> - : " ?");
> + std::string str
> + = (agent_id != AMD_DBGAPI_AGENT_NONE
> + ? string_printf (" %s", pulongest (agent_id.handle))
> + : "?");
>
> str += (queue_id != AMD_DBGAPI_QUEUE_NONE
> ? string_printf (":%s", pulongest (queue_id.handle))
> @@ -405,11 +415,25 @@ wave_coordinates::to_string () const
>
> str += string_printf (":%s", pulongest (wave_id.handle));
>
> - str += (group_ids[0] != UINT32_MAX
> - ? string_printf (" (%u,%u,%u)", group_ids[0], group_ids[1],
> - group_ids[2])
> - : " (?,?,?)");
> + return str;
> +}
> +
> +std::string
> +wave_coordinates::workgroup_coord_str () const
> +{
> + std::string str
> + = (group_ids[0] != UINT32_MAX
> + ? string_printf ("(%u,%u,%u)", group_ids[0], group_ids[1],
> + group_ids[2])
group_ids[2] could go on the previous line, for one less line wrap.
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] gdb, amd-dbgapi-target: split wave_coordinates::to_string
2026-07-03 13:16 ` Simon Marchi
@ 2026-07-06 10:45 ` Lancelot SIX
0 siblings, 0 replies; 8+ messages in thread
From: Lancelot SIX @ 2026-07-06 10:45 UTC (permalink / raw)
To: Simon Marchi, Tankut Baris Aktemur, gdb-patches
On 03/07/2026 14:16, Simon Marchi wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On 7/3/26 8:50 AM, Tankut Baris Aktemur wrote:
>> wave_coordinates::to_string() produces a string in the following format:
>>
>> AMDGPU Wave a:q:d:w (x,y,z)/i
>>
>> Split the method into smaller pieces:
>>
>> a:q:d:w : hierarchy_str
>> (x,y,z) : workgroup_coord_str
>> (x,y,z)/i : dispatch_pos_str
>>
>> This is a refactoring to allow reusing pieces. Currently the reuse
>> opportunity exists in the downstream debugger.
>
> I think this is a nice change even for upstream, as it helps document
> what the numbers mean. Assuming Lancelot is already fine with the
> change:
>
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
>
> Just one formatting comment below.
Hi,
Yes, I am fine with this welcome change as well.
Only comments would be linked to what V2 of patch 1 changed, %u ->
pulongest.
Other than that, this all looks good to me.
Approved-by: Lancelot Six <lancelot.six@amd.com> (amdgpu)
Best,
Lancelot.
>
>> ---
>> gdb/amd-dbgapi-target.c | 55 +++++++++++++++++++++++++++++++++--------
>> 1 file changed, 45 insertions(+), 10 deletions(-)
>>
>> diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
>> index 923b790664e..33512695fb5 100644
>> --- a/gdb/amd-dbgapi-target.c
>> +++ b/gdb/amd-dbgapi-target.c
>> @@ -147,6 +147,17 @@ struct wave_coordinates
>> : wave_id (wave_id)
>> {}
>>
>> + /* Return the string showing the agent -> queue -> dispatch -> wave
>> + hierarchy. */
>> + std::string hierarchy_str () const;
>> +
>> + /* Return the workgroup coordinates as a string. */
>> + std::string workgroup_coord_str () const;
>> +
>> + /* Return the dispatch position string for the wave this
>> + wave_coordinates is for. */
>> + std::string dispatch_pos_str () const;
>> +
>> /* Return the target ID string for the wave this wave_coordinates is
>> for. */
>> std::string to_string () const;
>> @@ -387,13 +398,12 @@ get_amd_dbgapi_inferior_info (inferior *inferior)
>> static async_event_handler *amd_dbgapi_async_event_handler = nullptr;
>>
>> std::string
>> -wave_coordinates::to_string () const
>> +wave_coordinates::hierarchy_str () const
>> {
>> - std::string str = "AMDGPU Wave";
>> -
>> - str += (agent_id != AMD_DBGAPI_AGENT_NONE
>> - ? string_printf (" %s", pulongest (agent_id.handle))
>> - : " ?");
>> + std::string str
>> + = (agent_id != AMD_DBGAPI_AGENT_NONE
>> + ? string_printf (" %s", pulongest (agent_id.handle))
>> + : "?");
>>
>> str += (queue_id != AMD_DBGAPI_QUEUE_NONE
>> ? string_printf (":%s", pulongest (queue_id.handle))
>> @@ -405,11 +415,25 @@ wave_coordinates::to_string () const
>>
>> str += string_printf (":%s", pulongest (wave_id.handle));
>>
>> - str += (group_ids[0] != UINT32_MAX
>> - ? string_printf (" (%u,%u,%u)", group_ids[0], group_ids[1],
>> - group_ids[2])
>> - : " (?,?,?)");
>> + return str;
>> +}
>> +
>> +std::string
>> +wave_coordinates::workgroup_coord_str () const
>> +{
>> + std::string str
>> + = (group_ids[0] != UINT32_MAX
>> + ? string_printf ("(%u,%u,%u)", group_ids[0], group_ids[1],
>> + group_ids[2])
>
> group_ids[2] could go on the previous line, for one less line wrap.
>
> Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] gdb, amd-dbgapi-target: use '%u' instead of '%d' in wave_coordinates::to_string
2026-07-03 12:50 [PATCH 1/2] gdb, amd-dbgapi-target: use '%u' instead of '%d' in wave_coordinates::to_string Tankut Baris Aktemur
2026-07-03 12:50 ` [PATCH 2/2] gdb, amd-dbgapi-target: split wave_coordinates::to_string Tankut Baris Aktemur
@ 2026-07-03 13:12 ` Simon Marchi
2026-07-03 13:27 ` Aktemur, Baris
2026-07-03 13:42 ` [PATCH v2 1/2] gdb, amd-dbgapi-target: use pulongest more Tankut Baris Aktemur
2 siblings, 1 reply; 8+ messages in thread
From: Simon Marchi @ 2026-07-03 13:12 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches; +Cc: lancelot.six
On 7/3/26 8:50 AM, Tankut Baris Aktemur wrote:
> The group id coordinates and the wave number in workgroup that are
> being formatted to a string in wave_coordinates::to_string are
> unsigned values. Use '%u' instead of '%d'.
Do we get a compiler warning for this? Or do they warn only if there is
a size mismatch, not a sign mismatch?
> ---
> gdb/amd-dbgapi-target.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
> index caae9a98501..923b790664e 100644
> --- a/gdb/amd-dbgapi-target.c
> +++ b/gdb/amd-dbgapi-target.c
> @@ -406,12 +406,12 @@ wave_coordinates::to_string () const
> str += string_printf (":%s", pulongest (wave_id.handle));
>
> str += (group_ids[0] != UINT32_MAX
> - ? string_printf (" (%d,%d,%d)", group_ids[0], group_ids[1],
> + ? string_printf (" (%u,%u,%u)", group_ids[0], group_ids[1],
> group_ids[2])
> : " (?,?,?)");
>
> str += (wave_in_group != UINT32_MAX
> - ? string_printf ("/%d", wave_in_group)
> + ? string_printf ("/%u", wave_in_group)
> : "/?");
All these are std::uint32_t, so we should probably use PRIu32 (or
pulongest).
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread* RE: [PATCH 1/2] gdb, amd-dbgapi-target: use '%u' instead of '%d' in wave_coordinates::to_string
2026-07-03 13:12 ` [PATCH 1/2] gdb, amd-dbgapi-target: use '%u' instead of '%d' in wave_coordinates::to_string Simon Marchi
@ 2026-07-03 13:27 ` Aktemur, Baris
0 siblings, 0 replies; 8+ messages in thread
From: Aktemur, Baris @ 2026-07-03 13:27 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Six, Lancelot
AMD General
On Friday, July 3, 2026 3:13 PM, Simon Marchi wrote:
> On 7/3/26 8:50 AM, Tankut Baris Aktemur wrote:
> > The group id coordinates and the wave number in workgroup that are
> > being formatted to a string in wave_coordinates::to_string are
> > unsigned values. Use '%u' instead of '%d'.
>
> Do we get a compiler warning for this? Or do they warn only if there is
> a size mismatch, not a sign mismatch?
As far as I know, no warning.
> > ---
> > gdb/amd-dbgapi-target.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
> > index caae9a98501..923b790664e 100644
> > --- a/gdb/amd-dbgapi-target.c
> > +++ b/gdb/amd-dbgapi-target.c
> > @@ -406,12 +406,12 @@ wave_coordinates::to_string () const
> > str += string_printf (":%s", pulongest (wave_id.handle));
> >
> > str += (group_ids[0] != UINT32_MAX
> > - ? string_printf (" (%d,%d,%d)", group_ids[0], group_ids[1],
> > + ? string_printf (" (%u,%u,%u)", group_ids[0], group_ids[1],
> > group_ids[2])
> > : " (?,?,?)");
> >
> > str += (wave_in_group != UINT32_MAX
> > - ? string_printf ("/%d", wave_in_group)
> > + ? string_printf ("/%u", wave_in_group)
> > : "/?");
>
> All these are std::uint32_t, so we should probably use PRIu32 (or
> pulongest).
I'll send an update with pulongest.
-Baris
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] gdb, amd-dbgapi-target: use pulongest more
2026-07-03 12:50 [PATCH 1/2] gdb, amd-dbgapi-target: use '%u' instead of '%d' in wave_coordinates::to_string Tankut Baris Aktemur
2026-07-03 12:50 ` [PATCH 2/2] gdb, amd-dbgapi-target: split wave_coordinates::to_string Tankut Baris Aktemur
2026-07-03 13:12 ` [PATCH 1/2] gdb, amd-dbgapi-target: use '%u' instead of '%d' in wave_coordinates::to_string Simon Marchi
@ 2026-07-03 13:42 ` Tankut Baris Aktemur
2026-07-06 10:31 ` Lancelot SIX
2 siblings, 1 reply; 8+ messages in thread
From: Tankut Baris Aktemur @ 2026-07-03 13:42 UTC (permalink / raw)
To: gdb-patches; +Cc: lancelot.six, simark
The group id coordinates and the wave number in workgroup that are
being formatted to a string in wave_coordinates::to_string are
uint32_t. Use pulongest to print them.
---
gdb/amd-dbgapi-target.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
index caae9a98501..6f71f5ff5d0 100644
--- a/gdb/amd-dbgapi-target.c
+++ b/gdb/amd-dbgapi-target.c
@@ -406,12 +406,13 @@ wave_coordinates::to_string () const
str += string_printf (":%s", pulongest (wave_id.handle));
str += (group_ids[0] != UINT32_MAX
- ? string_printf (" (%d,%d,%d)", group_ids[0], group_ids[1],
- group_ids[2])
+ ? string_printf (" (%s,%s,%s)", pulongest (group_ids[0]),
+ pulongest (group_ids[1]),
+ pulongest (group_ids[2]))
: " (?,?,?)");
str += (wave_in_group != UINT32_MAX
- ? string_printf ("/%d", wave_in_group)
+ ? string_printf ("/%s", pulongest (wave_in_group))
: "/?");
return str;
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/2] gdb, amd-dbgapi-target: use pulongest more
2026-07-03 13:42 ` [PATCH v2 1/2] gdb, amd-dbgapi-target: use pulongest more Tankut Baris Aktemur
@ 2026-07-06 10:31 ` Lancelot SIX
0 siblings, 0 replies; 8+ messages in thread
From: Lancelot SIX @ 2026-07-06 10:31 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches; +Cc: simark
On 03/07/2026 14:42, Tankut Baris Aktemur wrote:
> The group id coordinates and the wave number in workgroup that are
> being formatted to a string in wave_coordinates::to_string are
> uint32_t. Use pulongest to print them.
Hi,
this looks good to me, thanks.
Approved-by: Lancelot Six <lancelot.six@amd.com> (amdgpu)
Best,
Lancelot.
> ---
> gdb/amd-dbgapi-target.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
> index caae9a98501..6f71f5ff5d0 100644
> --- a/gdb/amd-dbgapi-target.c
> +++ b/gdb/amd-dbgapi-target.c
> @@ -406,12 +406,13 @@ wave_coordinates::to_string () const
> str += string_printf (":%s", pulongest (wave_id.handle));
>
> str += (group_ids[0] != UINT32_MAX
> - ? string_printf (" (%d,%d,%d)", group_ids[0], group_ids[1],
> - group_ids[2])
> + ? string_printf (" (%s,%s,%s)", pulongest (group_ids[0]),
> + pulongest (group_ids[1]),
> + pulongest (group_ids[2]))
> : " (?,?,?)");
>
> str += (wave_in_group != UINT32_MAX
> - ? string_printf ("/%d", wave_in_group)
> + ? string_printf ("/%s", pulongest (wave_in_group))
> : "/?");
>
> return str;
^ permalink raw reply [flat|nested] 8+ messages in thread