<div dir="ltr">Is there a reason you're not using scc_iterator?<div><br></div><div>-- Sean Silva</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 10, 2015 at 9:29 PM, Rui Ueyama via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ruiu<br>
Date: Thu Sep 10 23:29:03 2015<br>
New Revision: 247387<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=247387&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=247387&view=rev</a><br>
Log:<br>
COFF: Teach ICF to merge cyclic graphs.<br>
<br>
Previously, LLD's ICF couldn't merge cyclic graphs. That was unfortunate<br>
because, in COFF, cyclic graphs are not exceptional at all. That is<br>
pretty common.<br>
<br>
In this patch, sections are grouped by Tarjan's strongly connected<br>
component algorithm to get acyclic graphs. And then we try to merge<br>
SCCs whose outdegree is zero, and remove them from the graph. This<br>
makes other SCCs to have outdegree zero, so we can repeat the<br>
process until all SCCs are removed. When comparing two SCCs, we handle<br>
cycles properly.<br>
<br>
This algorithm works better than previous one. Previously, self-linking<br>
produced a 29.0MB executable. It now produces a 27.7MB. There's still some<br>
gap compared to MSVC linker which produces a 27.1MB executable for the<br>
same input. So the gap is narrowed, but still LLD is not on par with MSVC.<br>
I'll investigate that later.<br>
<br>
Modified:<br>
    lld/trunk/COFF/Chunks.h<br>
    lld/trunk/COFF/ICF.cpp<br>
    lld/trunk/test/COFF/icf-circular.test<br>
<br>
Modified: lld/trunk/COFF/Chunks.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.h?rev=247387&r1=247386&r2=247387&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.h?rev=247387&r1=247386&r2=247387&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/Chunks.h (original)<br>
+++ lld/trunk/COFF/Chunks.h Thu Sep 10 23:29:03 2015<br>
@@ -111,6 +111,18 @@ protected:<br>
   uint32_t Align = 1;<br>
 };<br>
