Details
- Reviewers
• tstellarAMD • rafael
Diff Detail
Event Timeline
Last time I proposed a patch adding AMDGPU Relocation enums, Rafael asked for public documentation of these relocations. Do we have public docs yet?
include/llvm/Object/RelocVisitor.h | ||
---|---|---|
417 | The & 0xFFFFFFFF is redundant with the truncate to uint32_t |
include/llvm/Support/ELFRelocs/AMDGPU.def | ||
---|---|---|
7 | Document what these are. | |
lib/MC/MCObjectFileInfo.cpp | ||
429–430 ↗ | (On Diff #53280) | Is anything testing this? Can in be in another patch? |
lib/Target/AMDGPU/MCTargetDesc/AMDGPUELFObjectWriter.cpp | ||
14 | I don't think you have a test for the changes is thin file. Can they be in another patch. |
include/llvm/Support/ELFRelocs/AMDGPU.def | ||
---|---|---|
7–10 | Could you add the corresponding entries to the VariantKind enum in include/llvm/MC/MCExpr.h too, and also MCSymbolRefExpr::getVariantKindName(VariantKind Kind) in lib/MC/MCExpr.cpp |
lib/MC/MCExpr.cpp | ||
---|---|---|
280–282 | I think for assembler support, you also need to add these to MCSymbolRefExpr::getVariantKindForName(StringRef Name) |
If this is just for dwarf, why do you need high and lo relocations?
On x86_64 we use 32 and 64 relocations, but they apply to 32 and 64 bit
values.
This patch adds all the relocation definitions, but only uses the ones required for dwarf. I'm working on patches to add support for relocations of global variables, which use the high relocations.
Please leave the high one out of this patch.
It looks like the low one is actually just a 32 bit reloc. If not, add a
test case that shows the difference.
It sounds like we should add a new R_AMDGPU_32 reloc type to use for debug information in order to distinguish it from R_AMDGPU_32_LO/HI.
The & 0xFFFFFFFF is redundant with the truncate to uint32_t