[PATCH] tsan: find races between atomic and plain memory accesses

Kostya Serebryany kcc at google.com
Thu Jan 31 07:32:09 PST 2013


  great in general!
  few comments:


================
Comment at: tsan/rtl/tsan_rtl.h:267
@@ -252,2 +266,3 @@
   u64 size() const { return 1ull << size_log(); }
   bool is_write() const { return x_ & 32; }
+  bool IsWrite() const { return x_ & 32; }
----------------
do we want to have both is_write and IsWrite?

================
Comment at: tsan/rtl/tsan_rtl.h:312
@@ -274,1 +311,3 @@
  private:
+  static const u64 kAtomicBit = 1ull << 6;
+
----------------
you have kAtomicBit, but you are still using 5 and 6 in functions above.
Maybe have kAtomicAndWriteBits? 

================
Comment at: tsan/rtl/tsan_interface_inl.h:22
@@ -21,3 +21,3 @@
 void __tsan_read1(void *addr) {
-  MemoryAccess(cur_thread(), CALLERPC, (uptr)addr, 0, 0);
+  MemoryAccess(cur_thread(), CALLERPC, (uptr)addr, 0, 0, 0);
 }
----------------
why do we have 1/0 instead of true/false?
maybe it's better to use enums with descriptive names here?
or 4 functions? 


http://llvm-reviews.chandlerc.com/D357



More information about the llvm-commits mailing list