[PATCH] D60765: [ELF] Place SectionPiece::{Live,Hash} bit fields together

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: ruiu, grimar, pcc.
Herald added subscribers: llvm-commits, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.

We read Live and write to OutputOff simultaneously in 3 places.
Separating them avoids data sharing and data races like D41884 <https://reviews.llvm.org/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 discard another bit.

Because all the use sites of OutputOff expect uint64_t/size, change its
type from int64_t to uint64_t.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D60765

Files:
  ELF/InputSection.h
  ELF/SyntheticSections.cpp
  ELF/SyntheticSections.h


Index: ELF/SyntheticSections.h
===================================================================
--- ELF/SyntheticSections.h
+++ ELF/SyntheticSections.h
@@ -862,7 +862,8 @@
   // 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
Index: ELF/SyntheticSections.cpp
===================================================================
--- ELF/SyntheticSections.cpp
+++ ELF/SyntheticSections.cpp
@@ -2935,8 +2935,10 @@
   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));
       }
     }
Index: ELF/InputSection.h
===================================================================
--- ELF/InputSection.h
+++ ELF/InputSection.h
@@ -227,13 +227,12 @@
 // 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");


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60765.195340.patch
Type: text/x-patch
Size: 1887 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190416/ed97753b/attachment.bin>