[Patch] GCN: Initial generic-target handling

[Andrew Stubbs]

On 22/10/2024 11:04, Tobias Burnus wrote:
> Hi Andrew,
> 
> Andrew Stubbs wrote:
>> On 21/10/2024 20:49, Tobias Burnus wrote:
>>>      GCN_DEVICE field descriptions:
>>> -      0  "name"  (text, external)
>>> +      0 Generic flag/version (0 = non-generic, 1 to 255 = generic 
>>> version,
>>> +        external)
>>
>> I think this should mention EF_AMDGPU_GENERIC_VERSION_V, otherwise 
>> it's just a magic number.
> 
> You mean like:
> 
> - external)
> 
> + external, used with EF_AMDGPU_GENERIC_VERSION_V)
> 
> ?

As in, in the documentation line. C.f. "Magic number used assigned to 
this device for use in elf_flags."

> 
> Otherwise, it is not magic but a version number and because no 
> compatibility issues have been found, it is '1' for all gfx*-generic.

A "magic" number is any bare constant that appears in code without being 
wrapped in a named const, enum, or labelled with a comment, but 
especially numbers that have an assigned meaning outside of mathematics. 
This one is most certainly a "1" that makes hidden "magic" happen.

>> Actually, where are the numbers defined? Is it just that the linker 
>> demands that they match, so we can define them ourselves?
> 
> Well, given that llvm-mc sets it, our use in mkoffload.cc must match 
> llvm-mc. And I guess that ROCm also might care about the value. But in 
> terms of the linker, indeed, it is just required that all 
> generic-version numbers are identical.

OK, agreed, that sounds like an external definition to me.

>> Also, why does this have to be arg0? The whole point of using ellipsis 
>> everywhere is so we can add new fields without touching every instance 
>> that doesn't care.
> 
> As it seems to be a flag which is very generic – 0 actual arch, 1 (or 
> later other number) generic.

Is it though? It means nothing to any of the existing architectures, 
you've only added two extra where it's actually used, and in those cases 
it's just an attribute that those devices need passed to the assembler.

> That seems to be much easier to follow than having it buried in the 
> later arguments, because it makes sense hierarchically, doing it 
> differently feels a bit convoluted when writing the .def file.

Except that you've had to touch 3 files that you didn't need to, plus 4 
extra hunks in the rest of the files, and renumbered 8 args, when it was 
specifically designed so you didn't have to! LOL :-)

Plus, what hierarchy? It's a flat list in the order that I happened to 
implement the features.

I'm going to push the base patch shortly.

Andrew