diff mbox series

[v2] gdb/testsuite/gdb.rocm: Check value returned by hipDeviceSynchronize

Message ID 20231017154545.312687-1-lancelot.six@amd.com
State New
Headers
Series [v2] gdb/testsuite/gdb.rocm: Check value returned by hipDeviceSynchronize |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 warning Patch is already merged

Commit Message

Lancelot SIX Oct. 17, 2023, 3:45 p.m. UTC 
  Functions of the hip runtime returning a hipError_t can be marked
nodiscard depending on the configuration[1] (when compiled with C++17).

This patch makes sure that we always check value returned by
hipDeviceSynchronize and friends, and print an error message when appropriate.
This avoid a wall of warnings when running the testsuite if the compiler
defaults to using C++17.

It is always a good practice to check the return values anyway.

[1] https://github.com/ROCm-Developer-Tools/HIP/blob/docs/5.7.1/include/hip/hip_runtime_api.h#L203-L218

Change-Id: I2a819a8ac45f4bcf814efe9a2ff12c6a7ad22f97
---
Changes since V1:
- Improved the commit message,
- Check the return value in gdb/testsuite/lib/rocm.exp
  (allow_hipcc_tests) as well.

---
 .../gdb.rocm/fork-exec-gpu-to-non-gpu-execer.cpp   | 13 ++++++++++++-
 .../gdb.rocm/fork-exec-non-gpu-to-gpu-execee.cpp   | 14 +++++++++++++-
 gdb/testsuite/gdb.rocm/multi-inferior-gpu.cpp      |  2 +-
 .../gdb.rocm/precise-memory-warning-sigsegv.cpp    | 13 ++++++++++++-
 gdb/testsuite/gdb.rocm/precise-memory.cpp          | 13 ++++++++++++-
 gdb/testsuite/lib/rocm.exp                         |  3 ++-
 6 files changed, 52 insertions(+), 6 deletions(-)

Comments

Simon Marchi Oct. 17, 2023, 7:56 p.m. UTC | #1 
On 10/17/23 11:45, Lancelot Six wrote:
> Functions of the hip runtime returning a hipError_t can be marked
> nodiscard depending on the configuration[1] (when compiled with C++17).
> 
> This patch makes sure that we always check value returned by

"check the value"

Otherwise:

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

Simon
Lancelot SIX Oct. 18, 2023, 8:30 a.m. UTC | #2 
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
> 
> Simon

Thanks,

I have just pushed this patch.

Best,
Lancelot.
diff mbox series

Patch

diff --git a/gdb/testsuite/gdb.rocm/fork-exec-gpu-to-non-gpu-execer.cpp b/gdb/testsuite/gdb.rocm/fork-exec-gpu-to-non-gpu-execer.cpp
index 1a731aeca51..e42c8f10bb3 100644
--- a/gdb/testsuite/gdb.rocm/fork-exec-gpu-to-non-gpu-execer.cpp
+++ b/gdb/testsuite/gdb.rocm/fork-exec-gpu-to-non-gpu-execer.cpp
@@ -18,6 +18,17 @@ 
 #include <hip/hip_runtime.h>
 #include <unistd.h>
 
+#define CHECK(cmd)                                                           \
+  {                                                                          \
+    hipError_t error = cmd;                                                  \
+    if (error != hipSuccess)                                                 \
+      {                                                                      \
+	fprintf (stderr, "error: '%s'(%d) at %s:%d\n",                       \
+		 hipGetErrorString (error), error, __FILE__, __LINE__);      \
+	exit (EXIT_FAILURE);                                                 \
+      }                                                                      \
+  }
+
 __global__ static void
 kernel1 ()
 {}
@@ -50,6 +61,6 @@  main ()
 
   kernel2<<<1, 1>>> ();
 
-  hipDeviceSynchronize ();
+  CHECK (hipDeviceSynchronize ());
   return 0;
 }
diff --git a/gdb/testsuite/gdb.rocm/fork-exec-non-gpu-to-gpu-execee.cpp b/gdb/testsuite/gdb.rocm/fork-exec-non-gpu-to-gpu-execee.cpp
index 2de8fe20a0d..cffeedb7896 100644
--- a/gdb/testsuite/gdb.rocm/fork-exec-non-gpu-to-gpu-execee.cpp
+++ b/gdb/testsuite/gdb.rocm/fork-exec-non-gpu-to-gpu-execee.cpp
@@ -17,6 +17,18 @@ 
 
 #include <hip/hip_runtime.h>
 
