<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">On Thu, Mar 28, 2019 at 2:28 PM Billy O'Neal (VC LIBS) <<a href="mailto:bion@microsoft.com" target="_blank">bion@microsoft.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">




<div dir="ltr">
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
><span style="font-size:15px;background-color:rgb(255,255,255);display:inline">Are you sure we shouldn't
 be marking these as atomic instead of volatile? Volatile is usually not suitable for anything except  </span></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<br>
</div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
I am implementing <atomic> <span id="gmail-m_6745075332462224179gmail-m_-3577172427236293292gmail-m_-2703270842879295710🙂" title=":slight_smile:">🙂</span></div></div></blockquote><div><br></div><div>My concern is that volatile in LLVM has nothing to do with atomicity, and I think what you really want is LLVM atomic loads and stores, with real ordering markers of monotonic, seq_cst, release, acquire, etc. This is all described here:</div><div><a href="https://llvm.org/docs/LangRef.html#volatile-memory-accesses" target="_blank">https://llvm.org/docs/LangRef.html#volatile-memory-accesses</a> </div><div><a href="https://llvm.org/docs/LangRef.html#memory-model-for-concurrent-operations" target="_blank">https://llvm.org/docs/LangRef.html#memory-model-for-concurrent-operations</a></div><div>In particular: "The optimizers may change the order of volatile operations relative to non-volatile operations. This is not Java’s “volatile” and has no cross-thread synchronization behavior."</div><div><br></div><div>So, I'm concerned that there is something subtly incorrect about using __iso_volatile_* to implement atomics. <br></div><div><br></div><div>After looking at the xatomic.h source, I think I have a better understanding of what is going on. I added some folks who probably have a better understanding of LLVM IR atomics, and maybe they can help guide the discussion to a simpler implementation of Visual C++ <atomic> that plays well with LLVM. We had a similar discussion about over- or under-emitting dmb fences in this code review: <a href="https://reviews.llvm.org/D52807">https://reviews.llvm.org/D52807</a>.</div><div><br></div><div>The Visual C++ headers currently add fences for ARM themselves. The code looks like this: <a href="https://reviews.llvm.org/P8137" target="_blank">https://reviews.llvm.org/P8137</a> In small test cases, this generates the appropriate code, a single load followed by a fence.</div><div><br></div><div>-----</div><div><br></div><div>Completely aside from improving the implementation of <atomic> with clang, I will go ahead and make those __iso_volatile_* intrinsics available everywhere.</div></div></div></div></div>