[tsan] Remove special SyncClock::kInvalidTid

[vitalybuka]

Summary

Followup for D101428.

[dvyukov]

LGTM with the Dirty size fix (if it's required).

compiler-rt/lib/tsan/rtl/tsan_clock.h

72

Does size of Dirty still equals to u64? I am not sure bitfields will be combined here.
Please add a static_assert on size.

[vitalybuka]

static_assert

compiler-rt/lib/tsan/rtl/tsan_clock.h

public:
 
   Iter begin();
   Iter end();
 
  private:
   friend class ThreadClock;
   friend class Iter;
   static const uptr kDirtyTids = 2;
-  // Full kInvalidTid won't fit into Dirty::tid.
-  static const u64 kInvalidTid = (1ull << (64 - kClkBits)) - 1;
 
   struct Dirty {
-    u64 epoch  : kClkBits;
-    u64 tid : 64 - kClkBits;  // kInvalidId if not active
+    u32 tid() const { return tid_ == kShortInvalidTid ? kInvalidTid : tid_; }
+    void set_tid(u32 tid) {
+      tid_ = tid == kInvalidTid ? kShortInvalidTid : tid;
+    }
+    u64 epoch : kClkBits;
+
+   private:
+    // Full kInvalidTid won't fit into Dirty::tid.
+    static const u64 kShortInvalidTid = (1ull << (64 - kClkBits)) - 1;
+    u64 tid_ : 64 - kClkBits;  // kInvalidId if not active
   };
 
+  static_assert(sizeof(Dirty) == 8, "Dirty is not 64bit");
+
   unsigned release_store_tid_;
   unsigned release_store_reused_;
   Dirty dirty_[kDirtyTids];
   // If size_ is 0, tab_ is nullptr.
   // If size <= 64 (kClockCount), tab_ contains pointer to an array with
   // 64 ClockElem's (ClockBlock::clock).
   // Otherwise, tab_ points to an array with up to 127 u32 elements,
   // each pointing to the second-level 512b block with 64 ClockElem's.
public:
   void ResetCached(ClockCache *c);
   void NoteGlobalAcquire(u64 v);
 
   void DebugReset();
   void DebugDump(int(*printf)(const char *s, ...));
 
  private:
   static const uptr kDirtyTids = SyncClock::kDirtyTids;
-  static const u64 kInvalidTid = SyncClock::kInvalidTid;
   // Index of the thread associated with he clock ("current thread").
   const unsigned tid_;
   const unsigned reused_;  // tid_ reuse count.
   // Current thread time when it acquired something from other threads.
   u64 last_acquire_;
 
   // Last time another thread has done a global acquire of this thread's clock.
   // It helps to avoid problem described in:

compiler-rt/lib/tsan/rtl/tsan_clock.cpp

void ThreadClock::acquire(ClockCache *c, SyncClock *src) {
   if (nclk == 0) {
     CPP_STAT_INC(StatClockAcquireEmpty);
     return;
   }
 
   bool acquired = false;
   for (unsigned i = 0; i < kDirtyTids; i++) {
     SyncClock::Dirty dirty = src->dirty_[i];
-    unsigned tid = dirty.tid;
+    unsigned tid = dirty.tid();
     if (tid != kInvalidTid) {
       if (clk_[tid] < dirty.epoch) {
         clk_[tid] = dirty.epoch;
         acquired = true;
       }
     }
   }
 
if (dst->size_ == 0 && cached_idx_ != 0) {
     // currently cached clock and release the old one. And for a shared
     // existing clock, we could replace it with the currently cached;
     // or unshare, update and cache. But, for simplicity, we currnetly reuse
     // cached clock only when the target clock is empty.
     dst->tab_ = ctx->clock_alloc.Map(cached_idx_);
     dst->tab_idx_ = cached_idx_;
     dst->size_ = cached_size_;
     dst->blocks_ = cached_blocks_;
-    CHECK_EQ(dst->dirty_[0].tid, kInvalidTid);
+    CHECK_EQ(dst->dirty_[0].tid(), kInvalidTid);
     // The cached clock is shared (immutable),
     // so this is where we store the current clock.
-    dst->dirty_[0].tid = tid_;
+    dst->dirty_[0].set_tid(tid_);
     dst->dirty_[0].epoch = clk_[tid_];
     dst->release_store_tid_ = tid_;
     dst->release_store_reused_ = reused_;
     // Rememeber that we don't need to acquire it in future.
     dst->elem(tid_).reused = reused_;
     // Grab a reference.
     atomic_fetch_add(ref_ptr(dst->tab_), 1, memory_order_relaxed);
     return;
void ThreadClock::ReleaseStore(ClockCache *c, SyncClock *dst) {
   // Note: dst can be larger than this ThreadClock.
   // This is fine since clk_ beyond size is all zeros.
   uptr i = 0;
   for (ClockElem &ce : *dst) {
     ce.epoch = clk_[i];
     ce.reused = 0;
     i++;
   }
-  for (uptr i = 0; i < kDirtyTids; i++)
-    dst->dirty_[i].tid = kInvalidTid;
+  for (uptr i = 0; i < kDirtyTids; i++) dst->dirty_[i].set_tid(kInvalidTid);
   dst->release_store_tid_ = tid_;
   dst->release_store_reused_ = reused_;
   // Rememeber that we don't need to acquire it in future.
   dst->elem(tid_).reused = reused_;
 
   // If the resulting clock is cachable, cache it for future release operations.
   // The clock is always cachable if we released to an empty sync object.
   if (cached_idx_ == 0 && dst->Cachable()) {
void ThreadClock::acq_rel(ClockCache *c, SyncClock *dst) {
   ReleaseStore(c, dst);
 }
 
 // Updates only single element related to the current thread in dst->clk_.
 void ThreadClock::UpdateCurrentThread(ClockCache *c, SyncClock *dst) const {
   // Update the threads time, but preserve 'acquired' flag.
   for (unsigned i = 0; i < kDirtyTids; i++) {
     SyncClock::Dirty *dirty = &dst->dirty_[i];
-    const unsigned tid = dirty->tid;
+    const unsigned tid = dirty->tid();
     if (tid == tid_ || tid == kInvalidTid) {
       CPP_STAT_INC(StatClockReleaseFast);
-      dirty->tid = tid_;
+      dirty->set_tid(tid_);
       dirty->epoch = clk_[tid_];
       return;
     }
   }
   // Reset all 'acquired' flags, O(N).
   // We are going to touch dst elements, so we need to unshare it.
   dst->Unshare(c);
   CPP_STAT_INC(StatClockReleaseSlow);
   dst->elem(tid_).epoch = clk_[tid_];
   for (uptr i = 0; i < dst->size_; i++)
     dst->elem(i).reused = 0;
   dst->FlushDirty();
 }
 
 // Checks whether the current thread has already acquired src.
 bool ThreadClock::IsAlreadyAcquired(const SyncClock *src) const {
   if (src->elem(tid_).reused != reused_)
     return false;
   for (unsigned i = 0; i < kDirtyTids; i++) {
     SyncClock::Dirty dirty = src->dirty_[i];
-    if (dirty.tid != kInvalidTid) {
-      if (clk_[dirty.tid] < dirty.epoch)
+    if (dirty.tid() != kInvalidTid) {
+      if (clk_[dirty.tid()] < dirty.epoch)
         return false;
     }
   }
   return true;
 }
 
 // Checks whether the current thread has acquired anything
 // from other clocks after releasing to dst (directly or indirectly).

 
 void SyncClock::ResetImpl() {
   tab_ = 0;
   tab_idx_ = 0;
   size_ = 0;
   blocks_ = 0;
   release_store_tid_ = kInvalidTid;
   release_store_reused_ = 0;
-  for (uptr i = 0; i < kDirtyTids; i++)
-    dirty_[i].tid = kInvalidTid;
+  for (uptr i = 0; i < kDirtyTids; i++) dirty_[i].set_tid(kInvalidTid);
 }
 
 void SyncClock::Resize(ClockCache *c, uptr nclk) {
   CPP_STAT_INC(StatClockReleaseResize);
   Unshare(c);
   if (nclk <= capacity()) {
     // Memory is already allocated, just increase the size.
     size_ = nclk;
void SyncClock::Resize(ClockCache *c, uptr nclk) {
   }
   size_ = nclk;
 }
 
 // Flushes all dirty elements into the main clock array.
 void SyncClock::FlushDirty() {
   for (unsigned i = 0; i < kDirtyTids; i++) {
     Dirty *dirty = &dirty_[i];
-    if (dirty->tid != kInvalidTid) {
-      CHECK_LT(dirty->tid, size_);
-      elem(dirty->tid).epoch = dirty->epoch;
-      dirty->tid = kInvalidTid;
+    if (dirty->tid() != kInvalidTid) {
+      CHECK_LT(dirty->tid(), size_);
+      elem(dirty->tid()).epoch = dirty->epoch;
+      dirty->set_tid(kInvalidTid);
     }
   }
 }
 
 bool SyncClock::IsShared() const {
   if (size_ == 0)
     return false;
   atomic_uint32_t *ref = ref_ptr(tab_);
void SyncClock::Unshare(ClockCache *c) {
   old.Reset(c);
 }
 
 // Can we cache this clock for future release operations?
 ALWAYS_INLINE bool SyncClock::Cachable() const {
   if (size_ == 0)
     return false;
   for (unsigned i = 0; i < kDirtyTids; i++) {
-    if (dirty_[i].tid != kInvalidTid)
+    if (dirty_[i].tid() != kInvalidTid)
       return false;
   }
   return atomic_load_relaxed(ref_ptr(tab_)) == 1;
 }
 
 // elem linearizes the two-level structure into linear array.
 // Note: this is used only for one time accesses, vector operations use
 // the iterator as it is much faster.
ALWAYS_INLINE void SyncClock::append_block(u32 idx) {
   CHECK_EQ(get_block(bi), 0);
   tab_->table[ClockBlock::kBlockIdx - bi] = idx;
 }
 
 // Used only by tests.
 u64 SyncClock::get(unsigned tid) const {
   for (unsigned i = 0; i < kDirtyTids; i++) {
     Dirty dirty = dirty_[i];
-    if (dirty.tid == tid)
+    if (dirty.tid() == tid)
       return dirty.epoch;
   }
   return elem(tid).epoch;
 }
 
 // Used only by Iter test.
 u64 SyncClock::get_clean(unsigned tid) const {
   return elem(tid).epoch;
 }
 
 void SyncClock::DebugDump(int(*printf)(const char *s, ...)) {
   printf("clock=[");
   for (uptr i = 0; i < size_; i++)
     printf("%s%llu", i == 0 ? "" : ",", elem(i).epoch);
   printf("] reused=[");
   for (uptr i = 0; i < size_; i++)
     printf("%s%llu", i == 0 ? "" : ",", elem(i).reused);
   printf("] release_store_tid=%d/%d dirty_tids=%d[%llu]/%d[%llu]",
-      release_store_tid_, release_store_reused_,
-      dirty_[0].tid, dirty_[0].epoch,
-      dirty_[1].tid, dirty_[1].epoch);
+         release_store_tid_, release_store_reused_, dirty_[0].tid(),
+         dirty_[0].epoch, dirty_[1].tid(), dirty_[1].epoch);
 }
 
 void SyncClock::Iter::Next() {
   // Finished with the current block, move on to the next one.
   block_++;
   if (block_ < parent_->blocks_) {
     // Iterate over the next second level block.
     u32 idx = parent_->get_block(block_);