+#define CHECK(cmd)                                                           \
+  {                                                                          \
+    hipError_t error = cmd;                                                  \
+    if (error != hipSuccess)                                                 \
+      {                                                                      \
+	fprintf (stderr, "error: '%s'(%d) at %s:%d\n",                       \
+		 hipGetErrorString (error), error, __FILE__, __LINE__);      \
+	exit (EXIT_FAILURE);                                                 \
+      }                                                                      \
+  }
+
+
 __device__ static void
 break_here_execee ()
 {}
@@ -31,6 +43,6 @@  int
 main ()
 {
   kernel<<<1, 1>>> ();
-  hipDeviceSynchronize ();
+  CHECK (hipDeviceSynchronize ());
   return 0;
 }
diff --git a/gdb/testsuite/gdb.rocm/multi-inferior-gpu.cpp b/gdb/testsuite/gdb.rocm/multi-inferior-gpu.cpp
index d64afdd1994..4ba406f6398 100644
--- a/gdb/testsuite/gdb.rocm/multi-inferior-gpu.cpp
+++ b/gdb/testsuite/gdb.rocm/multi-inferior-gpu.cpp
@@ -95,7 +95,7 @@  child (int argc, char **argv)
 
   CHECK (hipSetDevice (dev_number));
   kern<<<1, 1>>> ();
-  hipDeviceSynchronize ();
+  CHECK (hipDeviceSynchronize ());
   return 0;
 }
 
diff --git a/gdb/testsuite/gdb.rocm/precise-memory-warning-sigsegv.cpp b/gdb/testsuite/gdb.rocm/precise-memory-warning-sigsegv.cpp
index bf1451ad8d5..d8ea86920e3 100644
--- a/gdb/testsuite/gdb.rocm/precise-memory-warning-sigsegv.cpp
+++ b/gdb/testsuite/gdb.rocm/precise-memory-warning-sigsegv.cpp
@@ -17,6 +17,17 @@ 
 
 #include <hip/hip_runtime.h>
 
+#define CHECK(cmd)                                                           \
+  {                                                                          \
+    hipError_t error = cmd;                                                  \
+    if (error != hipSuccess)                                                 \
+      {                                                                      \
+	fprintf (stderr, "error: '%s'(%d) at %s:%d\n",                       \
+		 hipGetErrorString (error), error, __FILE__, __LINE__);      \
+	exit (EXIT_FAILURE);                                                 \
+      }                                                                      \
+  }
+
 __global__ void
 kernel ()
 {
@@ -28,6 +39,6 @@  int
 main (int argc, char* argv[])
 {
   kernel<<<1, 1>>> ();
-  hipDeviceSynchronize ();
+  CHECK (hipDeviceSynchronize ());
   return 0;
 }
diff --git a/gdb/testsuite/gdb.rocm/precise-memory.cpp b/gdb/testsuite/gdb.rocm/precise-memory.cpp
index 034f02382ad..610f1873df6 100644
--- a/gdb/testsuite/gdb.rocm/precise-memory.cpp
+++ b/gdb/testsuite/gdb.rocm/precise-memory.cpp
@@ -17,6 +17,17 @@ 
 
 #include <hip/hip_runtime.h>
 
+#define CHECK(cmd)                                                           \
+  {                                                                          \
+    hipError_t error = cmd;                                                  \
+    if (error != hipSuccess)                                                 \
+      {                                                                      \
+	fprintf (stderr, "error: '%s'(%d) at %s:%d\n",                       \
+		 hipGetErrorString (error), error, __FILE__, __LINE__);      \
+	exit (EXIT_FAILURE);                                                 \
+      }                                                                      \
+  }
+
 __global__ void
 kernel ()
 {
@@ -27,6 +38,6 @@  int
 main (int argc, char* argv[])
 {
   kernel<<<1, 1>>> ();
-  hipDeviceSynchronize ();
+  CHECK (hipDeviceSynchronize ());
   return 0;
 }
diff --git a/gdb/testsuite/lib/rocm.exp b/gdb/testsuite/lib/rocm.exp
index fcdf665aef9..c2ffc571933 100644
--- a/gdb/testsuite/lib/rocm.exp
+++ b/gdb/testsuite/lib/rocm.exp
@@ -93,7 +93,8 @@  gdb_caching_proc allow_hipcc_tests {} {
 	    main ()
 	    {
 		kern<<<1, 1>>> ();
-		hipDeviceSynchronize ();
+		if (hipDeviceSynchronize () != hipSuccess)
+		  return -1;
 		return 0;
 	    }
 	} executable $flags]} {