[sanitizer] address some of the dvyukov's comments on previous commits

llvm-svn: 201322

compiler-rt/lib/sanitizer_common/sanitizer_bitvector.h

@@ -23,24 +23,29 @@ template <class basic_int_t = uptr>
 class BasicBitVector {
  public:
   enum SizeEnum { kSize = sizeof(basic_int_t) * 8 };
+
   uptr size() const { return kSize; }
   // No CTOR.
   void clear() { bits_ = 0; }
   void setAll() { bits_ = ~(basic_int_t)0; }
   bool empty() const { return bits_ == 0; }
+
   // Returns true if the bit has changed from 0 to 1.
   bool setBit(uptr idx) {
     basic_int_t old = bits_;
     bits_ |= mask(idx);
     return bits_ != old;
   }
+
   // Returns true if the bit has changed from 1 to 0.
   bool clearBit(uptr idx) {
     basic_int_t old = bits_;
     bits_ &= ~mask(idx);
     return bits_ != old;
   }
+
   bool getBit(uptr idx) const { return bits_ & mask(idx); }
+
   uptr getAndClearFirstOne() {
     CHECK(!empty());
     // FIXME: change to LeastSignificantSetBitIndex?
@@ -61,18 +66,17 @@ class BasicBitVector {
   // Returns true if 'this' intersects with 'v'.
   bool intersectsWith(const BasicBitVector &v) const { return bits_ & v.bits_; }
 
-
  private:
   basic_int_t mask(uptr idx) const {
-    CHECK_LE(idx, size());
+    CHECK_LT(idx, size());
     return (basic_int_t)1UL << idx;
   }
   basic_int_t bits_;
 };
 
 // Fixed size bit vector of (kLevel1Size*BV::kSize**2) bits.
-// The implementation is optimized for a sparse bit vector, i.e. the one
-// that has few set bits.
+// The implementation is optimized for better performance on
+// sparse bit vectors, i.e. the those with few set bits.
 template <uptr kLevel1Size = 1, class BV = BasicBitVector<> >
 class TwoLevelBitVector {
   // This is essentially a 2-level bit vector.
@@ -83,11 +87,14 @@ class TwoLevelBitVector {
  public:
   enum SizeEnum { kSize = BV::kSize * BV::kSize * kLevel1Size };
   // No CTOR.
+
   uptr size() const { return kSize; }
+
   void clear() {
     for (uptr i = 0; i < kLevel1Size; i++)
       l1_[i].clear();
   }
+
   void setAll() {
     for (uptr i0 = 0; i0 < kLevel1Size; i0++) {
       l1_[i0].setAll();
@@ -95,12 +102,14 @@ class TwoLevelBitVector {
         l2_[i0][i1].setAll();
     }
   }
+
   bool empty() {
     for (uptr i = 0; i < kLevel1Size; i++)
       if (!l1_[i].empty())
         return false;
     return true;
   }
+
   // Returns true if the bit has changed from 0 to 1.
   bool setBit(uptr idx) {
     check(idx);
@@ -116,6 +125,7 @@ class TwoLevelBitVector {
     // idx, i0, i1, i2, res);
     return res;
   }
+
   bool clearBit(uptr idx) {
     check(idx);
     uptr i0 = idx0(idx);
@@ -129,6 +139,7 @@ class TwoLevelBitVector {
     }
     return res;
   }
+
   bool getBit(uptr idx) const {
     check(idx);
     uptr i0 = idx0(idx);
@@ -137,6 +148,7 @@ class TwoLevelBitVector {
     // Printf("%s: %zd => %zd %zd %zd\n", __FUNCTION__, idx, i0, i1, i2);
     return l1_[i0].getBit(i1) && l2_[i0][i1].getBit(i2);
   }
+
   uptr getAndClearFirstOne() {
     for (uptr i0 = 0; i0 < kLevel1Size; i0++) {
       if (l1_[i0].empty()) continue;
@@ -151,6 +163,7 @@ class TwoLevelBitVector {
     CHECK(0);
     return 0;
   }
+
   // Do "this |= v" and return whether new bits have been added.
   bool setUnion(const TwoLevelBitVector &v) {
     bool res = false;
@@ -176,6 +189,7 @@ class TwoLevelBitVector {
   bool intersectsWith(const TwoLevelBitVector &v) const {
     for (uptr i0 = 0; i0 < kLevel1Size; i0++) {
       BV t = l1_[i0];
+      // FIXME: optimization: t &= v.l1_[i0]
       while (!t.empty()) {
         uptr i1 = t.getAndClearFirstOne();
         if (!v.l1_[i0].getBit(i1)) continue;
@@ -188,16 +202,19 @@ class TwoLevelBitVector {
 
  private:
   void check(uptr idx) const { CHECK_LE(idx, size()); }
+
   uptr idx0(uptr idx) const {
     uptr res = idx / (BV::kSize * BV::kSize);
     CHECK_LE(res, kLevel1Size);
     return res;
   }
+
   uptr idx1(uptr idx) const {
     uptr res = (idx / BV::kSize) % BV::kSize;
     CHECK_LE(res, BV::kSize);
     return res;
   }
+
   uptr idx2(uptr idx) const {
     uptr res = idx % BV::kSize;
     CHECK_LE(res, BV::kSize);

compiler-rt/lib/sanitizer_common/sanitizer_bvgraph.h

@@ -34,12 +34,12 @@ class BVGraph {
 
   // Returns true if a new edge was added.
   bool addEdge(uptr from, uptr to) {
-    check(from|to);
+    check(from, to);
     return v[from].setBit(to);
   }
 
   bool hasEdge(uptr from, uptr to) const {
-    check(from|to);
+    check(from, to);
     return v[from].getBit(to);
   }
 
@@ -78,7 +78,7 @@ class BVGraph {
   }
 
  private:
-  void check(uptr idx) const { CHECK_LE(idx, size()); }
+  void check(uptr idx1, uptr idx2) const { CHECK_LE(idx1|idx2, size()); }
   BV v[kSize];
 };
 

compiler-rt/lib/sanitizer_common/sanitizer_deadlock_detector.h

@@ -30,10 +30,12 @@ class DeadlockDetectorTLS {
  public:
   // No CTOR.
   void clear() { n_locks_ = 0; }
+
   void addLock(uptr node) {
     CHECK_LT(n_locks_, ARRAY_SIZE(locks_));
     locks_[n_locks_++] = node;
   }
+
   void removeLock(uptr node) {
     CHECK_NE(n_locks_, 0U);
     for (sptr i = n_locks_ - 1; i >= 0; i--) {
@@ -45,7 +47,9 @@ class DeadlockDetectorTLS {
     }
     CHECK(0);
   }
+
   uptr numLocks() const { return n_locks_; }
+
   uptr getLock(uptr idx) const {
     CHECK_LT(idx, n_locks_);
     return locks_[idx];
@@ -67,6 +71,7 @@ class DeadlockDetector {
   typedef BV BitVector;
 
   uptr size() const { return g_.size(); }
+
   // No CTOR.
   void clear() {
     current_epoch_ = 0;
@@ -90,7 +95,7 @@ class DeadlockDetector {
       return getAvailableNode(data);
     }
     // We are out of vacant nodes. Flush and increment the current_epoch_.
-    uptr new_epoch = current_epoch_ + BV::kSize;
+    uptr new_epoch = current_epoch_ + size();
     clear();
     current_epoch_ = new_epoch;
     available_nodes_.setAll();
@@ -131,23 +136,28 @@ class DeadlockDetector {
 
  private:
   void check_idx(uptr idx) const { CHECK_LT(idx, size()); }
+
   void check_node(uptr node) const {
     CHECK_GE(node, size());
     CHECK_EQ(current_epoch_, node / size() * size());
   }
+
   uptr indexToNode(uptr idx) {
     check_idx(idx);
-    return idx | current_epoch_;
+    return idx + current_epoch_;
   }
+
   uptr nodeToIndex(uptr node) {
     check_node(node);
     return node % size();
   }
+
   uptr getAvailableNode(uptr data) {
     uptr idx = available_nodes_.getAndClearFirstOne();
     data_[idx] = data;
     return indexToNode(idx);
   }
+
   uptr current_epoch_;
   BV available_nodes_;
   BV recycled_nodes_;

compiler-rt/lib/sanitizer_common/tests/sanitizer_bitvector_test.cc

@@ -108,4 +108,6 @@ TEST(SanitizerCommon, TwoLevelBitVector) {
   TestBitVector<TwoLevelBitVector<> >(ws * ws);
   TestBitVector<TwoLevelBitVector<2> >(ws * ws * 2);
   TestBitVector<TwoLevelBitVector<3> >(ws * ws * 3);
+  TestBitVector<TwoLevelBitVector<3, BasicBitVector<u16> > >
+      (16 * 16 * 3);
 }

compiler-rt/lib/sanitizer_common/tests/sanitizer_deadlock_detector_test.cc

@@ -100,4 +100,6 @@ void TestDeadlockDetector() {
 TEST(SanitizerCommon, DeadlockDetector) {
   TestDeadlockDetector<BasicBitVector<> >();
   TestDeadlockDetector<TwoLevelBitVector<2> >();
+  TestDeadlockDetector<TwoLevelBitVector<3,
+      BasicBitVector<unsigned short> > >();
 }