[PATCH] D98762: [LTO][MC] Discard non-prevailing defined symbols in module-level assembly

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/LTO/LTO.cpp:756
   std::set<const Comdat *> NonPrevailingComdats;
+  std::set<StringRef> NonPrevailingAsmSymbols;
   for (const InputFile::Symbol &Sym : Syms) {
----------------
DenseSet<CachedHashStringRef>

std::set is inefficient. If you don't order guarantee, prefer an unordered container.


================
Comment at: llvm/lib/MC/MCParser/AsmParser.cpp:1894
 
+    if (discardLTOSymbol(IDVal))
+      return false;
----------------
Needs a test/MC/ELF test that reassignment does not error.


================
Comment at: llvm/lib/MC/MCParser/ELFAsmParser.cpp:185
 
+      if (getParser().discardLTOSymbol(Name)) {
+        if (getLexer().is(AsmToken::EndOfStatement))
----------------
Needs a test/MC/ELF test that reassignment does not error.


================
Comment at: llvm/test/LTO/X86/weak-asm.ll:13
+; RUN:  -r %t2,bar,pl
+; RUN: llvm-dis < %to1.0.0.preopt.bc -o - | FileCheck %s --check-prefix=ASM1
+; RUN: llvm-nm %to1.0 | FileCheck %s --check-prefix=SYM
----------------
Drop `-o - ` if you use `<`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98762