<div dir="ltr">Do relaxed atomics actually introduce that much of slowdown?<div><br></div><div>You're intentionally introducing a data race, this doesn't look good to me at all. However, I'm not confident about</div><div>what's allowed in LLVM IR - it's not C++ where any source-level data race is UB, but not an x86 assembly either.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jul 29, 2015 at 4:43 PM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Alexey Samsonov <<a href="mailto:vonosmas@gmail.com">vonosmas@gmail.com</a>> writes:<br>
> samsonov created this revision.<br>
> samsonov added reviewers: dnovillo, bogner.<br>
> samsonov added a subscriber: llvm-commits.<br>
><br>
> Since we introduced counters for functions in COMDAT sections (e.g.<br>
> inline functions from STL headers), these headers can easily be<br>
> incremented concurrently by multiple threads. Replace load-add-store<br>
> with a single "atomicrmw add" with monotonic memory ordering.<br>
<br>
</span>This significantly changes the performance characteristics of this code,<br>
pessimizing single-threaded users and potentially making the<br>
multithreaded performance issues even worse.<br>
<br>
It's fine to add an option to lower these to atomics, since this does<br>
guarantee accuracy, but I think we need a switch to choose which kind of<br>
lowering we're interested in in that case.<br>
<div class="HOEnZb"><div class="h5"><br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11579&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=lDx1rX-3B32oZTA_vCe21kRN0Y14ujW2ePnnU3JiUX4&s=tiiUXsu0al8aXyrPKSc28tcPx4YG5wxgSglC63-ASTQ&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11579</a><br>
><br>
> Files:<br>
>   lib/Transforms/Instrumentation/InstrProfiling.cpp<br>
><br>
> Index: lib/Transforms/Instrumentation/InstrProfiling.cpp<br>
> ===================================================================<br>
> --- lib/Transforms/Instrumentation/InstrProfiling.cpp<br>
> +++ lib/Transforms/Instrumentation/InstrProfiling.cpp<br>
> @@ -147,9 +147,9 @@<br>
>    IRBuilder<> Builder(Inc->getParent(), *Inc);<br>
>    uint64_t Index = Inc->getIndex()->getZExtValue();<br>
>    Value *Addr = Builder.CreateConstInBoundsGEP2_64(Counters, 0, Index);<br>
> -  Value *Count = Builder.CreateLoad(Addr, "pgocount");<br>
> -  Count = Builder.CreateAdd(Count, Builder.getInt64(1));<br>
> -  Inc->replaceAllUsesWith(Builder.CreateStore(Count, Addr));<br>
> +  Builder.CreateAtomicRMW(AtomicRMWInst::Add, Addr, Builder.getInt64(1),<br>
> +                          llvm::Monotonic);<br>
> +  assert(Inc->use_empty() && "InstrProfIncrementInst has uses!");<br>
>    Inc->eraseFromParent();<br>
>  }<br>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div></div>
</div>