On 20/01/2020 16:42, Harwath, Frederik wrote:
Hi Andrew,
Thanks for the review! I have attached a revised patch containing the changes
that you suggested.
On 20.01.20 11:00, Andrew Stubbs wrote:
On 20/01/2020 06:57, Harwath, Frederik wrote:
Is it ok to commit this patch to the master branch?
I can't see anything significantly wrong with the code of the patch, however I
have some minor issues I'd like fixed in the text.
[...] Please move the functions down into the "Utility functions" group. The
const static variables should probably go with them.
Done.
@@ -3294,7 +3415,11 @@ GOMP_OFFLOAD_init_device (int n)
&buf);
if (status != HSA_STATUS_SUCCESS)
return hsa_error ("Error querying the name of the agent", status);
- agent->gfx900_p = (strncmp (buf, "gfx900", 6) == 0);
+ agent->gfx900_p = (strncmp (buf, gcn_gfx900_s, gcn_isa_name_len) == 0);
+
+ agent->device_isa = isa_code (buf);
+ if (agent->device_isa < 0)
+ return hsa_error ("Unknown GCN agent architecture.", HSA_STATUS_ERROR);
Can device_isa not just replace gfx900_p? I think it's only tested in one
place, and that would be easily substituted.
Yes, I have changed that one place to use agent->device_isa.
I would commit the patch then if nobody objects :-). The other approaches (fat
binaries etc.) that have been discussed in
this thread seem to be long-term projects and until something like this gets
implemented the early error checking
implemented by this patch seems to be better than nothing.
OK.
Andrew
