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
