[PATCH] D54889: Fiber support for thread sanitizer

Yuri Per via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 20 07:52:21 PST 2018


yuri added a comment.

In D54889#1334562 <https://reviews.llvm.org/D54889#1334562>, @dvyukov wrote:

> I don't completely follow logic behind cur_thread/cur_thread_fast/cur_thread1 and how it does not introduce slowdown.
>  cur_thread_fast still includes an indirection, so it looks exactly the same as the first version.
>  cur_thread_fast does not check for NULL, so I would expect it to be used only in few carefully chosen functions. But it's used in most entry functions. Which makes me wonder: when cur_thread_fast cannot be used if it can be used in all these places?
>  It seems that you use cur_thread only to lazily initialize cur_thread1 in Initialize or ThreadStart. But then the question is: why don't we explicitly intialize cur_thread1 in these functions, and then just have a single cur_thread function as before with no 2 subtly different accessors?


I assume that after program or new thread starts sanitizer will first see few (or maybe zero)  interceptors, then _func_entry() and then everything else. Following this logic, I use cur_thread_fast() in all performance-critical entry points excluding _func_entry().

I ran tsan benchmarks and it looks like memory indirection by itself does not  affect performance because variable is in CPU cache in most cases. At the same time, conditional check has visible effect on performance.

Benchmarks result are the following:

- Mop2 and Mop8 are 3% faster
- FuncCall is 3% slower
- Mop2Write and Mop4Write are 8% slower
- everything else - no difference

Some degradation for FuncCall is expected. I will check later what happened to Mop2Write and Mop4Write. My initial guess is that because ThreadState pointer is now stored in register during entire function run, some other local variable was forced out of register to stack.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D54889





More information about the llvm-commits mailing list