Background
amd/comgr/src/comgr-hotswap-b0a0.cpp currently carries two hardcoded per-ISA constants for the GFX1250 B0-to-A0 rewrite policy:
static constexpr unsigned Gfx1250VgprGranuleSize = 8;
static constexpr unsigned Gfx1250SgprGranuleSize = 16;
These are hardware facts per AMDGPU generation (gfx10+ wave32: VGPR granule 8, SGPR granule 16) and are already encoded in LLVM — llvm::AMDGPU::IsaInfo::getVGPRAllocGranule(MCSubtargetInfo) / getSGPRAllocGranule(MCSubtargetInfo) — but that header lives inside the AMDGPU target:
llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
Comgr also builds standalone against an installed LLVM, and target-internal headers aren't reliably on that include path. That's the same constraint that kept us out of AMDGPUTargetStreamer::getElfMach() during #2201 review.
Proposal
Pick one of:
- Promote
AMDGPU::IsaInfo granule getters to a public LLVM header. Simplest API-wise. Adds a small public surface (llvm/include/llvm/TargetParser/AMDGPUIsaInfo.h or similar) that wraps the existing target-internal implementation. Upstream LLVM change.
- Add thin helpers to
llvm/Support/AMDHSAKernelDescriptor.h. Keeps the new surface AMDHSA-flavored and on a header Comgr already uses.
- Have comgr query
MCSubtargetInfo::getFeatureBits() + a tiny local family table. Avoids touching upstream, but duplicates logic.
Option 1 or 2 are preferred — option 3 recreates the same knowledge LLVM already has.
Once landed
Drop Gfx1250VgprGranuleSize / Gfx1250SgprGranuleSize (and any future per-ISA equivalents) from comgr-hotswap-b0a0.cpp and populate RewriteConfig::VgprGranuleSize / RewriteConfig::SgprGranuleSize via the public helper, driven by the parsed target ISA.
Context
Called out during PR #2203 review: #2203 (comment). Tracking this here so the constants don't accumulate as more source/target policies land. cc @chinmaydd @lamb-j
Background
amd/comgr/src/comgr-hotswap-b0a0.cppcurrently carries two hardcoded per-ISA constants for the GFX1250 B0-to-A0 rewrite policy:These are hardware facts per AMDGPU generation (gfx10+ wave32: VGPR granule 8, SGPR granule 16) and are already encoded in LLVM —
llvm::AMDGPU::IsaInfo::getVGPRAllocGranule(MCSubtargetInfo)/getSGPRAllocGranule(MCSubtargetInfo)— but that header lives inside the AMDGPU target:Comgr also builds standalone against an installed LLVM, and target-internal headers aren't reliably on that include path. That's the same constraint that kept us out of
AMDGPUTargetStreamer::getElfMach()during #2201 review.Proposal
Pick one of:
AMDGPU::IsaInfogranule getters to a public LLVM header. Simplest API-wise. Adds a small public surface (llvm/include/llvm/TargetParser/AMDGPUIsaInfo.hor similar) that wraps the existing target-internal implementation. Upstream LLVM change.llvm/Support/AMDHSAKernelDescriptor.h. Keeps the new surface AMDHSA-flavored and on a header Comgr already uses.MCSubtargetInfo::getFeatureBits()+ a tiny local family table. Avoids touching upstream, but duplicates logic.Option 1 or 2 are preferred — option 3 recreates the same knowledge LLVM already has.
Once landed
Drop
Gfx1250VgprGranuleSize/Gfx1250SgprGranuleSize(and any future per-ISA equivalents) fromcomgr-hotswap-b0a0.cppand populateRewriteConfig::VgprGranuleSize/RewriteConfig::SgprGranuleSizevia the public helper, driven by the parsed target ISA.Context
Called out during PR #2203 review: #2203 (comment). Tracking this here so the constants don't accumulate as more source/target policies land. cc @chinmaydd @lamb-j