<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 16, 2016 at 1:11 AM, Dmitry Vyukov <span dir="ltr"><<a href="mailto:dvyukov@google.com" target="_blank">dvyukov@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">dvyukov added a comment.<br>
<span class=""><br>
> Back to your proposal to use Monotonic load. It does not fix the actual race<br>
<br>
<br>
</span>Your definition of race disagrees with C/C++ standards and llvm rules. The proposal does fix the races and eliminates undefined behavior. It is legal to do atomic_store(p, atomic_load(p) + 1) if that's what you want (compiler ought to emit single ADD instruction for that).<br>
<span class=""><br>
> I don't agree with you about the 'incorrect code' part.<br>
<br>
<br>
</span>Check out <a href="http://llvm.org/docs/Atomics.html#optimization-outside-atomic" rel="noreferrer" target="_blank">http://llvm.org/docs/Atomics.html#optimization-outside-atomic</a><br>
"The basic 'load' and 'store' allow a variety of optimizations, but can lead to undefined results in a concurrent environment"<br>
<br></blockquote><div><br></div><div>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:</div><div><br></div><div><div>int x;</div><div>void f(int* a) {</div><div>  int xtemp = x;</div><div>  bool stored_p = false;</div><div>  for (int i = 0; i < 100; i++) {</div><div>    if (a[i]) {</div><div>      xtemp += 1;</div><div>      stored_p = true;</div><div>    }</div><div>  }</div><div><br></div><div> if (stored_p)</div><div>    x = xtemp;</div><div>}</div></div><div><br></div><div>David</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
<a href="http://reviews.llvm.org/D18164" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18164</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>