gdb, amdgpu: pass process id to amd_dbgapi_convert_address_space

[aktemur]

Starting with version 0.81, dbgapi expects a process id as the first argument to amd_dbgapi_convert_address_space.

[aktemur]

The corresponding dbgapi PR is ROCm/rocm-systems#10163.

[lancesix]

The issue is that we have quite a few 0.81 PR that are lined up.

Could we have some more detection of the actual change, to not commit in GDB a particular ordering in which dbgapi changes need to go in?

Something like this should do:

diff --git a/gdb/amdgpu-tdep.c b/gdb/amdgpu-tdep.c
index cfeddde7c4d..0c2620d5c39 100644
--- a/gdb/amdgpu-tdep.c
+++ b/gdb/amdgpu-tdep.c
@@ -1900,7 +1900,11 @@ amdgpu_get_watchable_aliases (struct gdbarch *gdbarch,
       /* Try to convert the address to an address in the
         default address space.  */
       if (amd_dbgapi_convert_address_space
-           (wave_id, simd_lane, dbgapi_from_addr_space_id,
+           (
+#ifdef AMD_DBGAPI_CONVERT_ASPACE_NEED_PROCESS_ID
+            get_amd_dbgapi_process_id (current_inferior ()),
+#endif
+            wave_id, simd_lane, dbgapi_from_addr_space_id,
             (amd_dbgapi_segment_address_t) addr,
             dbgapi_to_addr_space_id, &to_offset, &converted_size)
            != AMD_DBGAPI_STATUS_SUCCESS)
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -354,6 +354,34 @@ if test "$gdb_require_amd_dbgapi" = true \
        AC_MSG_ERROR([amd-dbgapi not supported for host ${gdb_host}])
        ;;
     esac
+
+    # Handle feature discovery for dbgapi, to avoid having to assume the order
+    # in which various features will land.
+    save_CPPFLAGS="$CPPFLAGS"
+    CPPFLAGS="$CPPFLAGS $AMD_DBGAPI_CFLAGS"
+    AC_MSG_CHECKING([whether amd_dbgapi_convert_address_space takes a process id])
+    AC_COMPILE_IFELSE(
+      [AC_LANG_PROGRAM(
+       [[#include <amd-dbgapi/amd-dbgapi.h>]],
+       [[amd_dbgapi_process_id_t p;
+         amd_dbgapi_wave_id_t w;
+         amd_dbgapi_lane_id_t l;
+         amd_dbgapi_address_space_id_t s, d;
+         amd_dbgapi_convert_address_space (p, w, l, s, 0, d, NULL, NULL);]]
+      )],
+      [dbgapi_convert_aspace_needs_process_id=yes
+       AC_MSG_RESULT([yes])],
+      [dbgapi_convert_aspace_needs_process_id=no
+       AC_MSG_RESULT([no])]
+    )
+    CPPFLAGS="$save_CPPFLAGS"
+
+    if test "x${dbgapi_convert_aspace_needs_process_id}" = "xyes"; then
+      AC_DEFINE(AMD_DBGAPI_CONVERT_ASPACE_NEED_PROCESS_ID, 1,
+               [Define to 1 if amd_dbgapi_convert_address_space needs a process_id])
+    fi
+

I left the autoconf output out of the diff for readibility.

[aktemur]

The issue is that we have quite a few 0.81 PR that are lined up.

Could we have some more detection of the actual change, to not commit in GDB a particular ordering in which dbgapi changes need to go in?

Isn't this an overkill? Once both GDB and dbgapi changes are approved, the submitter can coordinate the merges. The autoconf changes will be reverted right away with the third merge (1. GDB initial, 2. dbgapi, 3. GDB actual) One PR will win the next version number and the others will have to update (it will trigger a rebase conflict anyway).

[aktemur]

Could we have some more detection of the actual change, to not commit in GDB a particular ordering in which dbgapi changes need to go in?

I updated the PR with this.

[lancesix]

LGTM, thanks.