[PATCH] D54889: Fiber support for thread sanitizer

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added a comment.

Did another round of benchmarking of this change on the current HEAD using these 2 benchmarks:
https://github.com/llvm-mirror/compiler-rt/blob/master/lib/tsan/benchmarks/mop.cc
https://github.com/llvm-mirror/compiler-rt/blob/master/lib/tsan/benchmarks/func_entry_exit.cc

F8239980: fibers vs current (larger is better).png <https://reviews.llvm.org/F8239980>

Here fibers is this change, and fibers* is this change with 2 additional changes:

1. cur_thread_fast in __tsan_func_entry.
2. cur_thread1 embed in the ThreadState. The reason for this is that cur_thread1 and ThreadState are currently separated and in different cache lines (cur_thread is allocated with ALIGN(64) attribute and all hot members are in the beginning of the struct). So what I did is:

  struct ThreadState { FastState fast_state; u64 fast_synch_epoch; ThreadState* current;  // <----- ADD THIS ...

  INLINE ThreadState *cur_thread_fast() { //return cur_thread1; return reinterpret_cast<ThreadState *>(cur_thread_placeholder)->current; }

Now ~2% slowdown on highly synthetic benchmarks looks like something we can tolerate (2 cases are actually faster).

The cur_thread/cur_thread_fast separation still looks confusing to me. It's a convoluted way to do lazy initialization. If one adds any new calls to these functions, which one to choose is non-obvious. I think we should do lazy initialization explicitly. Namely, leave cur_thread alone, don't introduce cur_thread_fast, don't change any call sites. Instead, add init_cur_thread call that does lazy initialization to interceptor entry point and any other points that we expect can be the first call into tsan runtime overall or within a new thread. I think interceptors and __tsan_init should be enough (no __tsan_func_entry). We call __tsan_init from .preinit_array, instrumented code can't be executed before .preinit_array, only interceptors from dynamic loader can precede .preinit_array callbacks.

With these 3 changes, it looks good to me and I am ready to merge it.


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54889/new/

https://reviews.llvm.org/D54889