Resolve data races found by ThreadSanitizer

Signed-off-by: Eryk Szpotanski <[email protected]>
Co-authored-by: Krzysztof Bieganski <[email protected]>

Author: eszpotanski

graph/Graph.cc

@@ -586,6 +586,7 @@ Graph::makeArrivals(Vertex *vertex,
 Arrival *
 Graph::arrivals(Vertex *vertex)
 {
+  LockGuard lock(arrivals_lock_);
   return arrivals_.pointer(vertex->arrivals());
 }
 
@@ -619,6 +620,7 @@ Graph::makeRequireds(Vertex *vertex,
 Required *
 Graph::requireds(Vertex *vertex)
 {
+  LockGuard lock(requireds_lock_);
   return requireds_.pointer(vertex->requireds());
 }
 
@@ -650,6 +652,7 @@ Graph::makePrevPaths(Vertex *vertex,
 PathVertexRep *
 Graph::prevPaths(Vertex *vertex) const
 {
+  LockGuard lock(prev_paths_lock_);
   return prev_paths_.pointer(vertex->prevPaths());
 }
 
@@ -1352,20 +1355,31 @@ Vertex::setHasDownstreamClkPin(bool has_clk_pin)
   has_downstream_clk_pin_ = has_clk_pin;
 }
 
+#define IN_QUEUE(mask, index) (mask & (1 << unsigned(index)))
+#define SET_IN_QUEUE(mask, index) ((mask) |= (1 << unsigned(index)))
+#define CLEAR_IN_QUEUE(mask, index) ((mask) &= ~(1 << unsigned(index)))
+
 bool
 Vertex::bfsInQueue(BfsIndex index) const
 {
-  return (bfs_in_queue_ >> unsigned(index)) & 1;
+  return IN_QUEUE(bfs_in_queue_, index);
 }
 
-void
+bool
 Vertex::setBfsInQueue(BfsIndex index,
 		      bool value)
 {
-  if (value)
-    bfs_in_queue_ |= 1 << int(index);
-  else
-    bfs_in_queue_ &= ~(1 << int(index));
+  unsigned char expected = bfs_in_queue_;
+  unsigned char desired;
+  do {
+    if ((value && IN_QUEUE(expected, index)) || (!value && !IN_QUEUE(expected, index))) {
+      return false;
+    }
+    desired = expected;
+    SET_IN_QUEUE(value ? desired : expected, index);
+    CLEAR_IN_QUEUE(value ? expected : desired, index);
+  } while (!bfs_in_queue_.compare_exchange_weak(expected, desired));
+  return true;
 }
 
 ////////////////////////////////////////////////////////////////

include/sta/Graph.hh

@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include <atomic>
 #include <mutex>
 
 #include "Iterator.hh"
@@ -253,7 +254,7 @@ protected:
   RequiredsTable requireds_;
   std::mutex requireds_lock_;
   PrevPathsTable prev_paths_;
-  std::mutex prev_paths_lock_;
+  mutable std::mutex prev_paths_lock_;
   Vector<bool> arc_delay_annotated_;
   int slew_rf_count_;
   bool have_arc_delays_;
@@ -329,7 +330,7 @@ public:
   bool isConstrained() const { return is_constrained_; }
   void setIsConstrained(bool constrained);
   bool bfsInQueue(BfsIndex index) const;
-  void setBfsInQueue(BfsIndex index, bool value);
+  bool setBfsInQueue(BfsIndex index, bool value);
   bool isRegClk() const { return is_reg_clk_; }
   bool crprPathPruningDisabled() const { return crpr_path_pruning_disabled_;}
   void setCrprPathPruningDisabled(bool disabled);
@@ -357,10 +358,10 @@ protected:
   EdgeId out_edges_;		// Edges from this vertex.
 
   // 32 bits
-  unsigned int tag_group_index_:tag_group_index_bits; // 24
+  unsigned int tag_group_index_; // >= tag_group_index_bits = 24
   // Each bit corresponds to a different BFS queue.
-  unsigned int bfs_in_queue_:int(BfsIndex::bits); // 4
-  unsigned int slew_annotated_:slew_annotated_bits;
+  std::atomic<unsigned char> bfs_in_queue_; // >= int(BfsIndex::bits) = 4
+  unsigned char object_idx_; // >= VertexTable::idx_bits = 7
 
   // 32 bits
   unsigned int level_:Graph::vertex_level_bits;
@@ -385,7 +386,8 @@ protected:
   bool has_downstream_clk_pin_:1;
   bool crpr_path_pruning_disabled_:1;
   bool requireds_pruned_:1;
-  unsigned object_idx_:VertexTable::idx_bits;
+
+  unsigned int slew_annotated_:slew_annotated_bits;
 
 private:
   friend class Graph;

include/sta/Search.hh

@@ -587,7 +587,7 @@ protected:
   TagIndex tag_next_;
   // Holes in tags_ left by deleting filter tags.
   std::vector<TagIndex> tag_free_indices_;
-  std::mutex tag_lock_;
+  mutable std::mutex tag_lock_;
   TagGroupSet *tag_group_set_;
   TagGroup **tag_groups_;
   TagGroup **tag_groups_prev_;
@@ -596,7 +596,7 @@ protected:
   std::vector<TagIndex> tag_group_free_indices_;
   // Capacity of tag_groups_.
   TagGroupIndex tag_group_capacity_;
-  std::mutex tag_group_lock_;
+  mutable std::mutex tag_group_lock_;
   // Latches data outputs to queue on the next search pass.
   VertexSet *pending_latch_outputs_;
   std::mutex pending_latch_outputs_lock_;

search/Search.cc

@@ -2775,6 +2775,7 @@ Search::reportArrivals(Vertex *vertex) const
 TagGroup *
 Search::tagGroup(TagGroupIndex index) const
 {
+  LockGuard lock(tag_group_lock_);
   return tag_groups_[index];
 }
 
@@ -2784,13 +2785,14 @@ Search::tagGroup(const Vertex *vertex) const
   TagGroupIndex index = vertex->tagGroupIndex();
   if (index == tag_group_index_max)
     return nullptr;
-  else
-    return tag_groups_[index];
+  LockGuard lock(tag_group_lock_);
+  return tag_groups_[index];
 }
 
 TagGroupIndex
 Search::tagGroupCount() const
 {
+  LockGuard lock(tag_group_lock_);
   return tag_group_set_->size();
 }
 
@@ -2845,12 +2847,14 @@ Search::reportArrivalCountHistogram() const
 Tag *
 Search::tag(TagIndex index) const
 {
+  LockGuard lock(tag_lock_);
   return tags_[index];
 }
 
 TagIndex
 Search::tagCount() const
 {
+  LockGuard lock(tag_lock_);
   return tag_set_->size();
 }