[PATCH] D44716: [lld] fix data race in ICF.cpp

[Bob Haarman via Phabricator via llvm-commits]

inglorion updated this revision to Diff 139226.
inglorion added a comment.

Removed double computation of boundaries and special case for I == 0.


https://reviews.llvm.org/D44716

Files:
  lld/COFF/ICF.cpp


Index: lld/COFF/ICF.cpp
===================================================================
--- lld/COFF/ICF.cpp
+++ lld/COFF/ICF.cpp
@@ -168,6 +168,7 @@
   return std::equal(A->Relocs.begin(), A->Relocs.end(), B->Relocs.begin(), Eq);
 }
 
+// Find the first Chunk after Begin that has a different class from Begin.
 size_t ICF::findBoundary(size_t Begin, size_t End) {
   for (size_t I = Begin + 1; I < End; ++I)
     if (Chunks[Begin]->Class[Cnt % 2] != Chunks[I]->Class[Cnt % 2])
@@ -177,11 +178,8 @@
 
 void ICF::forEachClassRange(size_t Begin, size_t End,
                             std::function<void(size_t, size_t)> Fn) {
-  if (Begin > 0)
-    Begin = findBoundary(Begin - 1, End);
-
   while (Begin < End) {
-    size_t Mid = findBoundary(Begin, Chunks.size());
+    size_t Mid = findBoundary(Begin, End);
     Fn(Begin, Mid);
     Begin = Mid;
   }
@@ -197,12 +195,22 @@
     return;
   }
 
-  // Split sections into 256 shards and call Fn in parallel.
-  size_t NumShards = 256;
+  // Shard into non-overlapping intervals, and call Fn in parallel.
+  // The sharding must be completed before any calls to Fn are made
+  // so that Fn can modify the Chunks in its shard without causing data
+  // races.
+  const size_t NumShards = 256;
+  size_t Boundaries[NumShards + 1];
+  Boundaries[0] = 0;
   size_t Step = Chunks.size() / NumShards;
-  for_each_n(parallel::par, size_t(0), NumShards, [&](size_t I) {
-    size_t End = (I == NumShards - 1) ? Chunks.size() : (I + 1) * Step;
-    forEachClassRange(I * Step, End, Fn);
+  for_each_n(parallel::par, size_t(1), NumShards, [&](size_t I) {
+    Boundaries[I] = findBoundary(I * Step - 1, Chunks.size());
+  });
+  for_each_n(parallel::par, size_t(1), NumShards, [&](size_t I) {
+      if (Boundaries[I] > Boundaries[I - 1]) {
+        message(Twine("Shard ") + Twine(Boundaries[I - 1]) + ":" + Twine(Boundaries[I]));
+        forEachClassRange(Boundaries[I - 1], Boundaries[I], Fn);
+      }
   });
   ++Cnt;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44716.139226.patch
Type: text/x-patch
Size: 1975 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180321/06b18474/attachment.bin>