[PATCH] D106228: [LTO] Add SelectionKind to IRSymtab and use it in ld.lld/LLVMgold

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Thanks for the langref and IR patches, that helps my understanding. Have some suggestions below on new/improved comments needed.



================
Comment at: lld/COFF/InputFiles.cpp:1070
   for (size_t i = 0; i != obj->getComdatTable().size(); ++i)
     // FIXME: lto::InputFile doesn't keep enough data to do correct comdat
     // selection handling.
----------------
Can this FIXME be removed now? Or are there more improvements needed?


================
Comment at: lld/test/ELF/lto/comdat-noduplicates.ll:19
+
+; DATA: 0x[[#%x,]] 01000000 00000000  ........
+
----------------
Please add a comment - I'm not completely sure what this value is.


================
Comment at: lld/test/ELF/lto/comdat-noduplicates.ll:58
+
+; IR_AB: gv: (guid: [[PROFD]], summaries: (variable: (module: ^0, flags: (linkage: private, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 1, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0, constant: 0),
+; IR_AB: gv: (guid: [[FOO]], summaries: (function: (module: ^0, flags: (linkage: available_externally, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 4,
----------------
Some comments here and in the ABC case below about what specifically we are checking for in the resulting summaries would be helpful.


================
Comment at: llvm/tools/gold/gold-plugin.cpp:627
+      std::pair<StringRef, Comdat::SelectionKind> C = Obj->getComdatTable()[CI];
+      if (C.second != Comdat::NoDuplicates)
+        sym.comdat_key = strdup(C.first.str().c_str());
----------------
Comment needed on how this achieves NoDuplicates effect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106228