- User Since
- Dec 6 2012, 2:31 AM (327 w, 5 d)
Thu, Mar 14
I don't see any problems (because I don't understand cmake), so I am signing off :)
Thu, Mar 7
Wed, Mar 6
I should have read it better!
Mon, Mar 4
I don't mind merging this, but what's the usage/test story here? There are 30+ tests in test/Darwin/gcd-* but they don't run on linux.
Do you intend to do some further development of this support? I just afraid that most users won't be able to use it because it's not enabled, and the ones that will build own compiler will find it broken.
Mon, Feb 25
Kuba, please submit this. I can't test/build this.
Feb 13 2019
I see a similar failure was already reported:
There was no fix for it, right? Or did I forgot to squash something else?
- Forwarded message ---------
From: Yi-Hong Lyu via Phabricator <firstname.lastname@example.org>
Date: Wed, Feb 13, 2019 at 5:33 PM
Subject: [Diffusion] rL353817: tsan: add fiber support
This is submitted as part of 353947
Resubmitted in 353947 with 2 fixes squashed.
Looks good to me.
Feb 12 2019
Kuba, please take a look. This mostly touches Mac-specific files. I can't test this.
FTR updated check_analyze.sh in http://llvm.org/viewvc/llvm-project?view=revision&revision=353820
Submitted in 353805.
Testing and submitting.
Feb 11 2019
The change is now a very good shape.
I've seen some fixes has landed over the weekend. Is this fixed now? Or some breakages still remain?
Let's move the discussion to https://reviews.llvm.org/D57876 because there is already some.
Feb 8 2019
One thing that I noticed is that pthread_exit is a noreturn function and SCOPED_TSAN_INTERCEPTOR creates a local object with destructor. At the very least this leads to confusing code where readers assumption is that the destructor will run, but it will not.
Perhaps we should try to remove the SCOPED_TSAN_INTERCEPTOR entirely as the CHECK that we want to add here does not need it.
- Forwarded message ---------
From: Alex L
Date: Fri, Feb 8, 2019 at 1:17 AM
Subject: Re: [compiler-rt] r353385 - tsan: Implement pthread_exit() interceptor for Thread sanitizer
Feb 7 2019
To clarify the graph: it's difference in execution time in percents as compared to the current HEAD. I.e. -4 means that fibers are 4% slower than the current HEAD.
1w means mop.cc benchmark with 1-byte write accesses; 4r - 4-byte read accesses. ee is func_entry_exit.cc benchmark.
Did another round of benchmarking of this change on the current HEAD using these 2 benchmarks:
Committed in 353401
Committed in 353390
Committed in 353385
Feb 6 2019
Going forward I think we should get in all unrelated/preparatory changed first: thread type (creates lots of diffs), pthread_exit interceptor/test and spot optimizations to memory access functions.
Feb 5 2019
Also benchmarked function entry/exit using the following benchmark:
Since fiber support incurs slowdown for all current tsan users who don't use fibers, this is a hard decision.
Spent a while benchmarking this. I used 4 variations of a test program:
First just does read or write access of the given size:
Second, prepopulates shadow with some other accesses first:
Next has 2 threads that access the range in round-robin, both threads do writes (or reads):
The last one has 1 threads that does writes, followed by 2 threads that do reads:
Jan 21 2019
Now, when all performance issues are resolved, what else should be done to complete the review?
Jan 10 2019
Benchmark results with clang:
Jan 8 2019
Jan 7 2019
Jan 6 2019
Jan 4 2019
Jan 3 2019
What are tagged pointers? When they are used? What is the actual value? How does synchronized handle them? Is there a man page or something with more info? It should be included in the comment for future generations.
Jan 2 2019
I've benchmarked on 350140 with host gcc version 7.3.0 (Debian 7.3.0-5), running old/new binary alternated:
Dec 30 2018
This needs some tests, at least to exercise interceptors code once.
Nice! Interesting nobody reported this false positive on user-space code.
Dec 22 2018
I would like to land this and fix @synchronized with tagged pointers in a follow-up since more discussion is required what we actually want it to do.
Dec 21 2018
Dec 20 2018
Dec 19 2018
Merged as 349609.
Merging this now.
Dec 18 2018
FTR, here is current code:
I added default synchronization and flag to opt-out.
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?
Can't agree with this. Only fiber is visible to user. Of cause, it is the same as thread until user switches it.