<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 18, 2015 at 2:06 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: ruiu<br>
Date: Fri Sep 18 16:06:34 2015<br>
New Revision: 248038<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=248038&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=248038&view=rev</a><br>
Log:<br>
COFF: Parallelize ICF.<br>
<br>
The LLD's ICF algorithm is highly parallelizable. This patch does that<br>
using parallel_for_each.<br>
<br>
ICF accounted for about one third of total execution time. Previously,<br>
it took 324 ms when self-hosting. Now it takes only 62 ms.<br>
<br>
Of course your mileage may vary. My machine is a beefy 24-core Xeon machine,<br>
so you may not see this much speedup. But this optimization should be<br>
effective even for 2-core machine, since I saw speedup (324 ms -> 189 ms)<br>
when setting parallelism parameter to 2.<br>
<br>
Modified:<br>
    lld/trunk/COFF/Chunks.h<br>
    lld/trunk/COFF/ICF.cpp<br>
<br>
Modified: lld/trunk/COFF/Chunks.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.h?rev=248038&r1=248037&r2=248038&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.h?rev=248038&r1=248037&r2=248038&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/Chunks.h (original)<br>
+++ lld/trunk/COFF/Chunks.h Fri Sep 18 16:06:34 2015<br>
@@ -17,6 +17,7 @@<br>
 #include "llvm/ADT/iterator.h"<br>
 #include "llvm/ADT/iterator_range.h"<br>
 #include "llvm/Object/COFF.h"<br>
+#include <atomic><br>
 #include <map><br>
 #include <vector><br>
<br>
@@ -203,7 +204,7 @@ private:<br>
<br>
   // Used for ICF (Identical COMDAT Folding)<br>
   void replaceWith(SectionChunk *Other);<br>
-  uint64_t GroupID;<br>
+  std::atomic<uint64_t> GroupID = { 0 };<br>
<br>
   // Chunks are basically unnamed chunks of bytes.<br>
   // Symbols are associated for debugging and logging purposs only.<br>
<br>
Modified: lld/trunk/COFF/ICF.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/ICF.cpp?rev=248038&r1=248037&r2=248038&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/ICF.cpp?rev=248038&r1=248037&r2=248038&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/ICF.cpp (original)<br>
+++ lld/trunk/COFF/ICF.cpp Fri Sep 18 16:06:34 2015<br>
@@ -45,6 +45,7 @@<br>
<br>
 #include "Chunks.h"<br>
 #include "Symbols.h"<br>
+#include "lld/Core/Parallel.h"<br>
 #include "llvm/ADT/Hashing.h"<br>
 #include "llvm/Support/Debug.h"<br>
 #include "llvm/Support/raw_ostream.h"<br>
