[lld] a52b910 - [lld-macho] Allow order files and call graph sorting to be used together

Leonard Grey via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 17 11:20:19 PST 2022


Author: Leonard Grey
Date: 2022-02-17T14:19:34-05:00
New Revision: a52b9102d1f75ca0229e5e395d317fb9ecd51590

URL: https://github.com/llvm/llvm-project/commit/a52b9102d1f75ca0229e5e395d317fb9ecd51590
DIFF: https://github.com/llvm/llvm-project/commit/a52b9102d1f75ca0229e5e395d317fb9ecd51590.diff

LOG: [lld-macho] Allow order files and call graph sorting to be used together

If both an order file and a call graph profile are present, the edges of the
call graph which use symbols present in the order file are not used. All of
the symbols in the order file will appear at the beginning of the section just
as they do currently. In other words, the highest priority derived from the
call graph will be below the lowest priority derived from the order file.

Practically, this change renames CallGraphSort.{h,cpp} to SectionPriorities.{h,cpp},
and most order file and call graph profile related code is moved into the new
file to reduce duplication.

Differential Revision: https://reviews.llvm.org/D117354

Added: 
    lld/test/MachO/cgprofile-orderfile.s

Modified: 
    lld/MachO/Driver.cpp
    lld/MachO/SectionPriorities.cpp
    lld/MachO/SectionPriorities.h

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 790f31a44a3b9..aca794ffa41b6 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -460,20 +460,6 @@ static void addFileList(StringRef path, bool isLazy) {
     addFile(rerootPath(path), ForceLoad::Default, isLazy);
 }
 
-// An order file has one entry per line, in the following format:
-//
-//   <cpu>:<object file>:<symbol name>
-//
-// <cpu> and <object file> are optional. If not specified, then that entry
-// matches any symbol of that name. Parsing this format is not quite
-// straightforward because the symbol name itself can contain colons, so when
-// encountering a colon, we consider the preceding characters to decide if it
-// can be a valid CPU type or file path.
-//
-// If a symbol is matched by multiple entries, then it takes the lowest-ordered
-// entry (the one nearest to the front of the list.)
-//
-// The file can also have line comments that start with '#'.
 // We expect sub-library names of the form "libfoo", which will match a dylib
 // with a path of .*/libfoo.{dylib, tbd}.
 // XXX ld64 seems to ignore the extension entirely when matching sub-libraries;
@@ -1461,10 +1447,8 @@ bool macho::link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
     replaceCommonSymbols();
 
     StringRef orderFile = args.getLastArgValue(OPT_order_file);
-    if (!orderFile.empty()) {
+    if (!orderFile.empty())
       parseOrderFile(orderFile);
-      config->callGraphProfileSort = false;
-    }
 
     referenceStubBinder();
 

diff  --git a/lld/MachO/SectionPriorities.cpp b/lld/MachO/SectionPriorities.cpp
index 35510d7338e89..405cb23c42c53 100644
--- a/lld/MachO/SectionPriorities.cpp
+++ b/lld/MachO/SectionPriorities.cpp
@@ -16,6 +16,7 @@
 #include "InputFiles.h"
 #include "Symbols.h"
 #include "Target.h"
+
 #include "lld/Common/Args.h"
 #include "lld/Common/CommonLinkerContext.h"
 #include "lld/Common/ErrorHandler.h"
