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

[Yuanfang Chen via Phabricator via llvm-commits]

ychen marked 2 inline comments as done.
ychen 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) {
----------------
MaskRay wrote:
> DenseSet<CachedHashStringRef>
> 
> std::set is inefficient. If you don't order guarantee, prefer an unordered container.
Agreed that std::set is not the most appropriate one. I used the SmallSet to fit the use case better. DenseSet seems allocate more than needed for the usage.


================
Comment at: llvm/lib/MC/MCParser/AsmParser.cpp:1894
 
+    if (discardLTOSymbol(IDVal))
+      return false;
----------------
MaskRay wrote:
> Needs a test/MC/ELF test that reassignment does not error.
Something like `llvm/test/MC/AsmParser/reassign.s` for this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98762