[PATCH] D43102: Refactor how we decide which sections to sort

[Rafael Avila de Espindola via Phabricator via llvm-commits]

espindola created this revision.
espindola added reviewers: ruiu, Bigcheese.
Herald added a subscriber: emaste.

This is a bit more verbose, but it has a few advantages.

The logic on what to do with special sections like .init_array is not duplicated. Before we would need keep isKnownNonreorderableSection in sync.

I think with this the call graph based sorting can be implemented by "just" returning a new order from buildSectionOrder.


https://reviews.llvm.org/D43102

Files:
  ELF/Writer.cpp


Index: ELF/Writer.cpp
===================================================================
--- ELF/Writer.cpp
+++ ELF/Writer.cpp
@@ -1048,40 +1048,45 @@
   return SectionOrder;
 }
 
-static bool isKnownNonreorderableSection(const OutputSection *OS) {
-  return llvm::StringSwitch<bool>(OS->Name)
-      .Cases(".init", ".fini", ".init_array", ".fini_array", ".ctors",
-             ".dtors", true)
-      .Default(false);
-}
-
-// If no layout was provided by linker script, we want to apply default
-// sorting for special input sections. This also handles --symbol-ordering-file.
-template <class ELFT> void Writer<ELFT>::sortInputSections() {
-  // Sort input sections by priority using the list provided
-  // by --symbol-ordering-file.
-  DenseMap<SectionBase *, int> Order = buildSectionOrder();
-  if (!Order.empty())
-    for (BaseCommand *Base : Script->SectionCommands)
-      if (auto *Sec = dyn_cast<OutputSection>(Base))
-        if (Sec->Live && !isKnownNonreorderableSection(Sec))
-          Sec->sort([&](InputSectionBase *S) { return Order.lookup(S); });
-
-  if (Script->HasSectionsCommand)
+static void sortSection(OutputSection *Sec,
+                        const DenseMap<SectionBase *, int> &Order) {
+  if (!Sec->Live)
     return;
+  StringRef Name = Sec->Name;
 
   // Sort input sections by section name suffixes for
   // __attribute__((init_priority(N))).
-  if (OutputSection *Sec = findSection(".init_array"))
-    Sec->sortInitFini();
-  if (OutputSection *Sec = findSection(".fini_array"))
-    Sec->sortInitFini();
+  if (Name == ".init_array" || Name == ".fini_array") {
+    if (!Script->HasSectionsCommand)
+      Sec->sortInitFini();
+    return;
+  }
 
   // Sort input sections by the special rule for .ctors and .dtors.
-  if (OutputSection *Sec = findSection(".ctors"))
-    Sec->sortCtorsDtors();
-  if (OutputSection *Sec = findSection(".dtors"))
-    Sec->sortCtorsDtors();
+  if (Name == ".ctors" || Name == ".dtors") {
+    if (!Script->HasSectionsCommand)
+      Sec->sortCtorsDtors();
+    return;
+  }
+
+  // Never sort these.
+  if (Name == ".init" || Name == ".fini")
+    return;
+
+  // Sort input sections by priority using the list provided
+  // by --symbol-ordering-file.
+  if (!Order.empty())
+    Sec->sort([&](InputSectionBase *S) { return Order.lookup(S); });
+}
+
+// If no layout was provided by linker script, we want to apply default
+// sorting for special input sections. This also handles --symbol-ordering-file.
+template <class ELFT> void Writer<ELFT>::sortInputSections() {
+  // Build the order once since it is expensive.
+  DenseMap<SectionBase *, int> Order = buildSectionOrder();
+  for (BaseCommand *Base : Script->SectionCommands)
+    if (auto *Sec = dyn_cast<OutputSection>(Base))
+      sortSection(Sec, Order);
 }
 
 template <class ELFT> void Writer<ELFT>::sortSections() {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43102.133536.patch
Type: text/x-patch
Size: 2859 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180209/6edd45ba/attachment.bin>