[1/3] gdb/amd-dbgapi: use gdb::unordered_map

Message ID 20250114191818.2393132-2-simon.marchi@polymtl.ca
State New
Headers
Series Add displaced stepping support for AMD GPUs |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed

Commit Message

Simon Marchi Jan. 14, 2025, 7:16 p.m. UTC
  From: Simon Marchi <simon.marchi@efficios.com>

Since we have gdb::unordered_map, swap std::unordered_map for that.

Change-Id: If2ef652fe18c1a440a25cff6131d29e37091bdbe
---
 gdb/amd-dbgapi-target.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

Lancelot SIX Jan. 15, 2025, 10:55 a.m. UTC | #1
On 14/01/2025 19:16, simon.marchi@polymtl.ca wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> From: Simon Marchi <simon.marchi@efficios.com>
> 
> Since we have gdb::unordered_map, swap std::unordered_map for that.
> 

Hi,

That looks good to me, thanks.

Since it only touches the amdgpu backend,
Approved-By: Lancelot Six <lancelot.six@amd.com> (amdgpu)

Best,
Lancelot.

> Change-Id: If2ef652fe18c1a440a25cff6131d29e37091bdbe
> ---
>   gdb/amd-dbgapi-target.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
> index 2bb79acd76f0..153a35f788ed 100644
> --- a/gdb/amd-dbgapi-target.c
> +++ b/gdb/amd-dbgapi-target.c
> @@ -24,6 +24,7 @@
>   #include "cli/cli-cmds.h"
>   #include "cli/cli-decode.h"
>   #include "cli/cli-style.h"
> +#include "gdbsupport/unordered_map.h"
>   #include "inf-loop.h"
>   #include "inferior.h"
>   #include "objfiles.h"
> @@ -207,7 +208,7 @@ struct amd_dbgapi_inferior_info
>       bool enabled = false;
>     } precise_memory;
> 
> -  std::unordered_map<decltype (amd_dbgapi_breakpoint_id_t::handle),
> +  gdb::unordered_map<decltype (amd_dbgapi_breakpoint_id_t::handle),
>                       struct breakpoint *>
>       breakpoint_map;
> 
> @@ -221,7 +222,7 @@ struct amd_dbgapi_inferior_info
> 
>        wave_info objects are added when we first see the wave, and
>        removed from a thread_deleted observer.  */
> -  std::unordered_map<decltype (amd_dbgapi_wave_id_t::handle), wave_info>
> +  gdb::unordered_map<decltype (amd_dbgapi_wave_id_t::handle), wave_info>
>       wave_info_map;
>   };
> 
> --
> 2.47.1
>
  
Simon Marchi Jan. 15, 2025, 6:41 p.m. UTC | #2
On 2025-01-15 05:55, Lancelot SIX wrote:
> 
> 
> On 14/01/2025 19:16, simon.marchi@polymtl.ca wrote:
>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>>
>>
>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>> Since we have gdb::unordered_map, swap std::unordered_map for that.
>>
> 
> Hi,
> 
> That looks good to me, thanks.
> 
> Since it only touches the amdgpu backend,
> Approved-By: Lancelot Six <lancelot.six@amd.com> (amdgpu)

Thanks, added.

Is the (amdgpu) some format we use?  I haven't seen it used anywhere
else.  I don't mind, for instance, Eli could use Approved-By with (doc).

Simon
  
Simon Marchi Jan. 15, 2025, 6:46 p.m. UTC | #3
On 2025-01-15 13:41, Simon Marchi wrote:
> 
> 
> On 2025-01-15 05:55, Lancelot SIX wrote:
>>
>>
>> On 14/01/2025 19:16, simon.marchi@polymtl.ca wrote:
>>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>>>
>>>
>>> From: Simon Marchi <simon.marchi@efficios.com>
>>>
>>> Since we have gdb::unordered_map, swap std::unordered_map for that.
>>>
>>
>> Hi,
>>
>> That looks good to me, thanks.
>>
>> Since it only touches the amdgpu backend,
>> Approved-By: Lancelot Six <lancelot.six@amd.com> (amdgpu)
> 
> Thanks, added.
> 
> Is the (amdgpu) some format we use?  I haven't seen it used anywhere
> else.  I don't mind, for instance, Eli could use Approved-By with (doc).

