[Patch][RFC] GCN: Define ISA archs in gcn-devices.def and use it

[Andrew Stubbs]

On 15/03/2024 12:21, Tobias Burnus wrote:
> Given the large number of AMD GPU ISAs and the number of files which 
> have to be adapted, I wonder whether it makes sense to consolidate this 
> a bit, especially in the light that we may want to support more in the 
> future.
> 
> Besides using some macros, I also improved the diagnostic if the object 
> code couldn't be recognized (shouldn't happen) or if the GPU is 
> unsupported (likely; it now prints the GPU string). I was initially 
> thinking of resolving the arch encoded in the eflag to a string, but as 
> this is about GCC-generated code, it seemed to be unlikely of much use. 
> [It should that rare that we might also go back to the static string 
> instead of outputting the hex value of the eflag.]
> 
> Note: I only modified mkoffload.cc and plugin-gcn.c, but with some 
> tweaks it could also be used for other files in gcc/config/gcn/.
> 
> If you add a new ISA, you still need to update plugin-gcn.c's 
> max_isa_vgprs and the xnack/sram-ecc handling in mkoffload.c's main, but 
> that should be all for those two files.
> 
> Thoughts?

This is more-or-less what I was planning to do myself, but as I want to 
include all the other features that get parametrized in gcn.cc, gcn.h, 
gcn-hsa.h, gcn-opts.h, I hadn't got around to it yet.  Unfortunately, I 
think the gcn.opt and config.gcc will always need manually updating, but 
if that's all it'll be an improvement.

I don't like the idea of including AMDGPU_ISA_UNSUPPORTED; that list is 
going to be permanently out of date, and even if we maintain it 
fastidiously last year's release isn't going to have the updated list in 
the wild. I think it's not actually active in this patch in any case.

Instead of AMDGPU_ISA, I think "AMDGPU_ELF" makes more sense. The ISA is 
"CDNA2" or "RDNA3", etc., and the compiler needs to know about that.

Ultimately, I want to replace many of the conditionals like 
"TARGET_CDNA2_PLUS" from the code and replace them with feature flags 
derived from a def file, or at least a header file. We've acquired too 
many places where there are unsearchable conditionals that need finding 
and fixing every time a new device comes along.

I had imagined that this .def file would exist in gcc/config/gcn, but 
you've placed it in libgomp.... maybe it makes sense to have multiple 
such files if they contain very different data, but I had imagined one 
file and I'm not sure that the compiler definitions live in libgomp.

> Tobias
> 
> PS: I think the patch is fine and builds, but I have not tested it on an 
> AMD GPU machine, yet.
> 
> PPS: For using for other files, see also in config/nvptx which uses 
> nvptx-sm.def to generate several files.
> 

Andrew