[PATCH v2 0/5] Fix gdb.rocm/simple.exp on hosts without ROCm

[PATCH v2 0/5] Fix gdb.rocm/simple.exp on hosts without ROCm

[Lancelot SIX]

Hi, this is a V2 for https://sourceware.org/pipermail/gdb-patches/2023-February/196653.html.

Changes since V1:

- Fixed a typo in patch 1
- Removed the un-necessary "expr" call in patch 3
- Added patch 5 so both hipcc and rocm_agent_enumerator are searched in
  a consistent way

Best,
Lancelot.

Hi,

Tom De Vries reported that the gdb.rocm/simple.exp test (recently introduced
with the AMDGPU support) can fails[1].  I can reproduce this problem
(and variations of it) on systems where GDB is not build with the AMDGPU
support, or which do not have the ROCm stack installed.

This series fixes this test failure by only running the test if:
- GDB is build with AMDGPU support (patch 1 and 3)
- if the hipcc compiler is installed and can compile a simple HIP
  program which offloads a task to an AMDGPU device (patch 4).

Patch 2 is a small refactoring to use "require" in gdb.rocm/*.exp.

Patch 5 ensures that both hipcc (the hip compiler) and
rocm_agent_enumerator (the tool to list AMDGPU devices) are searched in
a consistent way.

All feedbacks are welcome.

Best,
Lancelot.

[1] https://sourceware.org/pipermail/gdb-patches/2023-February/196624.html

Lancelot SIX (5):
  gdb: 'show config' shows --with[out]-amd-dbgapi
  gdb/testsuite: Rename skip_hipcc_tests to allow_hipcc_tests
  gdb/testsuite: require amd-dbgapi support to run rocm tests
  gdb/testsuite: allow_hipcc_tests tests the hipcc compiler
  gdb/testsuite: look for hipcc in env(ROCM_PATH)

 gdb/config.in                     |  3 ++
 gdb/configure                     |  3 ++
 gdb/configure.ac                  |  1 +
 gdb/testsuite/gdb.rocm/simple.exp |  5 +-
 gdb/testsuite/lib/future.exp      |  7 ++-
 gdb/testsuite/lib/gdb.exp         |  4 ++
 gdb/testsuite/lib/rocm.exp        | 80 +++++++++++++++++++++++++++++--
 gdb/top.c                         | 10 ++++
 8 files changed, 104 insertions(+), 9 deletions(-)


base-commit: c920e5cc604c5b20f9af7c75402eea94aa1e11c6
-- 
2.34.1

[PATCH v2 1/5] gdb: 'show config' shows --with[out]-amd-dbgapi

[Lancelot SIX]

Ensure that the "show configuration" command and the "--configuration"
command line switch shows if GDB was built with the AMDGPU support or
not.

This will be used in a later patch in this series.
---
 gdb/config.in    |  3 +++
 gdb/configure    |  3 +++
 gdb/configure.ac |  1 +
 gdb/top.c        | 10 ++++++++++
 4 files changed, 17 insertions(+)

diff --git a/gdb/config.in b/gdb/config.in
index 7da131ebf04..a6027847444 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -84,6 +84,9 @@
    */
 #undef HAVE_ALLOCA_H
 
+/* Define if amd-dbgapi is being linked in. */
+#undef HAVE_AMD_DBGAPI
+
 /* Define to 1 if you have the `btowc' function. */
 #undef HAVE_BTOWC
 
diff --git a/gdb/configure b/gdb/configure
index 113b7cf8a30..8b2039912e7 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -18252,6 +18252,9 @@ $as_echo "yes" >&6; }
 fi
 
   if test "$has_amd_dbgapi" = "yes"; then
+
+$as_echo "#define HAVE_AMD_DBGAPI 1" >>confdefs.h
+
     TARGET_OBS="$TARGET_OBS amd-dbgapi-target.o"
 
     # If --enable-targets=all was provided, use the list of all files depending
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 7c7bf88b3fb..79eb013ce19 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -275,6 +275,7 @@ if test "$gdb_require_amd_dbgapi" = true \
 		    [has_amd_dbgapi=yes], [has_amd_dbgapi=no])
 
   if test "$has_amd_dbgapi" = "yes"; then
+    AC_DEFINE(HAVE_AMD_DBGAPI, 1, [Define if amd-dbgapi is being linked in.])
     TARGET_OBS="$TARGET_OBS amd-dbgapi-target.o"
 
     # If --enable-targets=all was provided, use the list of all files depending
diff --git a/gdb/top.c b/gdb/top.c
index 205eb360ba3..1b189d7c5ab 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1629,6 +1629,16 @@ This GDB was configured as follows:\n\
 "));
 #endif
 