@@ -56,6 +57,7 @@ using namespace llvm;<br>
 namespace lld {<br>
 namespace coff {<br>
<br>
+static const size_t NJOBS = 256;<br>
 typedef std::vector<SectionChunk *>::iterator ChunkIterator;<br>
 typedef bool (*Comparator)(const SectionChunk *, const SectionChunk *);<br>
<br>
@@ -72,7 +74,7 @@ private:<br>
   bool partition(ChunkIterator Begin, ChunkIterator End, Comparator Eq);<br>
<br>
   const std::vector<Chunk *> &Chunks;<br>
-  uint64_t NextID = 0;<br>
+  uint64_t NextID = 1;<br>
 };<br>
<br>
 // Entry point to ICF.<br>
@@ -179,51 +181,74 @@ bool ICF::forEachGroup(std::vector<Secti<br>
 // contents and relocations are all the same.<br>
 void ICF::run() {<br>
   // Collect only mergeable sections and group by hash value.<br>
-  std::vector<SectionChunk *> SChunks;<br>
-  for (Chunk *C : Chunks) {<br>
+  std::vector<std::vector<SectionChunk *>> VChunks(NJOBS);<br></blockquote><div><br></div><div>Could you move this variable down to the desired scope - after the parallel_for_each, just before the for-Chunks loop - then it'll be easier to tell there's no racy parallel use of it.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+  parallel_for_each(Chunks.begin(), Chunks.end(), [&](Chunk *C) {<br>
     if (auto *SC = dyn_cast<SectionChunk>(C)) {<br>
       bool Writable = SC->getPermissions() & llvm::COFF::IMAGE_SCN_MEM_WRITE;<br>
-      if (SC->isCOMDAT() && SC->isLive() && !Writable) {<br>
-        SChunks.push_back(SC);<br>
+      if (SC->isCOMDAT() && SC->isLive() && !Writable)<br>
         SC->GroupID = getHash(SC) | (uint64_t(1) << 63);<br>
+    }<br>
+  });<br>
+  for (Chunk *C : Chunks) {<br>
+    if (auto *SC = dyn_cast<SectionChunk>(C)) {<br>
+      if (SC->GroupID) {<br>
+        VChunks[SC->GroupID % NJOBS].push_back(SC);<br>
       } else {<br>
         SC->GroupID = NextID++;<br>
       }<br>
     }<br>
   }<br>
<br>
-  // From now on, sections in SChunks are ordered so that sections in<br>
+  // From now on, sections in Chunks are ordered so that sections in<br>
   // the same group are consecutive in the vector.<br>
-  std::sort(SChunks.begin(), SChunks.end(),<br>
-            [](SectionChunk *A, SectionChunk *B) {<br>
-              return A->GroupID < B->GroupID;<br>
-            });<br>
+  parallel_for_each(VChunks.begin(), VChunks.end(),<br>
+                    [&](std::vector<SectionChunk *> &SChunks) {<br>
+    std::sort(SChunks.begin(), SChunks.end(),<br>
+              [](SectionChunk *A, SectionChunk *B) {<br>
+                return A->GroupID < B->GroupID;<br></blockquote><div><br></div><div>This lambda is in two places - perhaps it should be written in one place (maybe SectionChunk could have op<, then we could use a "deref" variadic functor that just does the dereferencing? I could use more words to describe that if it's interesting/non-obvious).</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+              });<br>
+  });<br>
<br>
   // Split groups until we get a convergence.<br>
   int Cnt = 1;<br>
-  forEachGroup(SChunks, equalsConstant);<br>
-  while (forEachGroup(SChunks, equalsVariable))<br>
+  parallel_for_each(VChunks.begin(), VChunks.end(),<br>
+                    [&](std::vector<SectionChunk *> &SChunks) {<br>
+    forEachGroup(SChunks, equalsConstant);<br>
+  });<br>
+<br>
+  for (;;) {<br>
+    bool Redo = false;<br>
+    parallel_for_each(VChunks.begin(), VChunks.end(),<br>
+                      [&](std::vector<SectionChunk *> &SChunks) {<br>
+      if (forEachGroup(SChunks, equalsVariable))<br>
+        Redo = true;<br></blockquote><div><br></div><div>This looks like a race condition - multiple threads may be racing to write 'true' to Redo.<br><br>(a parallel_find would be a nice abstraction to have, probably)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+    });<br>
+    if (!Redo)<br>
+      break;<br>
     ++Cnt;<br>
+  }<br>
   if (Config->Verbose)<br>
     llvm::outs() << "\nICF needed " << Cnt << " iterations.\n";<br>
<br>
   // Merge sections in the same group.<br>
-  for (auto It = SChunks.begin(), End = SChunks.end(); It != End;) {<br>
-    SectionChunk *Head = *It;<br>
-    auto Bound = std::find_if(It + 1, End, [&](SectionChunk *SC) {<br>
-      return Head->GroupID != SC->GroupID;<br>
-    });<br>
-    if (std::distance(It, Bound) == 1) {</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
-      It = Bound;<br>
-      continue;<br>
-    }<br>
-    if (Config->Verbose)<br>
-      llvm::outs() << "Selected " << Head->getDebugName() << "\n";<br>
-    for (++It; It != Bound; ++It) {<br>
-      SectionChunk *SC = *It;<br>
+  for (std::vector<SectionChunk *> &SChunks : VChunks) {<br>
+    for (auto It = SChunks.begin(), End = SChunks.end(); It != End;) {<br>
+      SectionChunk *Head = *It;<br>
+      auto Bound = std::find_if(It + 1, End, [&](SectionChunk *SC) {<br>
+        return Head->GroupID != SC->GroupID;<br>
+      });<br>
+      if (std::distance(It, Bound) == 1) {<br></blockquote><div><br class="">This could be written as "It + 1 == Bound" (or "std::next(It) == Bound") which may or may not be more legible, up to you. Possibly pull out the It + 1 in the find above, and reuse the value of that expression here.<br><br>(std::distance is particularly useful for generic code, or when you're dealing with a non-random-access-iterator (one that only has increment/decrement, not arbitrary subtraction - it's perhaps more confusing than helpful in the common case like this))<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+        It = Bound;<br>
+        continue;<br>
+      }<br>
       if (Config->Verbose)<br>
-        llvm::outs() << "  Removed " << SC->getDebugName() << "\n";<br>
-      SC->replaceWith(Head);<br>
+        llvm::outs() << "Selected " << Head->getDebugName() << "\n";<br>
+      for (++It; It != Bound; ++It) {<br>
+        SectionChunk *SC = *It;<br>
+        if (Config->Verbose)<br>
+          llvm::outs() << "  Removed " << SC->getDebugName() << "\n";<br>
+        SC->replaceWith(Head);<br>
+      }<br>
     }<br>
   }<br>
 }<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></div>