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

[Bob Haarman via Phabricator via llvm-commits]

inglorion marked 2 inline comments as done.
inglorion added inline comments.


================
Comment at: lld/COFF/ICF.cpp:205-208
   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);
+    size_t Begin = I == 0 ? 0 : findBoundary(I * Step - 1, Chunks.size());
+    Boundaries[I] = findBoundary(Begin, Chunks.size());
+  });
----------------
ruiu wrote:
> This doubles the total number of computation because you call findBoundary calls twice for each iteration. I wonder if we need to parallelize this in the first place. Maybe it is better to do this with a regular for loop?
Ah, yeah. I meant to make this call findBoundary once per shard. Fixed in new upload.


================
Comment at: lld/COFF/ICF.cpp:210-213
+    if (I == 0)
+      forEachClassRange(0, Boundaries[I], Fn);
+    else if (Boundaries[I] > Boundaries[I - 1])
+      forEachClassRange(Boundaries[I - 1], Boundaries[I], Fn);
----------------
ruiu wrote:
> I would allocate NumShards+1 elements for Boundaries and set Boundaries[0] to 0, so that we don't need to special-case I==0.
Good idea! Done.


https://reviews.llvm.org/D44716