<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 15, 2016 at 4:28 PM, Anna Zaks via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">zaks.anna added a comment.<br>
<span class=""><br>
> We are probably talking about different things here.  My main concern is the additional constraints<br>
<br>
>  imposed on the optimizer that can penalize optimizations (no-op or not on a target).<br>
<br>
<br>
</span>I see. Are there specific benchmarks that I could use for testing this impact?<br></blockquote><div><br></div><div><br></div><div>SPEC or instrumented clang itself can be reasonable.  However the concern is  also about future optimizations -- more aggressive global optimizations on counter accesses (See Vedant's recent RFC about reducing instrumentation overhead).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> Since it is known certain combinations of instrumentions do not work together, why not documenting the<br>
<br>
>  behavior and let user filter out the false warnings?<br>
<br>
<br>
</span>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.<br></blockquote><div><br></div><div>so the use case include configs like:</div><div><br></div><div> - coverage + tsan</div><div> - coverage + asan</div><div> - coverage + ...san ?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> Back to your proposal to use Monotonic load. It does not fix the actual race (to fix the race, atomic<br>
<br>
>  fetch_add instruction is needed) nor can it silence tsan.<br>
<br>
<br>
</span>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.<br></blockquote><div><br></div><div>In the small case I tried, tsan  replaces the monotonic load with __tsan_atomic_load32(..) call -- i still saw the runtime warning.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> Having said that, I think it is reasonable to introduce an option to enable atomic profile counter update.<br>
<br>
>  Tsan can then be safely combined with that.<br>
<br>
<br>
</span>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?<br>
<div class="HOEnZb"><div class="h5"><br></div></div></blockquote><div><br></div><div><br></div><div>More than that. My suggestion is do it more aggressively. Introduce an option to enable lowering the counter increment into</div><div><br></div><div>__atomic_fetch_add(&counter, 1, __ATOMIC_RELAXED) </div><div><br></div><div>or </div><div><br></div><div>.. =  atomicrmw add ... monotonic   </div><div><br></div><div>at IR level.</div><div><br></div><div>This should be off by default.</div><div><br></div><div>David</div><div><br></div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
<br>
<a href="http://reviews.llvm.org/D18164" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18164</a><br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>