@@ -34,6 +35,9 @@ using namespace lld;
 using namespace lld::macho;
 
 namespace {
+
+size_t lowestPriority = std::numeric_limits<size_t>::max();
+
 struct Edge {
   int from;
   uint64_t weight;
@@ -208,7 +212,7 @@ DenseMap<const InputSection *, size_t> CallGraphSort::run() {
   // priority 0 and be placed at the end of sections.
   // NB: This is opposite from COFF/ELF to be compatible with the existing
   // order-file code.
-  int curOrder = clusters.size();
+  int curOrder = lowestPriority;
   for (int leader : sorted) {
     for (int i = leader;;) {
       orderMap[sections[i]] = curOrder--;
@@ -247,8 +251,17 @@ DenseMap<const InputSection *, size_t> CallGraphSort::run() {
   return orderMap;
 }
 
-static size_t getSymbolPriority(const SymbolPriorityEntry &entry,
-                                const InputFile *f) {
+static Optional<size_t> getSymbolPriority(const Defined *sym) {
+  if (sym->isAbsolute())
+    return None;
+
+  auto it = config->priorities.find(sym->getName());
+  if (it == config->priorities.end())
+    return None;
+  const SymbolPriorityEntry &entry = it->second;
+  const InputFile *f = sym->isec->getFile();
+  if (!f)
+    return entry.anyObjectFile;
   // We don't use toString(InputFile *) here because it returns the full path
   // for object files, and we only want the basename.
   StringRef filename;
@@ -262,6 +275,7 @@ static size_t getSymbolPriority(const SymbolPriorityEntry &entry,
 
 void macho::extractCallGraphProfile() {
   TimeTraceScope timeScope("Extract call graph profile");
+  bool hasOrderFile = !config->priorities.empty();
   for (const InputFile *file : inputFiles) {
     auto *obj = dyn_cast_or_null<ObjFile>(file);
     if (!obj)
@@ -271,8 +285,9 @@ void macho::extractCallGraphProfile() {
              entry.toIndex < obj->symbols.size());
       auto *fromSym = dyn_cast_or_null<Defined>(obj->symbols[entry.fromIndex]);
       auto *toSym = dyn_cast_or_null<Defined>(obj->symbols[entry.toIndex]);
-
-      if (!fromSym || !toSym)
+      if (!fromSym || !toSym ||
+          (hasOrderFile &&
+           (getSymbolPriority(fromSym) || getSymbolPriority(toSym))))
         continue;
       config->callGraphProfile[{fromSym->isec, toSym->isec}] += entry.count;
     }
@@ -280,6 +295,8 @@ void macho::extractCallGraphProfile() {
 }
 
 void macho::parseOrderFile(StringRef path) {
+  assert(config->callGraphProfile.empty() &&
+         "Order file must be parsed before call graph profile is processed");
   Optional<MemoryBufferRef> buffer = readFile(path);
   if (!buffer) {
     error("Could not read order file at " + path);
@@ -331,6 +348,7 @@ void macho::parseOrderFile(StringRef path) {
 
     --priority;
   }
+  lowestPriority = priority;
 }
 
 // Sort sections by the profile data provided by __LLVM,__cg_profile sections.
@@ -343,28 +361,20 @@ static DenseMap<const InputSection *, size_t> computeCallGraphProfileOrder() {
   return CallGraphSort().run();
 }
 
-// Each section gets assigned the priority of the highest-priority symbol it
-// contains.
 DenseMap<const InputSection *, size_t> macho::buildInputSectionPriorities() {
-  if (config->callGraphProfileSort)
-    return computeCallGraphProfileOrder();
   DenseMap<const InputSection *, size_t> sectionPriorities;
+  if (config->callGraphProfileSort)
+    sectionPriorities = computeCallGraphProfileOrder();
 
   if (config->priorities.empty())
     return sectionPriorities;
 
-  auto addSym = [&](Defined &sym) {
-    if (sym.isAbsolute())
-      return;
-
-    auto it = config->priorities.find(sym.getName());
-    if (it == config->priorities.end())
+  auto addSym = [&](const Defined *sym) {
+    Optional<size_t> symbolPriority = getSymbolPriority(sym);
+    if (!symbolPriority.hasValue())
       return;
-
-    SymbolPriorityEntry &entry = it->second;
-    size_t &priority = sectionPriorities[sym.isec];
-    priority =
-        std::max(priority, getSymbolPriority(entry, sym.isec->getFile()));
+    size_t &priority = sectionPriorities[sym->isec];
+    priority = std::max(priority, symbolPriority.getValue());
   };
 
   // TODO: Make sure this handles weak symbols correctly.
@@ -372,7 +382,7 @@ DenseMap<const InputSection *, size_t> macho::buildInputSectionPriorities() {
     if (isa<ObjFile>(file))
       for (Symbol *sym : file->symbols)
         if (auto *d = dyn_cast_or_null<Defined>(sym))
-          addSym(*d);
+          addSym(d);
   }
 
   return sectionPriorities;

diff  --git a/lld/MachO/SectionPriorities.h b/lld/MachO/SectionPriorities.h
index 9cc4eff958cd7..1557cc4747d93 100644
--- a/lld/MachO/SectionPriorities.h
+++ b/lld/MachO/SectionPriorities.h
@@ -44,7 +44,8 @@ void parseOrderFile(StringRef path);
 //
 // If either an order file or a call graph profile are present, this is used
 // as the source of priorities. If both are present, the order file takes
-// precedence. If neither is present, an empty map is returned.
+// precedence, but the call graph profile is still used for symbols that don't
+// appear in the order file. If neither is present, an empty map is returned.
 //
 // Each section gets assigned the priority of the highest-priority symbol it
 // contains.

diff  --git a/lld/test/MachO/cgprofile-orderfile.s b/lld/test/MachO/cgprofile-orderfile.s
new file mode 100644
index 0000000000000..eb3a30b27e1bd
--- /dev/null
+++ b/lld/test/MachO/cgprofile-orderfile.s
@@ -0,0 +1,50 @@
+# REQUIRES: x86
+
+# RUN: rm -rf %t; split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
+
+# RUN: %lld -e A %t/test.o -order_file %t/order_file -o %t/test 
+# RUN: llvm-nm --numeric-sort %t/test | FileCheck %s
+# RUN: %lld -e A %t/test.o -o %t/test 
+# RUN: llvm-nm --numeric-sort %t/test | FileCheck %s --check-prefix NO-ORDER
+
+
+#--- order_file
+B
+A
+
+#--- test.s
+
+.text
+    .globl  D
+D:
+    retq
+
+    .globl  C
+C:
+    retq
+
+    .globl  B
+B:
+    retq
+
+    .globl  A
+A:
+    retq
+
+.cg_profile A, B, 100
+.cg_profile A, C,  40
+.cg_profile C, D,  61
+
+.subsections_via_symbols
+
+# CHECK:      T B
+# CHECK-NEXT: T A
+# CHECK-NEXT: T C
+# CHECK-NEXT: T D
+
+# NO-ORDER:      T A
+# NO-ORDER-NEXT: T B
+# NO-ORDER-NEXT: T C
+# NO-ORDER-NEXT: T D
+


        


More information about the llvm-commits mailing list