Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lld][MachO] rename to bp-* options for SectionOrderer #118594

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

Colibrow
Copy link
Contributor

@Colibrow Colibrow commented Dec 4, 2024

Rename options related to profile guided function order (#96268) to prepare for the addition to the ELF port.

Copy link

github-actions bot commented Dec 4, 2024

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2024

@llvm/pr-subscribers-lld-macho
@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Max (Colibrow)

Changes

Overview

  • Added new profile-guided optimization options to ELF linker
  • Renamed existing options in MachO linker for consistency

Related in options.td both in ELF and MachO

  • irpgo_profile
  • bp_compression_sort_startup_functions
  • bp_compression_sort
  • verbose_bp_section_orderer

Full diff: https://github.com/llvm/llvm-project/pull/118594.diff

11 Files Affected:

  • (modified) lld/ELF/Config.h (+5)
  • (modified) lld/ELF/Driver.cpp (+36)
  • (modified) lld/ELF/Options.td (+14)
  • (modified) lld/MachO/Config.h (+5-5)
  • (modified) lld/MachO/Driver.cpp (+14-15)
  • (modified) lld/MachO/Options.td (+6-6)
  • (modified) lld/MachO/SectionPriorities.cpp (+5-6)
  • (added) lld/test/ELF/bp-section-orderer-err.s (+44)
  • (modified) lld/test/MachO/bp-section-orderer-errs.s (+9-9)
  • (modified) lld/test/MachO/bp-section-orderer-stress.s (+2-2)
  • (modified) lld/test/MachO/bp-section-orderer.s (+7-7)
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index a2836733c2715e..793f791cb2f647 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -264,6 +264,11 @@ struct Config {
   bool armBe8 = false;
   BsymbolicKind bsymbolic = BsymbolicKind::None;
   CGProfileSortKind callGraphProfileSort;
+  llvm::StringRef irpgoProfilePath;
+  bool bpCompressionSortStartupFunctions = false;
+  bool bpFunctionOrderForCompression = false;
+  bool bpDataOrderForCompression = false;
+  bool bpVerboseSectionOrderer = false;
   bool checkSections;
   bool checkDynamicRelocs;
   std::optional<llvm::DebugCompressionType> compressDebugSections;
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index bc4b967ccbbbb4..aa7bd06010537a 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1255,6 +1255,42 @@ static void readConfigs(Ctx &ctx, opt::InputArgList &args) {
       ctx.arg.bsymbolic = BsymbolicKind::All;
   }
   ctx.arg.callGraphProfileSort = getCGProfileSortKind(ctx, args);
+  ctx.arg.irpgoProfilePath = args.getLastArgValue(OPT_irpgo_profile);
+  ctx.arg.bpCompressionSortStartupFunctions =
+      args.hasFlag(OPT_bp_compression_sort_startup_functions,
+                   OPT_no_bp_compression_sort_startup_functions, false);
+  if (!ctx.arg.irpgoProfilePath.empty()) {
+    if (args.getLastArg(OPT_call_graph_ordering_file) != nullptr) {
+      ErrAlways(ctx) << "--irpgo-profile is incompatible with "
+                        "--call-graph-ordering-file";
+    }
+  } else {
+    if (ctx.arg.bpCompressionSortStartupFunctions)
+      ErrAlways(ctx) << "--bp-compression-sort-startup-functions must be used with "
+                        "--irpgo-profile";
+  }
+
+  if (auto *arg = args.getLastArg(OPT_bp_compression_sort)) {
+    StringRef compressionSortStr = arg->getValue();
+    if (compressionSortStr == "function") {
+      ctx.arg.bpFunctionOrderForCompression = true;
+    } else if (compressionSortStr == "data") {
+      ctx.arg.bpDataOrderForCompression = true;
+    } else if (compressionSortStr == "both") {
+      ctx.arg.bpFunctionOrderForCompression = true;
+      ctx.arg.bpDataOrderForCompression = true;
+    } else if (compressionSortStr != "none") {
+      ErrAlways(ctx) << "unknown value '" + compressionSortStr + "' for " +
+                            arg->getSpelling();
+    }
+    if (ctx.arg.bpDataOrderForCompression || ctx.arg.bpFunctionOrderForCompression) {
+      if (args.getLastArg(OPT_call_graph_ordering_file) != nullptr) {
+        ErrAlways(ctx) << "--bp-compression-sort is incompatible with "
+                          "--call-graph-ordering-file";
+      }
+    }
+  }
+  ctx.arg.bpVerboseSectionOrderer = args.hasArg(OPT_verbose_bp_section_orderer);
   ctx.arg.checkSections =
       args.hasFlag(OPT_check_sections, OPT_no_check_sections, true);
   ctx.arg.chroot = args.getLastArgValue(OPT_chroot);
diff --git a/lld/ELF/Options.td b/lld/ELF/Options.td
index ebe77204264210..fd35e2a6a2c3ff 100644
--- a/lld/ELF/Options.td
+++ b/lld/ELF/Options.td
@@ -141,6 +141,20 @@ def call_graph_profile_sort: JJ<"call-graph-profile-sort=">,
 def : FF<"no-call-graph-profile-sort">, Alias<call_graph_profile_sort>, AliasArgs<["none"]>,
   Flags<[HelpHidden]>;
 
+defm irpgo_profile: Eq<"irpgo-profile",
+    "Read the IRPGO profile at <profile>">;
+
+defm bp_compression_sort_startup_functions: BB<"bp-compression-sort-startup-functions",
+    "Order startup functions by balanced partition to improve compressed size in addition to startup time",
+    "Do not order startup function for compression">;
+
+def bp_compression_sort: JJ<"bp-compression-sort=">,
+    MetaVarName<"[none,function,data,both]">,
+    HelpText<"Order sections by balanced partition to improve compressed size">;
+
+def verbose_bp_section_orderer: FF<"verbose-bp-section-orderer">,
+    HelpText<"Print information on how many sections were ordered by balanced partitioning and a measure of the expected number of page faults">;
+
 // --chroot doesn't have a help text because it is an internal option.
 def chroot: Separate<["--"], "chroot">;
 
diff --git a/lld/MachO/Config.h b/lld/MachO/Config.h
index 60f76d12141040..c2061b40074fb1 100644
--- a/lld/MachO/Config.h
+++ b/lld/MachO/Config.h
@@ -222,11 +222,11 @@ struct Configuration {
   bool callGraphProfileSort = false;
   llvm::StringRef printSymbolOrder;
 
-  llvm::StringRef irpgoProfileSortProfilePath;
-  bool compressionSortStartupFunctions = false;
-  bool functionOrderForCompression = false;
-  bool dataOrderForCompression = false;
-  bool verboseBpSectionOrderer = false;
+  llvm::StringRef irpgoProfilePath;
+  bool bpCompressionSortStartupFunctions = false;
+  bool bpFunctionOrderForCompression = false;
+  bool bpDataOrderForCompression = false;
+  bool bpVerboseSectionOrderer = false;
 
   SectionRenameMap sectionRenameMap;
   SegmentRenameMap segmentRenameMap;
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index c95841d3a8adee..3b414606a54a59 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1838,26 +1838,25 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
       if (const Arg *arg = args.getLastArgNoClaim(OPT_call_graph_profile_sort))
         error(firstArgStr + " is incompatible with " + arg->getSpelling());
   };
-  if (const Arg *arg = args.getLastArg(OPT_irpgo_profile_sort)) {
-    config->irpgoProfileSortProfilePath = arg->getValue();
+  if (const Arg *arg = args.getLastArg(OPT_irpgo_profile)) {
+    config->irpgoProfilePath = arg->getValue();
     IncompatWithCGSort(arg->getSpelling());
   }
-  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)) {
+  config->bpCompressionSortStartupFunctions =
+      args.hasFlag(OPT_bp_compression_sort_startup_functions,
+                   OPT_no_bp_compression_sort_startup_functions, false);
+  if (config->irpgoProfilePath.empty() && config->bpCompressionSortStartupFunctions)
+    error("--bp-compression-sort-startup-functions must be used with "
+          "--irpgo-profile");
+  if (const Arg *arg = args.getLastArg(OPT_bp_compression_sort)) {
     StringRef compressionSortStr = arg->getValue();
     if (compressionSortStr == "function") {
-      config->functionOrderForCompression = true;
+      config->bpFunctionOrderForCompression = true;
     } else if (compressionSortStr == "data") {
-      config->dataOrderForCompression = true;
+      config->bpDataOrderForCompression = true;
     } else if (compressionSortStr == "both") {
-      config->functionOrderForCompression = true;
-      config->dataOrderForCompression = true;
+      config->bpFunctionOrderForCompression = true;
+      config->bpDataOrderForCompression = true;
     } else if (compressionSortStr != "none") {
       error("unknown value `" + compressionSortStr + "` for " +
             arg->getSpelling());
@@ -1865,7 +1864,7 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
     if (compressionSortStr != "none")
       IncompatWithCGSort(arg->getSpelling());
   }
-  config->verboseBpSectionOrderer = args.hasArg(OPT_verbose_bp_section_orderer);
+  config->bpVerboseSectionOrderer = args.hasArg(OPT_verbose_bp_section_orderer);
 
   for (const Arg *arg : args.filtered(OPT_alias)) {
     config->aliasedSymbols.push_back(
diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td
index 485e5968ff556c..f04e78f9132e8a 100644
--- a/lld/MachO/Options.td
+++ b/lld/MachO/Options.td
@@ -126,17 +126,17 @@ def no_call_graph_profile_sort : Flag<["--"], "no-call-graph-profile-sort">,
 def print_symbol_order_eq: Joined<["--"], "print-symbol-order=">,
     HelpText<"Print a symbol order specified by --call-graph-profile-sort into the specified file">,
     Group<grp_lld>;
-def irpgo_profile_sort: Separate<["--"], "irpgo-profile-sort">, Group<grp_lld>;
-def irpgo_profile_sort_eq: Joined<["--"], "irpgo-profile-sort=">,
-    Alias<!cast<Separate>(irpgo_profile_sort)>, MetaVarName<"<profile>">,
+def irpgo_profile: Separate<["--"], "irpgo-profile">, Group<grp_lld>;
+def irpgo_profile_eq: Joined<["--"], "irpgo-profile=">,
+    Alias<!cast<Separate>(irpgo_profile)>, 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">,
+def bp_compression_sort_startup_functions: Flag<["--"], "bp-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">,
+def no_bp_compression_sort_startup_functions: Flag<["--"], "no-bp-compression-sort-startup-functions">,
     HelpText<"Do not order startup function for compression">, Group<grp_lld>;
-def compression_sort: Joined<["--"], "compression-sort=">,
+def bp_compression_sort: Joined<["--"], "bp-compression-sort=">,
     MetaVarName<"[none,function,data,both]">,
     HelpText<"Order sections to improve compressed size">, Group<grp_lld>;
 def verbose_bp_section_orderer: Flag<["--"], "verbose-bp-section-orderer">,
diff --git a/lld/MachO/SectionPriorities.cpp b/lld/MachO/SectionPriorities.cpp
index 1e7fb5973b8086..1aaa3311452da8 100644
--- a/lld/MachO/SectionPriorities.cpp
+++ b/lld/MachO/SectionPriorities.cpp
@@ -353,14 +353,13 @@ void macho::PriorityBuilder::parseOrderFile(StringRef path) {
 DenseMap<const InputSection *, size_t>
 macho::PriorityBuilder::buildInputSectionPriorities() {
   DenseMap<const InputSection *, size_t> sectionPriorities;
-  if (!config->irpgoProfileSortProfilePath.empty() ||
-      config->functionOrderForCompression || config->dataOrderForCompression) {
+  if (!config->irpgoProfilePath.empty() ||
+      config->bpFunctionOrderForCompression || config->bpDataOrderForCompression) {
     TimeTraceScope timeScope("Balanced Partitioning Section Orderer");
     sectionPriorities = runBalancedPartitioning(
-        highestAvailablePriority, config->irpgoProfileSortProfilePath,
-        config->functionOrderForCompression, config->dataOrderForCompression,
-        config->compressionSortStartupFunctions,
-        config->verboseBpSectionOrderer);
+        highestAvailablePriority, config->irpgoProfilePath,
+        config->bpFunctionOrderForCompression, config->bpDataOrderForCompression,
+        config->bpCompressionSortStartupFunctions, config->bpVerboseSectionOrderer);
   } else if (config->callGraphProfileSort) {
     // Sort sections by the profile data provided by __LLVM,__cg_profile
     // sections.
diff --git a/lld/test/ELF/bp-section-orderer-err.s b/lld/test/ELF/bp-section-orderer-err.s
new file mode 100644
index 00000000000000..c45d46085d8fcc
--- /dev/null
+++ b/lld/test/ELF/bp-section-orderer-err.s
@@ -0,0 +1,44 @@
+# REQUIRES: aarch64
+
+# RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t
+# RUN: echo "A B 5" > %t.call_graph
+# RUN: echo "B C 50" >> %t.call_graph
+# RUN: echo "C D 40" >> %t.call_graph
+# RUN: echo "D B 10" >> %t.call_graph
+# RUN: not ld.lld -o /dev/null %t --irpgo-profile %s --call-graph-ordering-file=%t.call_graph 2>&1 | FileCheck %s --check-prefix=IRPGO-ERR
+# RUN: not ld.lld -o /dev/null %t --irpgo-profile=%s --call-graph-ordering-file=%t.call_graph 2>&1 | FileCheck %s --check-prefix=IRPGO-ERR
+# IRPGO-ERR: --irpgo-profile is incompatible with --call-graph-ordering-file
+
+# RUN: not ld.lld -o /dev/null --bp-compression-sort=function --call-graph-ordering-file %t.call_graph 2>&1 | FileCheck %s --check-prefix=COMPRESSION-ERR
+# COMPRESSION-ERR: --bp-compression-sort is incompatible with --call-graph-ordering-file
+
+# RUN: not ld.lld -o /dev/null --bp-compression-sort=malformed 2>&1 | FileCheck %s --check-prefix=COMPRESSION-MALFORM
+# COMPRESSION-MALFORM: unknown value 'malformed' for --bp-compression-sort=
+
+# RUN: not ld.lld -o /dev/null --bp-compression-sort-startup-functions 2>&1 | FileCheck %s --check-prefix=STARTUP
+# STARTUP: --bp-compression-sort-startup-functions must be used with --irpgo-profile
+
+# CHECK: B
+# CHECK-NEXT: C
+# CHECK-NEXT: D
+# CHECK-NEXT: A
+
+.section    .text.A,"ax",@progbits
+.globl  A
+A:
+ nop
+
+.section    .text.B,"ax",@progbits
+.globl  B
+B:
+ nop
+
+.section    .text.C,"ax",@progbits
+.globl  C
+C:
+ nop
+
+.section    .text.D,"ax",@progbits
+.globl  D
+D:
+ nop
diff --git a/lld/test/MachO/bp-section-orderer-errs.s b/lld/test/MachO/bp-section-orderer-errs.s
index 682eb0c08bf1f9..09099536417e9b 100644
--- a/lld/test/MachO/bp-section-orderer-errs.s
+++ b/lld/test/MachO/bp-section-orderer-errs.s
@@ -1,12 +1,12 @@
-# RUN: not %lld -o /dev/null --irpgo-profile-sort %s --call-graph-profile-sort 2>&1 | FileCheck %s --check-prefix=IRPGO-ERR
-# RUN: not %lld -o /dev/null --irpgo-profile-sort=%s --call-graph-profile-sort 2>&1 | FileCheck %s --check-prefix=IRPGO-ERR
-# IRPGO-ERR: --irpgo-profile-sort is incompatible with --call-graph-profile-sort
+# RUN: not %lld -o /dev/null --irpgo-profile %s --call-graph-profile-sort 2>&1 | FileCheck %s --check-prefix=IRPGO-ERR
+# RUN: not %lld -o /dev/null --irpgo-profile=%s --call-graph-profile-sort 2>&1 | FileCheck %s --check-prefix=IRPGO-ERR
+# IRPGO-ERR: --irpgo-profile is incompatible with --call-graph-profile-sort
 
-# RUN: not %lld -o /dev/null --compression-sort=function --call-graph-profile-sort %s 2>&1 | FileCheck %s --check-prefix=COMPRESSION-ERR
-# COMPRESSION-ERR: --compression-sort= is incompatible with --call-graph-profile-sort
+# RUN: not %lld -o /dev/null --bp-compression-sort=function --call-graph-profile-sort %s 2>&1 | FileCheck %s --check-prefix=COMPRESSION-ERR
+# COMPRESSION-ERR: --bp-compression-sort= is incompatible with --call-graph-profile-sort
 
-# 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 --bp-compression-sort=malformed 2>&1 | FileCheck %s --check-prefix=COMPRESSION-MALFORM
+# COMPRESSION-MALFORM: unknown value `malformed` for --bp-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
+# RUN: not %lld -o /dev/null --bp-compression-sort-startup-functions 2>&1 | FileCheck %s --check-prefix=STARTUP
+# STARTUP: --bp-compression-sort-startup-functions must be used with --irpgo-profile
diff --git a/lld/test/MachO/bp-section-orderer-stress.s b/lld/test/MachO/bp-section-orderer-stress.s
index 986e2d8fd1069b..82300187f7841e 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-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: %lld -arch arm64 -lSystem -e _main --icf=all -o - %t.o --irpgo-profile=%t.profdata --bp-compression-sort-startup-functions --bp-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=%t.profdata --bp-compression-sort-startup-functions --bp-compression-sort=both | llvm-nm --numeric-sort --format=just-symbols - > %t.order2.txt
 # RUN: diff %t.order1.txt %t.order2.txt
 
 import random
diff --git a/lld/test/MachO/bp-section-orderer.s b/lld/test/MachO/bp-section-orderer.s
index 407787025150d2..4f89fbe994adc2 100644
--- a/lld/test/MachO/bp-section-orderer.s
+++ b/lld/test/MachO/bp-section-orderer.s
@@ -4,12 +4,12 @@
 # RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/a.s -o %t/a.o
 # RUN: llvm-profdata merge %t/a.proftext -o %t/a.profdata
 
-# RUN: %lld -arch arm64 -lSystem -e _main -o %t/a.out %t/a.o --irpgo-profile-sort=%t/a.profdata --verbose-bp-section-orderer 2>&1 | FileCheck %s --check-prefix=STARTUP
-# RUN: %lld -arch arm64 -lSystem -e _main -o %t/a.out %t/a.o --irpgo-profile-sort=%t/a.profdata --verbose-bp-section-orderer --icf=all --compression-sort=none 2>&1 | FileCheck %s --check-prefix=STARTUP
+# RUN: %lld -arch arm64 -lSystem -e _main -o %t/a.out %t/a.o --irpgo-profile=%t/a.profdata --verbose-bp-section-orderer 2>&1 | FileCheck %s --check-prefix=STARTUP
+# RUN: %lld -arch arm64 -lSystem -e _main -o %t/a.out %t/a.o --irpgo-profile=%t/a.profdata --verbose-bp-section-orderer --icf=all --bp-compression-sort=none 2>&1 | FileCheck %s --check-prefix=STARTUP
 
 # STARTUP: Ordered 3 sections using balanced partitioning
 
-# RUN: %lld -arch arm64 -lSystem -e _main -o - %t/a.o --irpgo-profile-sort=%t/a.profdata -order_file %t/a.orderfile | llvm-nm --numeric-sort --format=just-symbols - | FileCheck %s --check-prefix=ORDERFILE
+# RUN: %lld -arch arm64 -lSystem -e _main -o - %t/a.o --irpgo-profile=%t/a.profdata -order_file %t/a.orderfile | llvm-nm --numeric-sort --format=just-symbols - | FileCheck %s --check-prefix=ORDERFILE
 
 # ORDERFILE: A
 # ORDERFILE: F
@@ -23,10 +23,10 @@
 # ORDERFILE-DAG: r1
 # ORDERFILE-DAG: r2
 
-# RUN: %lld -arch arm64 -lSystem -e _main -o %t/a.out %t/a.o --verbose-bp-section-orderer --compression-sort=function 2>&1 | FileCheck %s --check-prefix=COMPRESSION-FUNC
-# RUN: %lld -arch arm64 -lSystem -e _main -o %t/a.out %t/a.o --verbose-bp-section-orderer --compression-sort=data 2>&1 | FileCheck %s --check-prefix=COMPRESSION-DATA
-# RUN: %lld -arch arm64 -lSystem -e _main -o %t/a.out %t/a.o --verbose-bp-section-orderer --compression-sort=both 2>&1 | FileCheck %s --check-prefix=COMPRESSION-BOTH
-# RUN: %lld -arch arm64 -lSystem -e _main -o %t/a.out %t/a.o --verbose-bp-section-orderer --compression-sort=both --irpgo-profile-sort=%t/a.profdata 2>&1 | FileCheck %s --check-prefix=COMPRESSION-BOTH
+# RUN: %lld -arch arm64 -lSystem -e _main -o %t/a.out %t/a.o --verbose-bp-section-orderer --bp-compression-sort=function 2>&1 | FileCheck %s --check-prefix=COMPRESSION-FUNC
+# RUN: %lld -arch arm64 -lSystem -e _main -o %t/a.out %t/a.o --verbose-bp-section-orderer --bp-compression-sort=data 2>&1 | FileCheck %s --check-prefix=COMPRESSION-DATA
+# RUN: %lld -arch arm64 -lSystem -e _main -o %t/a.out %t/a.o --verbose-bp-section-orderer --bp-compression-sort=both 2>&1 | FileCheck %s --check-prefix=COMPRESSION-BOTH
+# RUN: %lld -arch arm64 -lSystem -e _main -o %t/a.out %t/a.o --verbose-bp-section-orderer --bp-compression-sort=both --irpgo-profile=%t/a.profdata 2>&1 | FileCheck %s --check-prefix=COMPRESSION-BOTH
 
 # COMPRESSION-FUNC: Ordered 7 sections using balanced partitioning
 # COMPRESSION-DATA: Ordered 4 sections using balanced partitioning

lld/MachO/Options.td Outdated Show resolved Hide resolved
lld/MachO/Options.td Show resolved Hide resolved
lld/test/ELF/bp-section-orderer-err.s Outdated Show resolved Hide resolved
@Colibrow
Copy link
Contributor Author

Colibrow commented Dec 5, 2024

@ellishg hi ellis, I've fixed these. could you please review again?

lld/MachO/Driver.cpp Outdated Show resolved Hide resolved
lld/MachO/Options.td Show resolved Hide resolved
lld/MachO/Driver.cpp Show resolved Hide resolved
lld/MachO/Options.td Show resolved Hide resolved
lld/MachO/Options.td Show resolved Hide resolved
lld/test/ELF/incompatible.s Outdated Show resolved Hide resolved
lld/test/MachO/bp-section-orderer-errs.s Outdated Show resolved Hide resolved
lld/test/ELF/incompatible.s Outdated Show resolved Hide resolved
lld/test/MachO/bp-section-orderer.s Outdated Show resolved Hide resolved
lld/test/MachO/bp-section-orderer.s Outdated Show resolved Hide resolved
ellishg added a commit that referenced this pull request Dec 6, 2024
…ver BP (#118889)

When both `-order_file` and `--irpgo-profile-sort=` (soon to be
`-bp-startup-sort=function` in
#118594) are used, we want to
confirm that symbols in the orderfile take precedence.
@MaskRay
Copy link
Member

MaskRay commented Dec 6, 2024

The ELF change should be reverted. The purpose of the PR is to add some bp- options and make existing options (deployed by Meta) aliases (for compatibility). The ELF port should not be touched as it doesn't have the functionality yet.

@Colibrow Colibrow force-pushed the lld-bp-options branch 2 times, most recently from 9fe2da8 to 9685525 Compare December 6, 2024 06:41
@Colibrow
Copy link
Contributor Author

Colibrow commented Dec 6, 2024

@MaskRay @ellishg Hi, I've removed the ELF parts(which will be added in another PR where it being used). And fixed the problems. Could you please review this PR?

@Colibrow Colibrow changed the title [lld][ELF][MachO] rename to bp-* options for SectionOrderer [lld][MachO] rename to bp-* options for SectionOrderer Dec 6, 2024
Copy link
Contributor

@ellishg ellishg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing all our comments. I'm starting to like this set of flags a lot more than before

lld/MachO/Driver.cpp Outdated Show resolved Hide resolved
lld/MachO/SectionPriorities.cpp Outdated Show resolved Hide resolved
lld/MachO/SectionPriorities.cpp Outdated Show resolved Hide resolved
lld/test/MachO/bp-section-orderer-stress.s Outdated Show resolved Hide resolved
lld/MachO/Driver.cpp Show resolved Hide resolved
lld/MachO/Driver.cpp Outdated Show resolved Hide resolved
lld/MachO/Options.td Outdated Show resolved Hide resolved
Copy link
Contributor

@ellishg ellishg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Colibrow
Copy link
Contributor Author

Colibrow commented Dec 10, 2024

@thevinster hi, can you please review this?

@Colibrow Colibrow requested a review from thevinster December 10, 2024 03:39
Copy link
Contributor

@thevinster thevinster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Colibrow Colibrow force-pushed the lld-bp-options branch 2 times, most recently from 544553d to bd3f4c7 Compare December 10, 2024 06:27
@Colibrow
Copy link
Contributor Author

@MaskRay @ellishg Hi, Can you merge this PR? I don't have the write access.

@MaskRay MaskRay merged commit a295907 into llvm:main Dec 10, 2024
4 of 7 checks passed
Copy link

@Colibrow Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

@MaskRay
Copy link
Member

MaskRay commented Dec 10, 2024

Note: the patch description is used as the default commit message, so please make sure to update it after the patch has been rewritten. I've changed the description.

broxigarchen pushed a commit to broxigarchen/llvm-project that referenced this pull request Dec 10, 2024
…ver BP (llvm#118889)

When both `-order_file` and `--irpgo-profile-sort=` (soon to be
`-bp-startup-sort=function` in
llvm#118594) are used, we want to
confirm that symbols in the orderfile take precedence.
broxigarchen pushed a commit to broxigarchen/llvm-project that referenced this pull request Dec 10, 2024
Rename options related to profile guided function order (llvm#96268) to
prepare for the addition to the ELF port.
chrsmcgrr pushed a commit to RooflineAI/llvm-project that referenced this pull request Dec 12, 2024
…ver BP (llvm#118889)

When both `-order_file` and `--irpgo-profile-sort=` (soon to be
`-bp-startup-sort=function` in
llvm#118594) are used, we want to
confirm that symbols in the orderfile take precedence.
TIFitis pushed a commit to TIFitis/llvm-project that referenced this pull request Dec 18, 2024
…ver BP (llvm#118889)

When both `-order_file` and `--irpgo-profile-sort=` (soon to be
`-bp-startup-sort=function` in
llvm#118594) are used, we want to
confirm that symbols in the orderfile take precedence.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants