[lld] r251300 - COFF: De-parallelize ICF for now.

[Rui Ueyama via llvm-commits]

Author: ruiu
Date: Mon Oct 26 11:20:00 2015
New Revision: 251300

URL: http://llvm.org/viewvc/llvm-project?rev=251300&view=rev
Log:
COFF: De-parallelize ICF for now.

There was a threading issue in the ICF code for COFF. That seems like
a venign bug in the sense that it doesn't produce an incorrect output,
but it oftentimes misses reducible sections. As a result, mergeable
sections could remain in outputs, which makes the output nondeterministic.

Basically the algorithm we are using for ICF is this: We group sections
so that identical sections will eventually be in the same group. Initially,
all sections are in one group. We split the group by relocation targets
until we get a convergence (if relocation targets are in different gruops,
the sections are different). Once a group is split, they will never be
merged.

Each section has a group ID. That variable itself is atomic, so there's
no threading issue at the level that we can use thread sanitizer.
The point is, when we split a group, we re-assign new group IDs to group
of sections. That are multiple separate writes to atomic varaibles.
Thus, splitting a group is not an atomic operation, and there's a small
chance that the other thread observes inconsistent group IDs.

Over-splitting is always "safe", so it will never create incorrect output.

I suspect that the nondeterminism stems from that point. However, I
cannot prove or fix that at this moment, so I'm going to avoid using
threads here.

Modified:
    lld/trunk/COFF/ICF.cpp

Modified: lld/trunk/COFF/ICF.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/ICF.cpp?rev=251300&r1=251299&r2=251300&view=diff
==============================================================================
--- lld/trunk/COFF/ICF.cpp (original)
+++ lld/trunk/COFF/ICF.cpp Mon Oct 26 11:20:00 2015
@@ -58,7 +58,6 @@ using namespace llvm;
 namespace lld {
 namespace coff {
 
-static const size_t NJOBS = 256;
 typedef std::vector<SectionChunk *>::iterator ChunkIterator;
 typedef bool (*Comparator)(const SectionChunk *, const SectionChunk *);
 
@@ -191,11 +190,11 @@ void ICF::run(const std::vector<Chunk *>
         SC->GroupID = getHash(SC) | (uint64_t(1) << 63);
     }
   });
-  std::vector<std::vector<SectionChunk *>> VChunks(NJOBS);
+  std::vector<SectionChunk *> Chunks;
   for (Chunk *C : Vec) {
     if (auto *SC = dyn_cast<SectionChunk>(C)) {
       if (SC->GroupID) {
-        VChunks[SC->GroupID % NJOBS].push_back(SC);
+        Chunks.push_back(SC);
       } else {
         SC->GroupID = NextID++;
       }
@@ -204,29 +203,17 @@ void ICF::run(const std::vector<Chunk *>
 
   // From now on, sections in Chunks are ordered so that sections in
   // the same group are consecutive in the vector.
-  parallel_for_each(VChunks.begin(), VChunks.end(),
-                    [&](std::vector<SectionChunk *> &Chunks) {
-    std::sort(Chunks.begin(), Chunks.end(),
-              [](SectionChunk *A, SectionChunk *B) {
-                return A->GroupID < B->GroupID;
-              });
-  });
+  std::sort(Chunks.begin(), Chunks.end(),
+            [](SectionChunk *A, SectionChunk *B) {
+              return A->GroupID < B->GroupID;
+            });
 
   // Split groups until we get a convergence.
   int Cnt = 1;
-  parallel_for_each(VChunks.begin(), VChunks.end(),
-                    [&](std::vector<SectionChunk *> &Chunks) {
-    forEachGroup(Chunks, equalsConstant);
-  });
+  forEachGroup(Chunks, equalsConstant);
 
   for (;;) {
-    std::atomic<bool> Redo(false);
-    parallel_for_each(VChunks.begin(), VChunks.end(),
-                      [&](std::vector<SectionChunk *> &Chunks) {
-      if (forEachGroup(Chunks, equalsVariable))
-        Redo = true;
-    });
-    if (!Redo)
+    if (!forEachGroup(Chunks, equalsVariable))
       break;
     ++Cnt;
   }
@@ -234,22 +221,20 @@ void ICF::run(const std::vector<Chunk *>
     llvm::outs() << "\nICF needed " << Cnt << " iterations.\n";
 
   // Merge sections in the same group.
-  for (std::vector<SectionChunk *> &Chunks : VChunks) {
-    for (auto It = Chunks.begin(), End = Chunks.end(); It != End;) {
-      SectionChunk *Head = *It++;
-      auto Bound = std::find_if(It, End, [&](SectionChunk *SC) {
-        return Head->GroupID != SC->GroupID;
-      });
-      if (It == Bound)
-        continue;
+  for (auto It = Chunks.begin(), End = Chunks.end(); It != End;) {
+    SectionChunk *Head = *It++;
+    auto Bound = std::find_if(It, End, [&](SectionChunk *SC) {
+      return Head->GroupID != SC->GroupID;
+    });
+    if (It == Bound)
+      continue;
+    if (Config->Verbose)
+      llvm::outs() << "Selected " << Head->getDebugName() << "\n";
+    while (It != Bound) {
+      SectionChunk *SC = *It++;
       if (Config->Verbose)
-        llvm::outs() << "Selected " << Head->getDebugName() << "\n";
-      while (It != Bound) {
-        SectionChunk *SC = *It++;
-        if (Config->Verbose)
-          llvm::outs() << "  Removed " << SC->getDebugName() << "\n";
-        Head->replace(SC);
-      }
+        llvm::outs() << "  Removed " << SC->getDebugName() << "\n";
+      Head->replace(SC);
     }
   }
 }