Re: [PATCH] Fix architecture support in OMP_OFFLOAD_init_device for gcn

2024-01-26 Thread Andrew Stubbs 

On 26/01/2024 11:42, Richard Biener wrote:

The following makes the existing architecture support check work
instead of being optimized away (enum vs. -1).  This avoids
later asserts when we assume such devices are never actually
used.

Tested as previously, now the error is

libgomp: GCN fatal error: Unknown GCN agent architecture
Runtime message: HSA_STATUS_ERROR: A generic error has occurred.

now will figure why we try to initialize that device.

OK?


OK.



libgomp/
* plugin/plugin-gcn.c
(EF_AMDGPU_MACH::EF_AMDGPU_MACH_UNSUPPORTED): Add.
(isa_code): Return that instead of -1.
(GOMP_OFFLOAD_init_device): Adjust.
---
  libgomp/plugin/plugin-gcn.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libgomp/plugin/plugin-gcn.c b/libgomp/plugin/plugin-gcn.c
index db28781dedb..588358bbbf9 100644
--- a/libgomp/plugin/plugin-gcn.c
+++ b/libgomp/plugin/plugin-gcn.c
@@ -384,6 +384,7 @@ struct gcn_image_desc
 See https://llvm.org/docs/AMDGPUUsage.html#amdgpu-ef-amdgpu-mach-table */
  
  typedef enum {

+  EF_AMDGPU_MACH_UNSUPPORTED = -1,
EF_AMDGPU_MACH_AMDGCN_GFX803 = 0x02a,
EF_AMDGPU_MACH_AMDGCN_GFX900 = 0x02c,
EF_AMDGPU_MACH_AMDGCN_GFX906 = 0x02f,
@@ -1727,7 +1728,7 @@ isa_code(const char *isa) {
if (!strncmp (isa, gcn_gfx1100_s, gcn_isa_name_len))
  return EF_AMDGPU_MACH_AMDGCN_GFX1100;
  
-  return -1;

+  return EF_AMDGPU_MACH_UNSUPPORTED;
  }
  
  /* CDNA2 devices have twice as many VGPRs compared to older devices.  */

@@ -3374,7 +3375,7 @@ GOMP_OFFLOAD_init_device (int n)
  return hsa_error ("Error querying the name of the agent", status);
  
agent->device_isa = isa_code (agent->name);

-  if (agent->device_isa < 0)
+  if (agent->device_isa == EF_AMDGPU_MACH_UNSUPPORTED)
  return hsa_error ("Unknown GCN agent architecture", HSA_STATUS_ERROR);
  
status = hsa_fns.hsa_agent_get_info_fn (agent->id, HSA_AGENT_INFO_VENDOR_NAME,

[PATCH] Fix architecture support in OMP_OFFLOAD_init_device for gcn

2024-01-26 Thread Richard Biener 
The following makes the existing architecture support check work
instead of being optimized away (enum vs. -1).  This avoids
later asserts when we assume such devices are never actually
used.

Tested as previously, now the error is

libgomp: GCN fatal error: Unknown GCN agent architecture
Runtime message: HSA_STATUS_ERROR: A generic error has occurred.

now will figure why we try to initialize that device.

OK?

libgomp/
* plugin/plugin-gcn.c
(EF_AMDGPU_MACH::EF_AMDGPU_MACH_UNSUPPORTED): Add.
(isa_code): Return that instead of -1.
(GOMP_OFFLOAD_init_device): Adjust.
---
 libgomp/plugin/plugin-gcn.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libgomp/plugin/plugin-gcn.c b/libgomp/plugin/plugin-gcn.c
index db28781dedb..588358bbbf9 100644
--- a/libgomp/plugin/plugin-gcn.c
+++ b/libgomp/plugin/plugin-gcn.c
@@ -384,6 +384,7 @@ struct gcn_image_desc
See https://llvm.org/docs/AMDGPUUsage.html#amdgpu-ef-amdgpu-mach-table */
 
 typedef enum {
+  EF_AMDGPU_MACH_UNSUPPORTED = -1,
   EF_AMDGPU_MACH_AMDGCN_GFX803 = 0x02a,
   EF_AMDGPU_MACH_AMDGCN_GFX900 = 0x02c,
   EF_AMDGPU_MACH_AMDGCN_GFX906 = 0x02f,
@@ -1727,7 +1728,7 @@ isa_code(const char *isa) {
   if (!strncmp (isa, gcn_gfx1100_s, gcn_isa_name_len))
 return EF_AMDGPU_MACH_AMDGCN_GFX1100;
 
-  return -1;
+  return EF_AMDGPU_MACH_UNSUPPORTED;
 }
 
 /* CDNA2 devices have twice as many VGPRs compared to older devices.  */
@@ -3374,7 +3375,7 @@ GOMP_OFFLOAD_init_device (int n)
 return hsa_error ("Error querying the name of the agent", status);
 
   agent->device_isa = isa_code (agent->name);
-  if (agent->device_isa < 0)
+  if (agent->device_isa == EF_AMDGPU_MACH_UNSUPPORTED)
 return hsa_error ("Unknown GCN agent architecture", HSA_STATUS_ERROR);
 
   status = hsa_fns.hsa_agent_get_info_fn (agent->id, 
HSA_AGENT_INFO_VENDOR_NAME,
-- 
2.35.3