[lld] r358645 - [ELF] Place SectionPiece::{Live, Hash} bit fields together

[Fangrui Song via llvm-commits]

Author: maskray
Date: Thu Apr 18 00:46:09 2019
New Revision: 358645

URL: http://llvm.org/viewvc/llvm-project?rev=358645&view=rev
Log:
[ELF] Place SectionPiece::{Live,Hash} bit fields together

Summary:
We access Live and OutputOff (which may share the same memory location)
concurrently in 2 parallelForEachN loops. Separating them avoids subtle
data races like D41884/PR35788. This patch places Live and Hash
together.

2 reasons this is appealing:

1) Hash is immutable. Live is almost read-only - only written once in MarkLive.cpp where
   Hash is not accessed
2) we already discard low bits of Hash to decide ShardID. It doesn't
   matter much if we make 32-bit Hash to 31-bit.

   For a huge internal clang -O3 executable (1.6GiB),
   `Strings` in StringTableBuilder::finalizeStringTable contains at most 310253 elements.
   The expected number of pair-wise collisions 2^(-31) * C(310253,2) ~= 22.41 is too small to have a negative impact on performance.
   Actually, my benchmark shows there is actually a minor performance improvement.

Differential Revision: https://reviews.llvm.org/D60765

Modified:
    lld/trunk/ELF/InputSection.h
    lld/trunk/ELF/SyntheticSections.cpp
    lld/trunk/ELF/SyntheticSections.h

Modified: lld/trunk/ELF/InputSection.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.h?rev=358645&r1=358644&r2=358645&view=diff
==============================================================================
--- lld/trunk/ELF/InputSection.h (original)
+++ lld/trunk/ELF/InputSection.h Thu Apr 18 00:46:09 2019
@@ -227,13 +227,12 @@ protected:
 // be found by looking at the next one).
 struct SectionPiece {
   SectionPiece(size_t Off, uint32_t Hash, bool Live)
-      : InputOff(Off), Hash(Hash), OutputOff(0),
-        Live(Live || !Config->GcSections) {}
+      : InputOff(Off), Live(Live || !Config->GcSections), Hash(Hash >> 1) {}
 
   uint32_t InputOff;
-  uint32_t Hash;
-  int64_t OutputOff : 63;
-  uint64_t Live : 1;
+  uint32_t Live : 1;
+  uint32_t Hash : 31;
+  uint64_t OutputOff = 0;
 };
 
 static_assert(sizeof(SectionPiece) == 16, "SectionPiece is too big");

Modified: lld/trunk/ELF/SyntheticSections.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SyntheticSections.cpp?rev=358645&r1=358644&r2=358645&view=diff
==============================================================================
--- lld/trunk/ELF/SyntheticSections.cpp (original)
+++ lld/trunk/ELF/SyntheticSections.cpp Thu Apr 18 00:46:09 2019
@@ -2876,8 +2876,10 @@ void MergeNoTailSection::finalizeContent
   parallelForEachN(0, Concurrency, [&](size_t ThreadId) {
     for (MergeInputSection *Sec : Sections) {
       for (size_t I = 0, E = Sec->Pieces.size(); I != E; ++I) {
+        if (!Sec->Pieces[I].Live)
+          continue;
         size_t ShardId = getShardId(Sec->Pieces[I].Hash);
-        if ((ShardId & (Concurrency - 1)) == ThreadId && Sec->Pieces[I].Live)
+        if ((ShardId & (Concurrency - 1)) == ThreadId)
           Sec->Pieces[I].OutputOff = Shards[ShardId].add(Sec->getData(I));
       }
     }

Modified: lld/trunk/ELF/SyntheticSections.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SyntheticSections.h?rev=358645&r1=358644&r2=358645&view=diff
==============================================================================
--- lld/trunk/ELF/SyntheticSections.h (original)
+++ lld/trunk/ELF/SyntheticSections.h Thu Apr 18 00:46:09 2019
@@ -859,7 +859,8 @@ private:
   // If we use lower bits, it significantly increases the probability of
   // hash collisons.
   size_t getShardId(uint32_t Hash) {
-    return Hash >> (32 - llvm::countTrailingZeros(NumShards));
+    assert((Hash >> 31) == 0);
+    return Hash >> (31 - llvm::countTrailingZeros(NumShards));
   }
 
   // Section size