[PATCH] D61493: AMDGPU/MC: Add .amdgpu_lds directive
Nicolai Hähnle via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 24 07:17:40 PDT 2019
nhaehnle added inline comments.
================
Comment at: include/llvm/BinaryFormat/ELF.h:1429
+enum {
+ SHN_AMDGPU_LDS = 0xff00, // Variable in LDS; symbol encoded like SHN_COMMON
+};
----------------
arsenm wrote:
> Is there a particular reason for this value?
Yes, this is the first processor-specific section index. SHN_HEXAGON_SCOMMON* are defined similarly.
================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:601-602
+ if (Machine == EM_AMDGPU) {
+ return Index == SHN_AMDGPU_LDS;
+ } else if (Machine == EM_HEXAGON) {
switch (Index) {
----------------
arsenm wrote:
> No else after return
Done.
================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:630
case SYMBOL_COMMON:
- case SYMBOL_HEXAGON_SCOMMON:
+ case SYMBOL_HEXAGON_SCOMMON: // aliases SYMBOL_AMDGPU_LDS
case SYMBOL_HEXAGON_SCOMMON_2:
----------------
arsenm wrote:
> This seems like a problem?
After some more meditation on this, I think the correct approach is to treat all OS- or processor-specific reserved sections as acceptable.
Otherwise, the code would have to be refactored with knowledge about what the current OS and processor are at this point, which seems a major and unnecessary change.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61493/new/
https://reviews.llvm.org/D61493
More information about the llvm-commits
mailing list