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

[Xinliang David Li via llvm-commits]

On Tue, Mar 15, 2016 at 11:38 AM, Anna Zaks <zaks.anna at gmail.com> wrote:

> zaks.anna added a comment.
>
> > It is not about this assumption, but more about racy counter updates not
> being a problem practically  (other than the precision of the profile data
> might be
>
> >  affected slightly in practice).
>
>
> "Racy counter updates not being a problem practically" assumes atomicity
> of the loads and stores. My understanding is that we assume that the loads
> and stores are atomic and on top of that we do not care about the precision
> of the profile data.
>
>

Right -- the point about racy update I brought up is irrelevant here.




>
> Which architecture do you worry about? If these are pointer sized reads
> and writes, which are atomic on x86 and ARM, the monotonic loads and stores
> will turn into no-ops.
>


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). For instance, memory dependence
analysis treats Monotonic loads as 'read+write' (ModRef). This can
certainly affect DSE, etc.  On x86, the pseudo instruction generated for
Monotonic load is ACQUIRE_MOV with Volatile bit set. It might have some
impact too.


>
> > Do they turn on all sanitizers + profiling or just certain combinations?
> I am just trying to understand the use case here.
>
>
> The question is if we should detect the profiler and sanitizer being
> turned on at the same time and error out in the driver or if we should
> allow it. I do not see a reason to disallow this. To answer your other
> question, most of the sanitizers cannot be turned on simultaneously.
>

 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? Supporting all corner case combinations adds additional
maintenance cost (and costs can add up).

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.

David

>
>
> http://reviews.llvm.org/D18164
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160315/d27051aa/attachment.html>