Just note that `b4 am` doesn't preserve the parenthesis, so it's not so
handy actually.

Simon
  
Guinevere Larsen Jan. 15, 2025, 6:50 p.m. UTC | #4
On 1/15/25 3:46 PM, Simon Marchi wrote:
>
> On 2025-01-15 13:41, Simon Marchi wrote:
>>
>> On 2025-01-15 05:55, Lancelot SIX wrote:
>>>
>>> On 14/01/2025 19:16, simon.marchi@polymtl.ca wrote:
>>>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>>>>
>>>>
>>>> From: Simon Marchi <simon.marchi@efficios.com>
>>>>
>>>> Since we have gdb::unordered_map, swap std::unordered_map for that.
>>>>
>>> Hi,
>>>
>>> That looks good to me, thanks.
>>>
>>> Since it only touches the amdgpu backend,
>>> Approved-By: Lancelot Six <lancelot.six@amd.com> (amdgpu)
>> Thanks, added.
>>
>> Is the (amdgpu) some format we use?  I haven't seen it used anywhere
>> else.  I don't mind, for instance, Eli could use Approved-By with (doc).

It was the thing we agreed on when I added the trailer information to 
the maintainers file. The examples specifically use a comma separated 
list of areas in parenthesis after the tag.

> Just note that `b4 am` doesn't preserve the parenthesis, so it's not so
> handy actually.
>
> Simon
>
Ah, that's unfortunate. We might want to update that format then, but we 
should also update the maintainers file
  
Lancelot SIX Jan. 15, 2025, 6:55 p.m. UTC | #5
On 15/01/2025 18:41, Simon Marchi wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On 2025-01-15 05:55, Lancelot SIX wrote:
>>
>>
>> On 14/01/2025 19:16, simon.marchi@polymtl.ca wrote:
>>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>>>
>>>
>>> From: Simon Marchi <simon.marchi@efficios.com>
>>>
>>> Since we have gdb::unordered_map, swap std::unordered_map for that.
>>>
>>
>> Hi,
>>
>> That looks good to me, thanks.
>>
>> Since it only touches the amdgpu backend,
>> Approved-By: Lancelot Six <lancelot.six@amd.com> (amdgpu)
> 
> Thanks, added.
> 
> Is the (amdgpu) some format we use?  I haven't seen it used anywhere
> else.  I don't mind, for instance, Eli could use Approved-By with (doc).
> 
> Simon

This was discussed when introducing the trailer tags for reviews, and is 
documented in the gdb/MAINTAINERS file (thanks Gwen for that).

   - Approved-By:

    […]
    This trailer can be specific to one or more areas of the project, as
    defined by the "Responsible maintainers" section of this file.  If
    that is the case, the area(s) should be added at the end of the tag
    in parenthesis in a comma separated list.

There are a couple of other instances of that already:

     $ git log origin/master |grep -i Approved-by: | sort |uniq|grep \(
       Approved-By: G... L... <...> (record-full)
       Approved-By: L... S... <...> (amdgpu)
       Approved-By: L... M... <...> (aarch64/arm)

Best,
Lancelot.
  

Patch

diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
index 2bb79acd76f0..153a35f788ed 100644
--- a/gdb/amd-dbgapi-target.c
+++ b/gdb/amd-dbgapi-target.c
@@ -24,6 +24,7 @@ 
 #include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
 #include "cli/cli-style.h"
+#include "gdbsupport/unordered_map.h"
 #include "inf-loop.h"
 #include "inferior.h"
 #include "objfiles.h"
@@ -207,7 +208,7 @@  struct amd_dbgapi_inferior_info
     bool enabled = false;
   } precise_memory;
 
-  std::unordered_map<decltype (amd_dbgapi_breakpoint_id_t::handle),
+  gdb::unordered_map<decltype (amd_dbgapi_breakpoint_id_t::handle),
 		     struct breakpoint *>
     breakpoint_map;
 
@@ -221,7 +222,7 @@  struct amd_dbgapi_inferior_info
 
      wave_info objects are added when we first see the wave, and
      removed from a thread_deleted observer.  */
-  std::unordered_map<decltype (amd_dbgapi_wave_id_t::handle), wave_info>
+  gdb::unordered_map<decltype (amd_dbgapi_wave_id_t::handle), wave_info>
     wave_info_map;
 };