[lld] a271f24 - [lld-macho][nfc] Canonicalize all pointers to InputSections early on

[Jez Ng via llvm-commits]

Author: Jez Ng
Date: 2021-10-29T11:00:28-04:00
New Revision: a271f2410f6c6ec367a12b74aef426239b96f70b

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

LOG: [lld-macho][nfc] Canonicalize all pointers to InputSections early on

Having to remember to call `canonical()` all over the place is
error-prone; let's do it in a centralized location instead. It also
appears to improve performance slightly.

             base           diff           difference (95% CI)
  sys_time   0.984 ± 0.009  0.983 ± 0.014  [  -0.8% ..   +0.6%]
  user_time  6.508 ± 0.035  6.475 ± 0.036  [  -0.8% ..   -0.2%]
  wall_time  5.321 ± 0.034  5.300 ± 0.033  [  -0.7% ..   -0.1%]
  samples    36             23

Reviewed By: #lld-macho, thakis

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

Added: 
    

Modified: 
    lld/MachO/InputSection.h
    lld/MachO/Symbols.cpp
    lld/MachO/Symbols.h
    lld/MachO/SyntheticSections.cpp
    lld/MachO/Writer.cpp

Removed: 
    


################################################################################
diff  --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h
index cc09a494ec9b..85036135570a 100644
--- a/lld/MachO/InputSection.h
+++ b/lld/MachO/InputSection.h
@@ -122,7 +122,7 @@ class ConcatInputSection final : public InputSection {
   void writeTo(uint8_t *buf);
 
   void foldIdentical(ConcatInputSection *redundant);
-  InputSection *canonical() override {
+  ConcatInputSection *canonical() override {
     return replacement ? replacement : this;
   }
 
@@ -131,7 +131,7 @@ class ConcatInputSection final : public InputSection {
   }
 
   // Points to the surviving section after this one is folded by ICF
-  InputSection *replacement = nullptr;
+  ConcatInputSection *replacement = nullptr;
   // Equivalence-class ID for ICF
   uint64_t icfEqClass[2] = {0, 0};
 

diff  --git a/lld/MachO/Symbols.cpp b/lld/MachO/Symbols.cpp
index 2385a7ca96ed..c663b21dd7d9 100644
--- a/lld/MachO/Symbols.cpp
+++ b/lld/MachO/Symbols.cpp
@@ -66,7 +66,7 @@ uint64_t Defined::getVA() const {
   if (isAbsolute())
     return value;
 
-  if (!isec->canonical()->isFinal) {
+  if (!isec->isFinal) {
     // A target arch that does not use thunks ought never ask for
     // the address of a function that has not yet been finalized.
     assert(target->usesThunks());
@@ -77,7 +77,14 @@ uint64_t Defined::getVA() const {
     // expedient to return a contrived out-of-range address.
     return TargetInfo::outOfRangeVA;
   }
-  return isec->canonical()->getVA(value);
+  return isec->getVA(value);
+}
+
+void Defined::canonicalize() {
+  if (compactUnwind)
+    compactUnwind = compactUnwind->canonical();
+  if (isec)
+    isec = isec->canonical();
 }
 
 uint64_t DylibSymbol::getVA() const {

diff  --git a/lld/MachO/Symbols.h b/lld/MachO/Symbols.h
index 01c60c4d75d7..54f1a3512728 100644
--- a/lld/MachO/Symbols.h
+++ b/lld/MachO/Symbols.h
@@ -126,6 +126,10 @@ class Defined : public Symbol {
 
   uint64_t getVA() const override;
 
+  // Ensure this symbol's pointers to InputSections point to their canonical
+  // copies.
+  void canonicalize();
+
   static bool classof(const Symbol *s) { return s->kind() == DefinedKind; }
 
   InputSection *isec;

diff  --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index c5dc5e6b9e5a..8a3a4aa658a9 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -924,7 +924,7 @@ void SymtabSection::emitStabs() {
     }
 
     StabsEntry symStab;
-    symStab.sect = defined->isec->canonical()->parent->index;
+    symStab.sect = defined->isec->parent->index;
     symStab.strx = stringTableSection.addString(defined->getName());
     symStab.value = defined->getVA();
 
@@ -1049,7 +1049,7 @@ template <class LP> void SymtabSectionImpl<LP>::writeTo(uint8_t *buf) const {
         nList->n_value = defined->value;
       } else {
         nList->n_type = scope | N_SECT;
-        nList->n_sect = defined->isec->canonical()->parent->index;
+        nList->n_sect = defined->isec->parent->index;
         // For the N_SECT symbol type, n_value is the address of the symbol
         nList->n_value = defined->getVA();
       }

diff  --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index c2aef4d55400..4c01398a6d57 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -671,10 +671,11 @@ void Writer::scanRelocations() {
 
 void Writer::scanSymbols() {
   TimeTraceScope timeScope("Scan symbols");
-  for (const Symbol *sym : symtab->getSymbols()) {
-    if (const auto *defined = dyn_cast<Defined>(sym)) {
+  for (Symbol *sym : symtab->getSymbols()) {
+    if (auto *defined = dyn_cast<Defined>(sym)) {
       if (!defined->isLive())
         continue;
+      defined->canonicalize();
       if (defined->overridesWeakDef)
         in.weakBinding->addNonWeakDefinition(defined);
       if (!defined->isAbsolute() && isCodeSection(defined->isec))
@@ -691,10 +692,14 @@ void Writer::scanSymbols() {
   for (const InputFile *file : inputFiles) {
     if (auto *objFile = dyn_cast<ObjFile>(file))
       for (Symbol *sym : objFile->symbols) {
-        if (auto *defined = dyn_cast_or_null<Defined>(sym))
-          if (!defined->isExternal() && defined->isLive() &&
-              !defined->isAbsolute() && isCodeSection(defined->isec))
+        if (auto *defined = dyn_cast_or_null<Defined>(sym)) {
+          if (!defined->isLive())
+            continue;
+          defined->canonicalize();
+          if (!defined->isExternal() && !defined->isAbsolute() &&
+              isCodeSection(defined->isec))
             in.unwindInfo->addSymbol(defined);
+        }
       }
   }
 }
@@ -1120,6 +1125,8 @@ template <class LP> void Writer::run() {
   treatSpecialUndefineds();
   if (config->entry && !isa<Undefined>(config->entry))
     prepareBranchTarget(config->entry);
+  // Canonicalization of all pointers to InputSections should be handled by
+  // these two methods.
   scanSymbols();
   scanRelocations();