Re: [PATCH] Avoid registering unsupported OMP offload devices

[Andrew Stubbs]

On 26/01/2024 14:21, Richard Biener wrote:
On Fri, 26 Jan 2024, Jakub Jelinek wrote:
On Fri, Jan 26, 2024 at 03:04:11PM +0100, Richard Biener wrote:
Otherwise it looks reasoanble to me, but let's see what Andrew thinks.

'n' before 'a', please. ;-)

?!

I've misspelled a word.

@@ -1443,6 +1445,16 @@ suitable_hsa_agent_p (hsa_agent_t agent)
    switch (device_type)
      {
      case HSA_DEVICE_TYPE_GPU:
+      {
+       char name[64] = "nil";
+       if ((hsa_fns.hsa_agent_get_info_fn (agent, HSA_AGENT_INFO_NAME, name)
+            != HSA_STATUS_SUCCESS)
+           || isa_code (name) == EF_AMDGPU_MACH_UNSUPPORTED)
+         {
+           GCN_DEBUG ("Ignoring unsupported agent '%s'\n", name);
+           return false;
+         }

I must say I know nothing about HSA libraries, but generally if a function
that is supposed to fill some buffer fails the content of the buffer is
undefined/unpredictable.
So might be better not to not initialize name before calling the function
(unless it has to be initialized) and strcpy it to nil or something similar
if it fails.

Yeah, sorry.  Here's a proper engineered variant.  I don't expect that
function to ever fail of course.

I keep crossing emails with you today. :(

This version looks good to me. It has to work for all versions of ROCm 
from maybe 3.8 (the last version known to work with the Fiji devices) 
onwards to forever.

OK.

Andrew

 From 445891ba57e858d980441bd63249e3bc94632db3 Mon Sep 17 00:00:00 2001
From: Richard Biener <rguent...@suse.de>
Date: Fri, 26 Jan 2024 12:57:10 +0100
Subject: [PATCH] Avoid registering unsupported OMP offload devices
To: gcc-patches@gcc.gnu.org

The following avoids registering unsupported GCN offload devices
when iterating over available ones.  With a Zen4 desktop CPU
you will have an IGPU (unspported) which will otherwise be made
available.  This causes testcases like
libgomp.c-c++-common/non-rect-loop-1.c which iterate over all
decives to FAIL.

OK?

libgomp/
        * plugin/plugin-gcn.c (suitable_hsa_agent_p): Filter out
        agents with unsupported ISA.
---
  libgomp/plugin/plugin-gcn.c | 14 ++++++++++++++
  1 file changed, 14 insertions(+)

diff --git a/libgomp/plugin/plugin-gcn.c b/libgomp/plugin/plugin-gcn.c
index 588358bbbf9..2771123252a 100644
--- a/libgomp/plugin/plugin-gcn.c
+++ b/libgomp/plugin/plugin-gcn.c
@@ -1427,6 +1427,8 @@ init_hsa_runtime_functions (void)
  #undef DLSYM_FN
  }
  
+static gcn_isa isa_code (const char *isa);
+
  /* Return true if the agent is a GPU and can accept of concurrent submissions
     from different threads.  */
  
@@ -1443,6 +1445,18 @@ suitable_hsa_agent_p (hsa_agent_t agent)
    switch (device_type)
      {
      case HSA_DEVICE_TYPE_GPU:
+      {
+       char name[64];
+       hsa_status_t status
+         = hsa_fns.hsa_agent_get_info_fn (agent, HSA_AGENT_INFO_NAME, name);
+       if (status != HSA_STATUS_SUCCESS
+           || isa_code (name) == EF_AMDGPU_MACH_UNSUPPORTED)
+         {
+           GCN_DEBUG ("Ignoring unsupported agent '%s'\n",
+                      status == HSA_STATUS_SUCCESS ? name : "invalid");
+           return false;
+         }
+      }
        break;
      case HSA_DEVICE_TYPE_CPU:
        if (!support_cpu_devices)