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

[Xinliang David Li via llvm-commits]

On Thu, Mar 17, 2016 at 1:31 AM, Dmitry Vyukov <dvyukov at google.com> wrote:

> dvyukov added a comment.
>
> > yes -- C++ memory model does not allow speculative store motion. However
> in that example,  The compiler can do the following to remove the memory
> access of x in loop:
>
>
> I don't see how it relates. Races still lead to undefined behavior.
>

I think this is already assuming that the original program is race free
(i.e., plain load/stores are atomic,). Otherwise the behavior of the
original program would also be undefined.



>
> > DSE won't happen with monotonic used (as below) -- I am not sure if that
> is a bug.
>
>
> DSE is permitted by C/C++ standards in this case. So, yes, this is a bug.
>
>

Do you have a reference? Assuming monotonic maps to 'memory_order_relaxed',
then we have this specified:

"Atomic operations tagged memory_order_relaxed are not synchronization
operations, they do not order memory. They only guarantee atomicity and
modification order consistency."

Notice the mentioning of 'modification order consistency' there.  For
modification order, we have:

"Modification order
All modifications to any particular atomic variable occur in a total order
that is specific to this one atomic variable.
The following four requirements are guaranteed for all atomic operations:
1) Write-write coherence: If evaluation A that modifies some atomic M (a
write) happens-before evaluation B that modifies M, then A appears earlier
than B in the modification order of M
...
"

Would DSE change the modification order?

I checked with GCC -- it does not do DSE for this case either.

Besides, if 'monotonic' is considered completely 'no-op' (for x86) such as
optimizer should not be imposed with any constraints, why Tsan behaves
differently here (on x86)?

atomic_store(p, atomic_load(p,relaxed) + 1, relaxed)  <--- no warning
store(p, load(p) + 1)  <--- warning

thanks,

David




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