[PATCH] D130134: [lld-macho] Fold cfstrings with --deduplicate-literals

[Jez Ng via Phabricator via llvm-commits]

int3 accepted this revision.
int3 added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: lld/MachO/Driver.cpp:1636
         markAddrSigSymbols();
-      foldIdenticalSections();
+      foldIdenticalSections(/*onlyCfStrings*/ false);
+    } else if (config->dedupLiterals) {
----------------
this is the more typical codebase convention


================
Comment at: lld/MachO/ICF.cpp:403
 
-void macho::foldIdenticalSections() {
+void macho::foldIdenticalSections(bool onlyCfStrings) {
   TimeTraceScope timeScope("Fold Identical Code Sections");
----------------
keith wrote:
> I'm not happy with hijacking this function for this specific use case, so it would probably make sense for me to try to extract something here
yeah that would be nice but no rush


================
Comment at: lld/test/MachO/cfstring-dedup.s:40-51
+# LITERALS-ONLY:       Rebase table:
+# LITERALS-ONLY-NEXT:  segment  section          address  type
+# LITERALS-ONLY-NEXT:  __DATA_CONST __cfstring   {{.*}}   pointer
+# LITERALS-ONLY-NEXT:  __DATA_CONST __cfstring   {{.*}}   pointer
+# LITERALS-ONLY-NEXT:  __DATA_CONST __cfstring   {{.*}}   pointer
+# LITERALS-ONLY-EMPTY:
+# LITERALS-ONLY-NEXT:  Bind table:
----------------
isn't this identical to the `CHECK` lines above? don't think we need to define a separate CHECK prefix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130134