Context
While preparing the HotSwap infrastructure PRs (#2201, #2202, #2203) we had to
locally redefine two AMDGPU-specific ELF note constants because they only live
in target-internal LLVM headers that are not exported by a comgr standalone
install, and are not currently registered in any public llvm::ELF enum.
Constants that need a public home
1. AMDGPU note owner string ("AMDGPU")
Currently defined in llvm/lib/Target/AMDGPU/AMDGPUPTNote.h:
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPUPTNote.h#L22-L29
This is a target-internal header (under llvm/lib/Target/AMDGPU/) and is not
installed or usable by out-of-tree consumers such as Comgr in its standalone
build mode.
Comgr's hotswap PRs duplicate this constant as:
// amd/comgr/src/comgr-hotswap-internal.h
static constexpr llvm::StringLiteral kAmdgpuNoteOwner = \"AMDGPU\";
2. AMDGPU ISA-version note type (NT_AMDGPU_ISA_VERSION = 27)
There is no corresponding entry in llvm/include/llvm/BinaryFormat/ELF.h for
this note type. The note-name block of that header registers many
vendor-specific NT_ constants (see for example the AMD GPU block at
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/BinaryFormat/ELF.h#L1980-L1995),
but not the ISA-version note used by AMDGPU code-object v3+.
Comgr's hotswap PRs duplicate the raw value as:
// amd/comgr/src/comgr-hotswap-internal.h
static constexpr uint32_t kNoteTypeIsaVersion = 27;
Proposal
Upstream both constants into public LLVM headers so Comgr (and any other
out-of-tree consumer) can use the canonical LLVM definitions without duplication
or divergence risk:
- Move or re-export
AMDGPU::ElfNote::NoteNameV3 (and any related
NoteName* constants) from AMDGPUPTNote.h into a public header such as
llvm/include/llvm/BinaryFormat/AMDGPUMetadataVerifier.h or a new
llvm/include/llvm/BinaryFormat/AMDGPU.h.
- Add the AMDGPU ISA-version note type (value
27) to the AMDGPU block in
llvm/include/llvm/BinaryFormat/ELF.h, e.g. as NT_AMDGPU_ISA_VERSION.
Once the public definitions exist, Comgr will switch to them and drop the
local duplicates.
Context PRs
Tracked in reviewer feedback on:
Context
While preparing the HotSwap infrastructure PRs (#2201, #2202, #2203) we had to
locally redefine two AMDGPU-specific ELF note constants because they only live
in target-internal LLVM headers that are not exported by a comgr standalone
install, and are not currently registered in any public
llvm::ELFenum.Constants that need a public home
1. AMDGPU note owner string (
"AMDGPU")Currently defined in
llvm/lib/Target/AMDGPU/AMDGPUPTNote.h:https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPUPTNote.h#L22-L29
This is a target-internal header (under
llvm/lib/Target/AMDGPU/) and is notinstalled or usable by out-of-tree consumers such as Comgr in its standalone
build mode.
Comgr's hotswap PRs duplicate this constant as:
2. AMDGPU ISA-version note type (
NT_AMDGPU_ISA_VERSION = 27)There is no corresponding entry in
llvm/include/llvm/BinaryFormat/ELF.hforthis note type. The note-name block of that header registers many
vendor-specific
NT_constants (see for example the AMD GPU block athttps://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/BinaryFormat/ELF.h#L1980-L1995),
but not the ISA-version note used by AMDGPU code-object v3+.
Comgr's hotswap PRs duplicate the raw value as:
Proposal
Upstream both constants into public LLVM headers so Comgr (and any other
out-of-tree consumer) can use the canonical LLVM definitions without duplication
or divergence risk:
AMDGPU::ElfNote::NoteNameV3(and any relatedNoteName*constants) fromAMDGPUPTNote.hinto a public header such asllvm/include/llvm/BinaryFormat/AMDGPUMetadataVerifier.hor a newllvm/include/llvm/BinaryFormat/AMDGPU.h.27) to the AMDGPU block inllvm/include/llvm/BinaryFormat/ELF.h, e.g. asNT_AMDGPU_ISA_VERSION.Once the public definitions exist, Comgr will switch to them and drop the
local duplicates.
Context PRs
Tracked in reviewer feedback on:
kAmdgpuNoteOwner: [AMDGPU] comgr: add HotSwap ELF infrastructure (1/3) #2201 (comment)