[PATCH] D12338: Add a boolean parameter to make the initial load atomic.

JF Bastien via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 25 15:18:10 PDT 2015


jfb added a comment.

This isn't just for PNaCl, so the description should be updated. You should mention something along the lines of:

> On weak memory systems the CPU can speculate on subsequent loads (e.g. the `cmpxchg`) and observe them without honoring the happens-before ordering of the corresponding stores. This is the "load buffering" problem in literature, and occurs on ARM and POWER.



================
Comment at: lib/CodeGen/AtomicExpandPass.cpp:557
@@ -554,1 +556,3 @@
+    InitLoaded->setOrdering(SequentiallyConsistent);
+  }
   Builder.CreateBr(LoopBB);
----------------
I think this should be an unconditional `InitLoaded->setOrdering(AtomicCmpXchgInst::getStrongestFailureOrdering(MemOpOrder));` and there shouldn't be an `IsRelaxed` parameter. This will give the leading `load` a memory order that's valid for loads, and corresponds to the ordering the RMW instruction had.


http://reviews.llvm.org/D12338





More information about the llvm-commits mailing list