[PATCH] D74006: [MC][ELF] Make linked-to symbol name part of ELFSectionKey

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 5 00:32:19 PST 2020


grimar added a comment.

It looks reasonable to me. Two minor nits are below, the rest looks probably fine.
Lets see what other people think.



================
Comment at: llvm/include/llvm/MC/MCContext.h:217
+          return O < 0;
         return UniqueID < Other.UniqueID;
       }
----------------
Perhaps just:
```
if (UniqueID  != Other.UniqueID)
  return UniqueID < Other.UniqueID;
return LinkedToName < Other.LinkedToName;
```


================
Comment at: llvm/lib/MC/MCContext.cpp:320
   unsigned UniqueID = Section->getUniqueID();
   ELFUniquingMap.erase(
+      ELFSectionKey{Section->getSectionName(), GroupName, "", UniqueID});
----------------
MaskRay wrote:
> As a note: this is only used by renaming `.debug_*` to `.zdebug_*`, which don't do SHF_LINK_ORDER magic. So using an empty linked-to symbol name will be fine.
> 
> As I understand it, `.zdebug*` are deprecated. ELF toolchain moves to SHF_COMPRESSED, so we don't need to add more support to `.zdebug*`.
It deserves a comment in the code I think.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74006/new/

https://reviews.llvm.org/D74006





More information about the llvm-commits mailing list