[PATCH] D140517: [lld-macho] Flip string deduplication default

[Jez Ng via Phabricator via llvm-commits]

int3 added a comment.

Thanks for doing this!



================
Comment at: lld/MachO/InputFiles.cpp:346
     if (sectionType(sec.flags) == S_CSTRING_LITERALS ||
-        (config->dedupLiterals && isWordLiteralSection(sec.flags))) {
-      if (sec.nreloc && config->dedupLiterals)
+        (config->dedupStrings && isWordLiteralSection(sec.flags))) {
+      if (sec.nreloc && config->dedupStrings)
----------------
since we're deduping all word literals unconditionally now, we should hoist line 362 into its own `if` block & not gate it on `config->dedupStrings`. And I guess if we find any relocations in literal sections we should just error out unconditionally.


================
Comment at: lld/MachO/Options.td:61
+def no_deduplicate_strings: Flag<["--"], "no-deduplicate-strings">,
+    HelpText<"Disable string deduplication. This helps uncover undefined behavior comparing string literals and might have a performance benefit.">,
     Group<grp_lld>;
----------------
Is comparing string pointers actually undefined, or just not what the code author intended?


================
Comment at: lld/MachO/Writer.cpp:1330
   in.wordLiteralSection =
-      config->dedupLiterals ? make<WordLiteralSection>() : nullptr;
+      config->dedupStrings ? make<WordLiteralSection>() : nullptr;
   if (config->emitChainedFixups) {
----------------
make unconditional


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140517