[PATCH] Relax atomic restrictions on memory dependence analysis

Philip Reames listmail at philipreames.com
Thu Aug 7 15:39:36 PDT 2014


See comment inline.  I ask that you do not submit this (even with the required bug fix) until I explicitly okay it.  I need time to a) read the paper and b) try to understand it.  

In general, I find these simple rules useful:
- A release operation can not be reordered w.r.t. any preceding memory operation.  It has no limitation w.r.t. following operations.  
- A memory operation can not be reordered w.r.t. a previous acquire.  Any memory operation can move after an acquire.  
- Two monotonic or stronger loads or stores can not be reordered unless you can prove doing so doesn't effect the global order for the locations effected.  (In addition to normal data dependence rules.)

I've found these two posts useful for understanding the C++ model:
http://preshing.com/20131125/acquire-and-release-fences-dont-work-the-way-youd-expect/
http://preshing.com/20120913/acquire-and-release-semantics/

================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:430
@@ +429,3 @@
+      if (LI->getOrdering() == Acquire
+              || LI->getOrdering() == SequentiallyConsistent)
+        HasSeenAcquire = true;
----------------
This is still not correct.  Consider this:
load %addr1, monotonic
load %addr2, monotonic

Unless you *know* that addr2 and addr1 are NoAlias, the second load must depend on the first.  Consider two threads with this pattern, one which reorders, one which doesn't.  The two threads would observe an inconsistent order of writes from a third thread which wrote a series of increasing integers.  

>From the LangRef monotonic spec: "If one atomic read happens before another atomic read of the same address, the later read must see the same value or a later value in the address’s modification order. *This disallows reordering of monotonic (or stronger) operations on the same address*. If an address is written monotonic-ally by one thread, and other threads monotonic-ally read that address repeatedly, the other threads must eventually see the write. This corresponds to the C++0x/C1x memory_order_relaxed."

To be explicit here, the problem I'm pointing out is *not* with your proposed optimization per se, it's with the removal of the early exit without adding a required change in default behaviour w.r.t aliasing. Getting *this part* right, should be it's own patch.   

================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:472
@@ -454,3 +471,3 @@
         // dependence.
         continue;
       }
----------------
Specifically, this comment is *wrong* for monotonic loads.

http://reviews.llvm.org/D4797






More information about the llvm-commits mailing list