[InstrProfiling] Fix data race on profile counters by using AtomicRMW.

[samsonov]

Summary

Since we introduced counters for functions in COMDAT sections (e.g.
inline functions from STL headers), these headers can easily be
incremented concurrently by multiple threads. Replace load-add-store
with a single "atomicrmw add" with monotonic memory ordering.

[dnovillo]

I'm confused. Your change seems to be modifying the semantics of the expansion. Could you elaborate on why this is safe? The code used to expect uses of the increment instruction.

[samsonov]

What kind of uses do you refer to? InstrProfIncrementInst we're replacing has type "void", so it doesn't return any value that can be consumed.

[dnovillo]

In D11579#214650, @samsonov wrote:

What kind of uses do you refer to? InstrProfIncrementInst we're replacing has type "void", so it doesn't return any value that can be consumed.

Yeah, that's what's confusing me. Why was the code before calling RAUW?

Other than that, your change looks fine to me. But there's clearly something I'm not quite understanding here. I'd like Justin to chime in.

[dvyukov]

If your intention is to fix data races rather than to improve precision, then the more straightforward fix would be:

atomic_store_n(&cnt, atomic_load_n(&cnt, __ATOMIC_RELAXED) + 1, __ATOMIC_RELAXED);

It should not affect performance of the generated code (unless there is a bug in atomic codegen).

lib/Transforms/Instrumentation/InstrProfiling.cpp

 }
 
 void InstrProfiling::lowerIncrement(InstrProfIncrementInst *Inc) {
   GlobalVariable *Counters = getOrCreateRegionCounters(Inc);
 
   IRBuilder<> Builder(Inc->getParent(), *Inc);
   uint64_t Index = Inc->getIndex()->getZExtValue();
   Value *Addr = Builder.CreateConstInBoundsGEP2_64(Counters, 0, Index);
-  Value *Count = Builder.CreateLoad(Addr, "pgocount");
-  Count = Builder.CreateAdd(Count, Builder.getInt64(1));
-  Inc->replaceAllUsesWith(Builder.CreateStore(Count, Addr));
+  Builder.CreateAtomicRMW(AtomicRMWInst::Add, Addr, Builder.getInt64(1),
+                          llvm::Monotonic);
+  assert(Inc->use_empty() && "InstrProfIncrementInst has uses!");
   Inc->eraseFromParent();
 }
 
 void InstrProfiling::lowerCoverageData(GlobalVariable *CoverageData) {
   CoverageData->setSection(getCoverageSection());
   CoverageData->setAlignment(8);
 
   Constant *Init = CoverageData->getInitializer();