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

[ben via Phabricator via llvm-commits]

bd1976llvm added a comment.

In D74006#1860074 <https://reviews.llvm.org/D74006#1860074>, @MaskRay wrote:

> Another thing is whether we should use sh_flags/sh_type to differentiate sections. In addition, whether we should use sh_entsize.
>  If yes, the implementation can be a bit clumsy. We may have to add many `Elf*_Shdr` fields to `ELFSectionKey`.
>  If no (GNU as behavior), we should add warnings. D73999 <https://reviews.llvm.org/D73999> will do that.
>
> The section group name is part of the key in both MC and GNU as. By analogy, SHF_LINK_ORDER+sh_link should probably be part of the key.
>
> Field not specified by the `.section` directive (sh_addrline,sh_addr,sh_offset,sh_info) should definitely not be part of the key.
>
> I have a slight preference for "no", though I don't currently have a particular good reason why "sh_entsize" should not be part of a key. Maybe we are fine with current `.rodata.cst16` naming.


An alternative design to this patch is for the assembler to *never* create more than one section with a given name (apart from for COMDATs because that is established behaviour we can't change) unless the ,unique, N, extension is used. However, the assembler can give warnings as in D73999 <https://reviews.llvm.org/D73999>. This is really simple for users to understand - the rule is you have to choose a unique name for your sections.



================
Comment at: llvm/include/llvm/MC/MCContext.h:199
+    // combined into one section.
     struct ELFSectionKey {
       std::string SectionName;
----------------
MaskRay wrote:
> bd1976llvm wrote:
> > Here start using "linked_to" terminology instead of "associated". Should we stick to just one?
> "linked-to section" is used by the ELF spec. By analogy, I use "linked-to symbol" here (omitted "Sym" in "LinkedToName"). "linked-to" implies a directed edge and makes it clear its relation with "sh_link", while one can argue that "associated" means an undirected edge. I think we should rename MCSymbol related "Associated" to "LinkedToSym". The IR level "!associated" can be left unchanged. I can do that in a separate change.
SGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74006