<br>
+class SectionChunk;<br>
+<br>
+// A container of SectionChunks. Used by ICF to store computation<br>
+// results of strongly connected components. You can ignore this<br>
+// unless you are interested in ICF.<br>
+struct Component {<br>
+  Component(std::vector<SectionChunk *> V) : Members(V) {}<br>
+  std::vector<SectionChunk *> Members;<br>
+  std::vector<Component *> Predecessors;<br>
+  int Outdegree = 0;<br>
+};<br>
+<br>
 // A chunk corresponding a section of an input file.<br>
 class SectionChunk : public Chunk {<br>
 public:<br>
@@ -182,12 +194,15 @@ public:<br>
   // with other chunk by ICF, it points to another chunk,<br>
   // and this chunk is considrered as dead.<br>
   SectionChunk *Ptr;<br>
-  int Outdegree = 0;<br>
-  std::vector<SectionChunk *> Ins;<br>
+  uint32_t Index = 0;<br>
+  uint32_t LowLink = 0;<br>
+  bool OnStack = false;<br>
+  Component *SCC = nullptr;<br>
<br>
   // The CRC of the contents as described in the COFF spec 4.5.5.<br>
   // Auxiliary Format 5: Section Definitions. Used for ICF.<br>
   uint32_t Checksum = 0;<br>
+  mutable uint64_t Hash = 0;<br>
<br>
 private:<br>
   ArrayRef<uint8_t> getContents() const;<br>
<br>
Modified: lld/trunk/COFF/ICF.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/ICF.cpp?rev=247387&r1=247386&r2=247387&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/ICF.cpp?rev=247387&r1=247386&r2=247387&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/ICF.cpp (original)<br>
+++ lld/trunk/COFF/ICF.cpp Thu Sep 10 23:29:03 2015<br>
@@ -7,7 +7,31 @@<br>
 //<br>
 //===----------------------------------------------------------------------===//<br>
 //<br>
-// Implements ICF (Identical COMDAT Folding)<br>
+// Identical COMDAT Folding is a feature to merge COMDAT sections not by<br>
+// name (which is regular COMDAT handling) but by contents. If two COMDAT<br>
+// sections have the same data, relocations, attributes, etc., then the two<br>
+// are considered identical and merged by the linker. This optimization<br>
+// makes outputs smaller.<br>
+//<br>
+// ICF is theoretically a problem of reducing graphs by merging as many<br>
+// isomorphic subgraphs as possible, if we consider sections as vertices and<br>
+// relocations as edges. This may be a bit more complicated problem than you<br>
+// might think. The order of processing sections matters since merging two<br>
+// sections can make other sections, whose relocations now point to the<br>
+// section, mergeable. Graphs may contain cycles, which is common in COFF.<br>
+// We need a sophisticated algorithm to do this properly and efficiently.<br>
+//<br>
+// What we do in this file is this. We first compute strongly connected<br>
+// components of the graphs to get acyclic graphs. Then, we remove SCCs whose<br>
+// outdegree is zero from the graphs and try to merge them. This operation<br>
+// makes other SCCs to have outdegree zero, so we repeat the process until<br>
+// all SCCs are removed.<br>
+//<br>
+// This algorithm is different from what GNU gold does which is described in<br>
+// <a href="http://research.google.com/pubs/pub36912.html" rel="noreferrer" target="_blank">http://research.google.com/pubs/pub36912.html</a>. I don't know which is<br>
+// faster, this or Gold's, in practice. It'd be interesting to implement the<br>
+// other algorithm to compare. Note that the gold's algorithm cannot handle<br>
+// cycles, so we need to tweak it, though.<br>
 //<br>
 //===----------------------------------------------------------------------===//<br>
<br>
@@ -15,6 +39,10 @@<br>
 #include "Symbols.h"<br>
 #include "llvm/ADT/Hashing.h"<br>
 #include "llvm/ADT/STLExtras.h"<br>
+#include "llvm/Support/Debug.h"<br>
+#include "llvm/Support/raw_ostream.h"<br>
+#include <algorithm><br>
+#include <functional><br>
 #include <tuple><br>
 #include <unordered_set><br>
 #include <vector><br>
@@ -37,15 +65,156 @@ struct Equals {<br>
<br>
 } // anonymous namespace<br>
<br>
+// Invoke Fn for each live COMDAT successor sections of SC.<br>
+static void forEach(SectionChunk *SC, std::function<void(SectionChunk *)> Fn) {<br>
+  for (SectionChunk *C : SC->children())<br>
+    Fn(C);<br>
+  for (SymbolBody *B : SC->symbols()) {<br>
+    if (auto *D = dyn_cast<DefinedRegular>(B)) {<br>
+      SectionChunk *C = D->getChunk();<br>
+      if (C->isCOMDAT() && C->isLive())<br>
+        Fn(C);<br>
+    }<br>
+  }<br>
+}<br>
+<br>
+typedef std::vector<Component *>::iterator ComponentIterator;<br>
+<br>
+// Try to merge two SCCs, A and B. A and B are likely to be isomorphic<br>
+// because all sections have the same hash values.<br>
+static void tryMerge(std::vector<SectionChunk *> &A,<br>
+                     std::vector<SectionChunk *> &B) {<br>
+  // Assume that relocation targets are the same.<br>
+  size_t End = A.size();<br>
+  for (size_t I = 0; I != End; ++I) {<br>
+    assert(B[I] == B[I]->Ptr);<br>
+    B[I]->Ptr = A[I];<br>
+  }<br>
+  for (size_t I = 0; I != End; ++I) {<br>
+    if (A[I]->equals(B[I]))<br>
+      continue;<br>
+    // If we reach here, the assumption was wrong. Reset the pointers<br>
+    // to the original values and terminate the comparison.<br>
+    for (size_t I = 0; I != End; ++I)<br>
+      B[I]->Ptr = B[I];<br>
+    return;<br>
+  }<br>
+  // If we reach here, the assumption was correct. Actually replace them.<br>
+  for (size_t I = 0; I != End; ++I)<br>
+    B[I]->replaceWith(A[I]);<br>
+}<br>
+<br>
+// Try to merge components. All components given to this function are<br>
+// guaranteed to have the same number of members.<br>
+static void doUniquefy(ComponentIterator Begin, ComponentIterator End) {<br>
+  // Sort component members by hash value.<br>
+  for (auto It = Begin; It != End; ++It) {<br>
+    Component *SCC = *It;<br>
+    auto Comp = [](SectionChunk *A, SectionChunk *B) {<br>
+      return A->getHash() < B->getHash();<br>
+    };<br>
+    std::sort(SCC->Members.begin(), SCC->Members.end(), Comp);<br>
+  }<br>
+<br>
+  // Merge as much component members as possible.<br>
+  for (auto It = Begin; It != End;) {<br>
+    Component *SCC = *It;<br>
+    auto Bound = std::partition(It + 1, End, [&](Component *C) {<br>
+      for (size_t I = 0, E = SCC->Members.size(); I != E; ++I)<br>
+        if (SCC->Members[I]->getHash() != C->Members[I]->getHash())<br>
+          return false;<br>
+      return true;<br>
+    });<br>
+<br>
+    // Components [I, Bound) are likely to have the same members<br>
+    // because all members have the same hash values. Verify that.<br>
+    for (auto I = It + 1; I != Bound; ++I)<br>
+      tryMerge(SCC->Members, (*I)->Members);<br>
+    It = Bound;<br>
+  }<br>
+}<br>
+<br>
+static void uniquefy(ComponentIterator Begin, ComponentIterator End) {<br>
+  for (auto It = Begin; It != End;) {<br>
+    Component *SCC = *It;<br>
+    size_t Size = SCC->Members.size();<br>
+    auto Bound = std::partition(It + 1, End, [&](Component *C) {<br>
+      return C->Members.size() == Size;<br>
+    });<br>
+    doUniquefy(It, Bound);<br>
+    It = Bound;<br>
+  }<br>
+}<br>
+<br>
+// Returns strongly connected components of the graph formed by Chunks.<br>
+// Chunks (a list of Live COMDAT sections) are considred as vertices,<br>
+// and their relocations or association are considered as edges.<br>
+static std::vector<Component *><br>
+getSCC(const std::vector<SectionChunk *> &Chunks) {<br>
+  std::vector<Component *> Ret;<br>
+  std::vector<SectionChunk *> V;<br>
+  uint32_t Idx;<br>
+<br>
+  std::function<void(SectionChunk *)> StrongConnect = [&](SectionChunk *SC) {<br>
+    SC->Index = SC->LowLink = Idx++;<br>
+    size_t Curr = V.size();<br>
+    V.push_back(SC);<br>
+    SC->OnStack = true;<br>
+<br>
+    forEach(SC, [&](SectionChunk *C) {<br>
+      if (C->Index == 0) {<br>
+        StrongConnect(C);<br>
+        SC->LowLink = std::min(SC->LowLink, C->LowLink);<br>
+      } else if (C->OnStack) {<br>
+        SC->LowLink = std::min(SC->LowLink, C->Index);<br>
+      }<br>
+    });<br>
+<br>
+    if (SC->LowLink != SC->Index)<br>
+      return;<br>
+    auto *SCC = new Component(<br>
+        std::vector<SectionChunk *>(V.begin() + Curr, V.end()));<br>
+    for (size_t I = Curr, E = V.size(); I != E; ++I) {<br>
+      V[I]->OnStack = false;<br>
+      V[I]->SCC = SCC;<br>
+    }<br>
+    Ret.push_back(SCC);<br>
+    V.erase(V.begin() + Curr, V.end());<br>
+  };<br>
+<br>
+  for (SectionChunk *SC : Chunks) {<br>
+    if (SC->Index == 0) {<br>
+      Idx = 1;<br>
+      StrongConnect(SC);<br>
+    }<br>
+  }<br>
+<br>
+  for (Component *SCC : Ret) {<br>
+    for (SectionChunk *SC : SCC->Members) {<br>
+      forEach(SC, [&](SectionChunk *C) {<br>
+        if (SCC == C->SCC)<br>
+          return;<br>
+        ++SCC->Outdegree;<br>
+        C->SCC->Predecessors.push_back(SCC);<br>
+      });<br>
+    }<br>
+  }<br>
+  return Ret;<br>
+}<br>
+<br>
 uint64_t SectionChunk::getHash() const {<br>
-  return hash_combine(getPermissions(),<br>
-                      hash_value(SectionName),<br>
-                      NumRelocs,<br>
-                      uint32_t(Header->SizeOfRawData),<br>
-                      std::distance(Relocs.end(), Relocs.begin()),<br>
-                      Checksum);<br>
+  if (Hash == 0) {<br>
+    Hash = hash_combine(getPermissions(),<br>
+                        hash_value(SectionName),<br>
+                        NumRelocs,<br>
+                        uint32_t(Header->SizeOfRawData),<br>
+                        std::distance(Relocs.end(), Relocs.begin()),<br>
+                        Checksum);<br>
+  }<br>
+  return Hash;<br>
 }<br>
<br>
+<br>
 // Returns true if this and a given chunk are identical COMDAT sections.<br>
 bool SectionChunk::equals(const SectionChunk *X) const {<br>
   // Compare headers<br>
@@ -90,28 +259,6 @@ bool SectionChunk::equals(const SectionC<br>
   return std::equal(Relocs.begin(), Relocs.end(), X->Relocs.begin(), Eq);<br>
 }<br>
<br>
-static void link(SectionChunk *From, SectionChunk *To) {<br>
-  ++From->Outdegree;<br>
-  To->Ins.push_back(From);<br>
-}<br>
-<br>
-typedef std::vector<SectionChunk *>::iterator ChunkIterator;<br>
-<br>
-static void uniquefy(ChunkIterator Begin, ChunkIterator End) {<br>
-  std::unordered_set<SectionChunk *, Hasher, Equals> Set;<br>
-  for (auto It = Begin; It != End; ++It) {<br>
-    SectionChunk *SC = *It;<br>
-    auto P = Set.insert(SC);<br>
-    bool Inserted = P.second;<br>
-    if (Inserted)<br>
-      continue;<br>
-    SectionChunk *Existing = *P.first;<br>
-    SC->replaceWith(Existing);<br>
-    for (SectionChunk *In : SC->Ins)<br>
-      --In->Outdegree;<br>
-  }<br>
-}<br>
-<br>
 // Merge identical COMDAT sections.<br>
 // Two sections are considered the same if their section headers,<br>
 // contents and relocations are all the same.<br>
@@ -122,26 +269,19 @@ void doICF(const std::vector<Chunk *> &C<br>
       if (SC->isCOMDAT() && SC->isLive())<br>
         SChunks.push_back(SC);<br>
<br>
-  // Initialize SectionChunks' outdegrees and in-chunk lists.<br>
-  for (SectionChunk *SC : SChunks) {<br>
-    for (SectionChunk *C : SC->children())<br>
-      link(SC, C);<br>
-    for (SymbolBody *B : SC->symbols())<br>
-      if (auto *D = dyn_cast<DefinedRegular>(B))<br>
-        link(SC, D->getChunk());<br>
-  }<br>
-<br>
-  // By merging two sections, more sections can become mergeable<br>
-  // because two originally different relocations can now point to<br>
-  // the same section. We process sections whose outdegree is zero<br>
-  // first to deal with that.<br>
-  auto Pred = [](SectionChunk *SC) { return SC->Outdegree > 0; };<br>
-  for (;;) {<br>
-    auto Bound = std::partition(SChunks.begin(), SChunks.end(), Pred);<br>
-    if (Bound == SChunks.end())<br>
-      return;<br>
-    uniquefy(Bound, SChunks.end());<br>
-    SChunks.erase(Bound, SChunks.end());<br>
+  std::vector<Component *> Components = getSCC(SChunks);<br>
+<br>
+  while (Components.size() > 0) {<br>
+    auto Bound = std::partition(Components.begin(), Components.end(),<br>
+                                [](Component *SCC) { return SCC->Outdegree > 0; });<br>
+    uniquefy(Bound, Components.end());<br>
+<br>
+    for (auto It = Bound, E = Components.end(); It != E; ++It) {<br>
+      Component *SCC = *It;<br>
+      for (Component *Pred : SCC->Predecessors)<br>
+        --Pred->Outdegree;<br>
+    }<br>
+    Components.erase(Bound, Components.end());<br>
   }<br>
 }<br>
<br>
<br>
Modified: lld/trunk/test/COFF/icf-circular.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/icf-circular.test?rev=247387&r1=247386&r2=247387&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/icf-circular.test?rev=247387&r1=247386&r2=247387&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/test/COFF/icf-circular.test (original)<br>
+++ lld/trunk/test/COFF/icf-circular.test Thu Sep 10 23:29:03 2015<br>
@@ -3,7 +3,7 @@<br>
 # RUN:   /opt:lldicf /verbose %t.obj > %t.log 2>&1<br>
 # RUN: FileCheck %s < %t.log<br>
<br>
-# CHECK-NOT: Replaced bar<br>
+# CHECK: Replaced bar<br>
<br>
 ---<br>
 header:<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>