+#if HAVE_AMD_DBGAPI
+  gdb_printf (stream, _("\
+	     --with-amd-dbgapi\n\
+"));
+#else
+  gdb_printf (stream, _("\
+	     --without-amd-dbgapi\n\
+"));
+#endif
+
 #if HAVE_SOURCE_HIGHLIGHT
   gdb_printf (stream, _("\
 	     --enable-source-highlight\n\
-- 
2.34.1

[PATCH v2 2/5] gdb/testsuite: Rename skip_hipcc_tests to allow_hipcc_tests

[Lancelot SIX]

Rename skip_hipcc_tests to allow_hipcc_tests so it can be used as a
"require" predicate in tests.

Use require in gdb.rocm/simple.exp.
---
 gdb/testsuite/gdb.rocm/simple.exp | 5 +----
 gdb/testsuite/lib/rocm.exp        | 6 +++---
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.rocm/simple.exp b/gdb/testsuite/gdb.rocm/simple.exp
index f84df71414e..befcc7aaabc 100644
--- a/gdb/testsuite/gdb.rocm/simple.exp
+++ b/gdb/testsuite/gdb.rocm/simple.exp
@@ -20,10 +20,7 @@ load_lib rocm.exp
 
 standard_testfile .cpp
 
-if [skip_hipcc_tests] {
-    verbose "skipping hip test: ${testfile}"
-    return
-}
+require allow_hipcc_tests
 
 if {[build_executable "failed to prepare" $testfile $srcfile {debug hip}]} {
     return
diff --git a/gdb/testsuite/lib/rocm.exp b/gdb/testsuite/lib/rocm.exp
index e22f392deb1..1440ac85d32 100644
--- a/gdb/testsuite/lib/rocm.exp
+++ b/gdb/testsuite/lib/rocm.exp
@@ -15,14 +15,14 @@
 #
 # Support library for testing ROCm (AMD GPU) GDB features.
 
-proc skip_hipcc_tests { } {
+proc allow_hipcc_tests { } {
     # Only the native target supports ROCm debugging.  E.g., when
     # testing against GDBserver, there's no point in running the ROCm
     # tests.
     if {[target_info gdb_protocol] != ""} {
-        return 1
+	return 0
     }
-    return 0
+    return 1
 }
 
 # The lock file used to ensure that only one GDB has access to the GPU
-- 
2.34.1

[PATCH v2 3/5] gdb/testsuite: require amd-dbgapi support to run rocm tests

[Lancelot SIX]

Update allow_hipcc_tests to check that GDB has the amd-dbgapi support
built-in.  Without this support, all tests using hipcc and the rocm
stack will fail.
---
 gdb/testsuite/lib/rocm.exp | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gdb/testsuite/lib/rocm.exp b/gdb/testsuite/lib/rocm.exp
index 1440ac85d32..a78b9f63353 100644
--- a/gdb/testsuite/lib/rocm.exp
+++ b/gdb/testsuite/lib/rocm.exp
@@ -22,6 +22,13 @@ proc allow_hipcc_tests { } {
     if {[target_info gdb_protocol] != ""} {
 	return 0
     }
+
+    # Ensure that GDB is built with amd-dbgapi support.
+    set output [remote_exec host $::GDB "$::INTERNAL_GDBFLAGS --configuration"]
+    if { [string first "--with-amd-dbgapi" $output] == -1 } {
+	return 0
+    }
+
     return 1
 }
 
-- 
2.34.1

[PATCH v2 4/5] gdb/testsuite: allow_hipcc_tests tests the hipcc compiler

[Lancelot SIX]

Update allow_hipcc_tests so all gdb.rocm tests are skipped if we do not
have a working hipcc compiler available.

To achieve this, adjust gdb_simple_compile to ensure that the hip
program is saved in a ".cpp" file before calling hipcc otherwise
compilation will fail.

One thing to note is that it is possible to have a hipcc installed with
a CUDA backend.  Compiling with this back-end will successfully result
in an application, but GDB cannot debug it (at least for the offload
part). In the context of the gdb.rocm tests, we want to detect such
situation where gdb_simple_compile would give a false positive.

To achieve this, this patch checks that there is at least one AMDGPU
device available and that hipcc can compile for this or those targets.
Detecting the device is done using the rocm_agent_enumerator tool which
is installed with the all ROCm installations (it is used by hipcc to
detect identify targets if this is not specified on the comand line).

This patch also makes the allow_hipcc_tests proc a cached proc.
---
 gdb/testsuite/lib/gdb.exp  |  4 +++
 gdb/testsuite/lib/rocm.exp | 69 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index faa0ac05a9a..6333728f71e 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4581,6 +4581,10 @@ proc gdb_simple_compile {name code {type object} {compile_flags {}} {object obj}
 	    set ext "go"
 	    break
 	}
+	if { "$flag" eq "hip" } {
+	    set ext "cpp"
+	    break
+	}
     }
     set src [standard_temp_file $name-[pid].$ext]
     set obj [standard_temp_file $name-[pid].$postfix]
diff --git a/gdb/testsuite/lib/rocm.exp b/gdb/testsuite/lib/rocm.exp
index a78b9f63353..125fa000170 100644
--- a/gdb/testsuite/lib/rocm.exp
+++ b/gdb/testsuite/lib/rocm.exp
@@ -15,7 +15,51 @@
 #
 # Support library for testing ROCm (AMD GPU) GDB features.
 
-proc allow_hipcc_tests { } {
+# Get the list of gpu targets to compile for.
+#
+# If HCC_AMDGPU_TARGET is set in the environment, use it.  Otherwise,
+# try reading it from the system using the rocm_agent_enumerator
+# utility.
+
+proc hcc_amdgpu_targets {} {
+    # Look for HCC_AMDGPU_TARGET (same env var hipcc uses).  If
+    # that fails, try using rocm_agent_enumerator (again, same as
+    # hipcc does).
+    if {[info exists ::env(HCC_AMDGPU_TARGET)]} {
+	return [split $::env(HCC_AMDGPU_TARGET) ","]
+    }
+
+    set rocm_agent_enumerator "rocm_agent_enumerator"
+
+    # If available, use ROCM_PATH to locate rocm_agent_enumerator.
+    if { [info exists ::env(ROCM_PATH)] } {
+	set rocm_agent_enumerator \
+	    "$::env(ROCM_PATH)/bin/rocm_agent_enumerator"
+    }
+
+    # If we fail to locate the rocm_agent_enumerator, just return an empty
+    # list of targets and let the caller decide if this should be an error.
+    if { [which $rocm_agent_enumerator] == 0 } {
+	return [list]
+    }
+
+    set result [remote_exec host $rocm_agent_enumerator]
+    if { [lindex $result 0] != 0 } {
+	error "rocm_agent_enumerator failed"
+    }
+
+    set targets [list]
+    foreach target [lindex $result 1] {
+	# Ignore gfx000 which is the host CPU.
+	if { $target ne "gfx000" } {
+	    lappend targets $target
+	}
+    }
+
+    return $targets
+}
+
+gdb_caching_proc allow_hipcc_tests {
     # Only the native target supports ROCm debugging.  E.g., when
     # testing against GDBserver, there's no point in running the ROCm
     # tests.
@@ -29,6 +73,29 @@ proc allow_hipcc_tests { } {
 	return 0
     }
 
+    # Check we have a working hipcc compiler available.
+    set targets [hcc_amdgpu_targets]
+    if { [llength $targets] == 0} {
+	return 0
+    }
+
+    set flags [list hip additional_flags=--offload-arch=[join $targets ","]]
+    if {![gdb_simple_compile hipprobe {
+	    #include <hip/hip_runtime.h>
+	    __global__ void
+	    kern () {}
+
+	    int
+	    main ()
+	    {
+		kern<<<1, 1>>> ();
+		hipDeviceSynchronize ();
+		return 0;
+	    }
+	} executable $flags]} {
+	return 0
+    }
+
     return 1
 }
 
-- 
2.34.1

[PATCH v2 5/5] gdb/testsuite: look for hipcc in env(ROCM_PATH)

[Lancelot SIX]

If the hipcc compiler cannot be found in dejagnu's tool_root_dir, look
for it in $::env(ROCM_PATH) (if set).  If hipcc is still not found,
fallback to "hipcc" so the compiler will be searched in the PATH.  This
removes the fallback to the hard-coded "/opt/rocm/bin" prefix.

This change is done so ROCM tools are searched in a uniform manner.
---
 gdb/testsuite/lib/future.exp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/lib/future.exp b/gdb/testsuite/lib/future.exp
index 5720d3837d5..fa839fcd12b 100644
--- a/gdb/testsuite/lib/future.exp
+++ b/gdb/testsuite/lib/future.exp
@@ -125,8 +125,11 @@ proc gdb_find_hipcc {} {
     global tool_root_dir
     if {![is_remote host]} {
 	set hipcc [lookfor_file $tool_root_dir hipcc]
-	if {$hipcc == ""} {
-	    set hipcc [lookfor_file /opt/rocm/bin hipcc]
+	if {$hipcc eq "" && [info exists ::env(ROCM_PATH)]} {
+	    set hipcc [lookfor_file $::env(ROCM_PATH)/bin hipcc]
+	}
+	if {$hipcc eq ""} {
+	    set hipcc hipcc
 	}
     } else {
 	set hipcc ""
-- 
2.34.1

Re: [PATCH v2 0/5] Fix gdb.rocm/simple.exp on hosts without ROCm

[Simon Marchi]

On 2/9/23 06:26, Lancelot SIX via Gdb-patches wrote:
> Hi, this is a V2 for https://sourceware.org/pipermail/gdb-patches/2023-February/196653.html.
> 
> Changes since V1:
> 
> - Fixed a typo in patch 1
> - Removed the un-necessary "expr" call in patch 3
> - Added patch 5 so both hipcc and rocm_agent_enumerator are searched in
>   a consistent way
> 
> Best,
> Lancelot.

Thanks, that all LGTM:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon

Re: [PATCH v2 0/5] Fix gdb.rocm/simple.exp on hosts without ROCm

[Lancelot SIX]

> Thanks, that all LGTM:
> 
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
> 
> Simon

Thanks,

I just pushed those 5 patches.

Best,
Lancelot.