[lld] [lld][InstrProf] Sort startup functions for compression (PR #107348)

Ellis Hoag via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 5 15:08:53 PDT 2024


https://github.com/ellishg updated https://github.com/llvm/llvm-project/pull/107348

>From 55fee69f0246c8209edbfeeed3aea52dac5cd364 Mon Sep 17 00:00:00 2001
From: Ellis Hoag <ellis.sparky.hoag at gmail.com>
Date: Wed, 4 Sep 2024 20:31:55 -0700
Subject: [PATCH 1/3] [InstrProf][LLD] Sort startup functions for compression

|                       | Page Fault Area | Compressed Size |
| --------------------- | --------------- | --------------- |
| Base                  | 1.045659e+09    | 25098246        |
| Startup Compression   | 1.430330e+09    | 24740192        |
| Remove Startup Hashes | 2.272126e+09    | 24574929        |
---
 lld/MachO/BPSectionOrderer.cpp             | 20 +++++++++++++++++---
 lld/MachO/BPSectionOrderer.h               |  2 +-
 lld/MachO/Config.h                         |  1 +
 lld/MachO/Driver.cpp                       |  3 +++
 lld/MachO/Options.td                       |  2 ++
 lld/MachO/SectionPriorities.cpp            |  1 +
 lld/test/MachO/bp-section-orderer-stress.s |  4 ++--
 7 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/lld/MachO/BPSectionOrderer.cpp b/lld/MachO/BPSectionOrderer.cpp
index 97458c96fd80da..a8255294306826 100644
--- a/lld/MachO/BPSectionOrderer.cpp
+++ b/lld/MachO/BPSectionOrderer.cpp
@@ -158,7 +158,8 @@ static SmallVector<std::pair<unsigned, UtilityNodes>> getUnsForCompression(
 
 DenseMap<const InputSection *, size_t> lld::macho::runBalancedPartitioning(
     size_t &highestAvailablePriority, StringRef profilePath,
-    bool forFunctionCompression, bool forDataCompression, bool verbose) {
+    bool forFunctionCompression, bool forDataCompression,
+    bool compressionSortStartupFunctions, bool verbose) {
 
   SmallVector<const InputSection *> sections;
   DenseMap<const InputSection *, uint64_t> sectionToIdx;
@@ -268,8 +269,15 @@ DenseMap<const InputSection *, size_t> lld::macho::runBalancedPartitioning(
     }
   }
 
-  // Map a section index (to be ordered for compression) to a list of duplicate
-  // section indices (not ordered for compression).
+  SmallVector<unsigned> startupIdxs;
+  if (compressionSortStartupFunctions)
+    for (auto &[sectionIdx, uns] : startupSectionIdxUNs)
+      startupIdxs.push_back(sectionIdx);
+  auto unsForStartupFunctionCompression =
+      getUnsForCompression(sections, sectionToIdx, startupIdxs, nullptr, maxUN);
+
+  // Map a section index (order directly) to a list of duplicate section indices
+  // (not ordered directly).
   DenseMap<unsigned, SmallVector<unsigned>> duplicateSectionIdxs;
   auto unsForFunctionCompression = getUnsForCompression(
       sections, sectionToIdx, sectionIdxsForFunctionCompression,
@@ -278,6 +286,12 @@ DenseMap<const InputSection *, size_t> lld::macho::runBalancedPartitioning(
       sections, sectionToIdx, sectionIdxsForDataCompression,
       &duplicateSectionIdxs, maxUN);
 
+  for (auto &[sectionIdx, compressionUns] : unsForStartupFunctionCompression) {
+    auto &uns = startupSectionIdxUNs[sectionIdx];
+    uns.append(compressionUns);
+    llvm::sort(uns);
+    uns.erase(std::unique(uns.begin(), uns.end()), uns.end());
+  }
   std::vector<BPFunctionNode> nodesForStartup, nodesForFunctionCompression,
       nodesForDataCompression;
   for (auto &[sectionIdx, uns] : startupSectionIdxUNs)
diff --git a/lld/MachO/BPSectionOrderer.h b/lld/MachO/BPSectionOrderer.h
index 6f9eefd5d82beb..cefd0ceb10e561 100644
--- a/lld/MachO/BPSectionOrderer.h
+++ b/lld/MachO/BPSectionOrderer.h
@@ -30,7 +30,7 @@ llvm::DenseMap<const lld::macho::InputSection *, size_t>
 runBalancedPartitioning(size_t &highestAvailablePriority,
                         llvm::StringRef profilePath,
                         bool forFunctionCompression, bool forDataCompression,
-                        bool verbose);
+                        bool compressionSortStartupFunctions, bool verbose);
 
 } // namespace lld::macho
 
diff --git a/lld/MachO/Config.h b/lld/MachO/Config.h
index 5beb0662ba7274..17259ee75aa4db 100644
--- a/lld/MachO/Config.h
+++ b/lld/MachO/Config.h
@@ -218,6 +218,7 @@ struct Configuration {
   llvm::StringRef printSymbolOrder;
 
   llvm::StringRef irpgoProfileSortProfilePath;
+  bool compressionSortStartupFunctions = false;
   bool functionOrderForCompression = false;
   bool dataOrderForCompression = false;
   bool verboseBpSectionOrderer = false;
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 09a539d71dab3b..73922620969bbf 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1772,6 +1772,9 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
     config->irpgoProfileSortProfilePath = arg->getValue();
     IncompatWithCGSort(arg->getSpelling());
   }
+  config->compressionSortStartupFunctions =
+      args.hasFlag(OPT_compression_sort_startup_functions,
+                   OPT_no_compression_sort_startup_functions, false);
   if (const Arg *arg = args.getLastArg(OPT_compression_sort)) {
     StringRef compressionSortStr = arg->getValue();
     if (compressionSortStr == "function") {
diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td
index 9c9570cdbeb05c..4ac1e2080de5cc 100644
--- a/lld/MachO/Options.td
+++ b/lld/MachO/Options.td
@@ -131,6 +131,8 @@ def irpgo_profile_sort_eq: Joined<["--"], "irpgo-profile-sort=">,
     Alias<!cast<Separate>(irpgo_profile_sort)>, MetaVarName<"<profile>">,
     HelpText<"Read the IRPGO profile at <profile> to order sections to improve startup time">,
     Group<grp_lld>;
+def compression_sort_startup_functions: Flag<["--"], "compression-sort-startup-functions">, HelpText<"TODO">, Group<grp_lld>;
+def no_compression_sort_startup_functions: Flag<["--"], "no-compression-sort-startup-functions">, HelpText<"TODO">, Group<grp_lld>;
 def compression_sort: Joined<["--"], "compression-sort=">,
     MetaVarName<"[none,function,data,both]">,
     HelpText<"Order sections to improve compressed size">, Group<grp_lld>;
diff --git a/lld/MachO/SectionPriorities.cpp b/lld/MachO/SectionPriorities.cpp
index 69c301d8ff8a71..1e7fb5973b8086 100644
--- a/lld/MachO/SectionPriorities.cpp
+++ b/lld/MachO/SectionPriorities.cpp
@@ -359,6 +359,7 @@ macho::PriorityBuilder::buildInputSectionPriorities() {
     sectionPriorities = runBalancedPartitioning(
         highestAvailablePriority, config->irpgoProfileSortProfilePath,
         config->functionOrderForCompression, config->dataOrderForCompression,
+        config->compressionSortStartupFunctions,
         config->verboseBpSectionOrderer);
   } else if (config->callGraphProfileSort) {
     // Sort sections by the profile data provided by __LLVM,__cg_profile
diff --git a/lld/test/MachO/bp-section-orderer-stress.s b/lld/test/MachO/bp-section-orderer-stress.s
index fdc6a20e2655b9..986e2d8fd1069b 100644
--- a/lld/test/MachO/bp-section-orderer-stress.s
+++ b/lld/test/MachO/bp-section-orderer-stress.s
@@ -7,8 +7,8 @@
 # RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t.s -o %t.o
 # RUN: llvm-profdata merge %t.proftext -o %t.profdata
 
-# RUN: %lld -arch arm64 -lSystem -e _main --icf=all -o - %t.o --irpgo-profile-sort=%t.profdata --compression-sort=both | llvm-nm --numeric-sort --format=just-symbols - > %t.order1.txt
-# RUN: %lld -arch arm64 -lSystem -e _main --icf=all -o - %t.o --irpgo-profile-sort=%t.profdata --compression-sort=both | llvm-nm --numeric-sort --format=just-symbols - > %t.order2.txt
+# RUN: %lld -arch arm64 -lSystem -e _main --icf=all -o - %t.o --irpgo-profile-sort=%t.profdata --compression-sort-startup-functions --compression-sort=both | llvm-nm --numeric-sort --format=just-symbols - > %t.order1.txt
+# RUN: %lld -arch arm64 -lSystem -e _main --icf=all -o - %t.o --irpgo-profile-sort=%t.profdata --compression-sort-startup-functions --compression-sort=both | llvm-nm --numeric-sort --format=just-symbols - > %t.order2.txt
 # RUN: diff %t.order1.txt %t.order2.txt
 
 import random

>From db369763cf86abea938a1c2a4fdc2d11a854ceef Mon Sep 17 00:00:00 2001
From: Ellis Hoag <ellis.sparky.hoag at gmail.com>
Date: Thu, 5 Sep 2024 10:08:21 -0700
Subject: [PATCH 2/3] Add help message

---
 lld/MachO/Driver.cpp                     | 4 ++++
 lld/MachO/Options.td                     | 7 +++++--
 lld/test/MachO/bp-section-orderer-errs.s | 3 +++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 73922620969bbf..b27c76036d8529 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1775,6 +1775,10 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
   config->compressionSortStartupFunctions =
       args.hasFlag(OPT_compression_sort_startup_functions,
                    OPT_no_compression_sort_startup_functions, false);
+  if (config->irpgoProfileSortProfilePath.empty() &&
+      config->compressionSortStartupFunctions)
+    error("--compression-sort-startup-functions must be used with "
+          "--irpgo-profile-sort");
   if (const Arg *arg = args.getLastArg(OPT_compression_sort)) {
     StringRef compressionSortStr = arg->getValue();
     if (compressionSortStr == "function") {
diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td
index 4ac1e2080de5cc..cbd28bbceef786 100644
--- a/lld/MachO/Options.td
+++ b/lld/MachO/Options.td
@@ -131,8 +131,11 @@ def irpgo_profile_sort_eq: Joined<["--"], "irpgo-profile-sort=">,
     Alias<!cast<Separate>(irpgo_profile_sort)>, MetaVarName<"<profile>">,
     HelpText<"Read the IRPGO profile at <profile> to order sections to improve startup time">,
     Group<grp_lld>;
-def compression_sort_startup_functions: Flag<["--"], "compression-sort-startup-functions">, HelpText<"TODO">, Group<grp_lld>;
-def no_compression_sort_startup_functions: Flag<["--"], "no-compression-sort-startup-functions">, HelpText<"TODO">, Group<grp_lld>;
+def compression_sort_startup_functions: Flag<["--"], "compression-sort-startup-functions">,
+    HelpText<"Order startup functions to improve compressed size in addition to startup time">,
+    Group<grp_lld>;
+def no_compression_sort_startup_functions: Flag<["--"], "no-compression-sort-startup-functions">,
+    HelpText<"Do not order startup function for compression">, Group<grp_lld>;
 def compression_sort: Joined<["--"], "compression-sort=">,
     MetaVarName<"[none,function,data,both]">,
     HelpText<"Order sections to improve compressed size">, Group<grp_lld>;
diff --git a/lld/test/MachO/bp-section-orderer-errs.s b/lld/test/MachO/bp-section-orderer-errs.s
index 8392b88508481f..682eb0c08bf1f9 100644
--- a/lld/test/MachO/bp-section-orderer-errs.s
+++ b/lld/test/MachO/bp-section-orderer-errs.s
@@ -7,3 +7,6 @@
 
 # RUN: not %lld -o /dev/null --compression-sort=malformed 2>&1 | FileCheck %s --check-prefix=COMPRESSION-MALFORM
 # COMPRESSION-MALFORM: unknown value `malformed` for --compression-sort=
+
+# RUN: not %lld -o /dev/null --compression-sort-startup-functions 2>&1 | FileCheck %s --check-prefix=STARTUP
+# STARTUP: --compression-sort-startup-functions must be used with --irpgo-profile-sort

>From 40f945f5dce72d14a39ed0c22671bdf97082e4e5 Mon Sep 17 00:00:00 2001
From: Ellis Hoag <ellis.sparky.hoag at gmail.com>
Date: Thu, 5 Sep 2024 15:08:26 -0700
Subject: [PATCH 3/3] Combine new code under flag

---
 lld/MachO/BPSectionOrderer.cpp | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/lld/MachO/BPSectionOrderer.cpp b/lld/MachO/BPSectionOrderer.cpp
index a8255294306826..07b44d48d65932 100644
--- a/lld/MachO/BPSectionOrderer.cpp
+++ b/lld/MachO/BPSectionOrderer.cpp
@@ -269,12 +269,21 @@ DenseMap<const InputSection *, size_t> lld::macho::runBalancedPartitioning(
     }
   }
 
-  SmallVector<unsigned> startupIdxs;
-  if (compressionSortStartupFunctions)
+  if (compressionSortStartupFunctions) {
+    SmallVector<unsigned> startupIdxs;
     for (auto &[sectionIdx, uns] : startupSectionIdxUNs)
       startupIdxs.push_back(sectionIdx);
-  auto unsForStartupFunctionCompression =
-      getUnsForCompression(sections, sectionToIdx, startupIdxs, nullptr, maxUN);
+    auto unsForStartupFunctionCompression =
+        getUnsForCompression(sections, sectionToIdx, startupIdxs,
+                             /*duplicateSectionIdxs=*/nullptr, maxUN);
+    for (auto &[sectionIdx, compressionUns] :
+         unsForStartupFunctionCompression) {
+      auto &uns = startupSectionIdxUNs[sectionIdx];
+      uns.append(compressionUns);
+      llvm::sort(uns);
+      uns.erase(std::unique(uns.begin(), uns.end()), uns.end());
+    }
+  }
 
   // Map a section index (order directly) to a list of duplicate section indices
   // (not ordered directly).
@@ -286,12 +295,6 @@ DenseMap<const InputSection *, size_t> lld::macho::runBalancedPartitioning(
       sections, sectionToIdx, sectionIdxsForDataCompression,
       &duplicateSectionIdxs, maxUN);
 
-  for (auto &[sectionIdx, compressionUns] : unsForStartupFunctionCompression) {
-    auto &uns = startupSectionIdxUNs[sectionIdx];
-    uns.append(compressionUns);
-    llvm::sort(uns);
-    uns.erase(std::unique(uns.begin(), uns.end()), uns.end());
-  }
   std::vector<BPFunctionNode> nodesForStartup, nodesForFunctionCompression,
       nodesForDataCompression;
   for (auto &[sectionIdx, uns] : startupSectionIdxUNs)



More information about the llvm-commits mailing list