[PATCH] D98223: [lld-macho] implement options -(un)exported_symbol(s_list)
Jez Ng via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 9 06:18:14 PST 2021
int3 accepted this revision.
int3 added inline comments.
This revision is now accepted and ready to land.
================
Comment at: lld/MachO/Config.h:53-54
+class SymbolPatterns {
+ llvm::DenseSet<llvm::StringRef> literals;
+ std::vector<llvm::GlobPattern> globs;
+
----------------
Hmm, GlobPattern.cpp has a special case for patterns without wildcards, but I guess having a separate literals set is still faster with the O(1) lookup
Since we're optimizing for speed here, I would use `CachedHashStringRef` for the literals set
================
Comment at: lld/MachO/Driver.cpp:741
+bool SymbolPatterns::matchGlob(StringRef symbolName) const {
+ for (auto const &glob : globs)
+ if (glob.match(symbolName))
----------------
avoid `auto`
================
Comment at: lld/test/MachO/export-options.s:11-12
+
+CONFLICT: error: cannot use both -exported_symbol* and -unexported_symbol* options
+CONFLICT-NEXT: >>> ignoring unexports
+
----------------
the lack of comment character works because split-file means it isn't passed into llvm-mc, but we should still make it a comment for uniformity
================
Comment at: lld/test/MachO/export-options.s:16-20
+.macro DEFSYM, type, sym
+\type \sym
+\sym:
+ retq
+.endm
----------------
I'm not sure I see much value in these macros... you're not really saving that many characters in exchange for an additional level of indirection
Without macros, you could save on the `retq`s by doing
```
_keep_globl:
_hide_globl:
_keep_private:
_show_private:
retq
```
just my 2c, keep the macros if you like them
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98223/new/
https://reviews.llvm.org/D98223
More information about the llvm-commits
mailing list