[PATCH] D18164: [tsan] Do not instrument reads/writes to instruction profile counters.

[Anna Zaks via llvm-commits]

zaks.anna added a comment.

> We are probably talking about different things here.  My main concern is the additional constraints 

>  imposed on the optimizer that can penalize optimizations (no-op or not on a target).


I see. Are there specific benchmarks that I could use for testing this impact?

> Since it is known certain combinations of instrumentions do not work together, why not documenting the 

>  behavior and let user filter out the false warnings?


These are not corner case combinations. We encourage users to run their tests with code coverage. At the same time, we advise to use sanitizers for bug finding. It is very likely that people will want to have both in their CI or when running tests.

> Back to your proposal to use Monotonic load. It does not fix the actual race (to fix the race, atomic 

>  fetch_add instruction is needed) nor can it silence tsan.


It does silence TSan, which is what I mainly care about. TSan performs it's instrumentation on LLVM IR level, where it sees the atomic load/store.

> Having said that, I think it is reasonable to introduce an option to enable atomic profile counter update.  

>  Tsan can then be safely combined with that.


Are you saying that you are OK with having this option but it should not be turned on by default unless we can show that the overhead of using Monotonic is minimal?


http://reviews.llvm.org/D18164