<div dir="ltr">Apparently I did this in March, and then completely forgot about it: <a href="http://reviews.llvm.org/rC357220">http://reviews.llvm.org/rC357220</a> <div><br></div><div>I think I still had concerns about whether implementing <atomic> with _ReadWriteBarrier and plain, non-LLVM-atomic, volatile loads and stores is completely correct. I never figured out how to express those concerns, so I guess I'll just raise it to JF and Eli, since they probably know more about the C++ memory model than I do. Are there any correctness problems with the VC++ library <atomic> implementation strategy?</div><div><br></div><div>I think the only downside to using _ReadWriteBarrier, which compiles to an LLVM IR fence, and volatile memory operations, is that they won't optimize as well as LLVM-IR-atomic loads and stores. But as long as it's correct, we can worry about optimization, perhaps using _Atomic, later.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jul 25, 2019 at 12:18 PM Billy O'Neal (VC LIBS) <<a href="mailto:bion@microsoft.com">bion@microsoft.com</a>> wrote:<br></div><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)">
Update: MSVC++ should gain this ability in 2019 Update 4; so it would be nice to enable these as soon as possible. The user in <a href="https://developercommunity.visualstudio.com/content/problem/274938/vs2017-1567158p2-stdatomicload-causes-write-access.html" target="_blank">https://developercommunity.visualstudio.com/content/problem/274938/vs2017-1567158p2-stdatomicload-causes-write-access.html</a> has
been long suffering <span id="gmail-m_2566262313982254463🙂" title=":slight_smile:">🙂</span></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<span title=":slight_smile:"><br>
</span></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<span title=":slight_smile:">Billy3</span></div>
<div id="gmail-m_2566262313982254463appendonsend"></div>
<hr style="display:inline-block;width:98%">
<div id="gmail-m_2566262313982254463divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Billy O'Neal (VC LIBS) <<a href="mailto:bion@microsoft.com" target="_blank">bion@microsoft.com</a>><br>
<b>Sent:</b> Thursday, March 28, 2019 03:13 PM<br>
<b>To:</b> Reid Kleckner <<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>>; Friedman, Eli <<a href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>>; Grang, Mandeep Singh <<a href="mailto:mgrang@codeaurora.org" target="_blank">mgrang@codeaurora.org</a>><br>
<b>Cc:</b> JF Bastien <<a href="mailto:jfbastien@apple.com" target="_blank">jfbastien@apple.com</a>>; cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>><br>
<b>Subject:</b> Re: [cfe-dev] FYI, Intel folks might be looking to add the __iso_volatile_Xxx family for MSVC STL <atomic> soon</font>
<div> </div>
</div>
<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">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.</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)">
Right, but if they want to link with MSVC++ generated code they need to at least respect
<a href="https://docs.microsoft.com/en-us/cpp/intrinsics/readwritebarrier?view=vs-2017" title="https://docs.microsoft.com/en-us/cpp/intrinsics/readwritebarrier?view=vs-2017" target="_blank">
_ReadWriteBarrier</a>.</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 can certainly mess with our <atomic> to play more nicely with you folks if you want us calling different builtins instead. <atomic> is entirely library tech for us.</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)">
Billy3</div>
<div id="gmail-m_2566262313982254463x_appendonsend"></div>
<hr style="display:inline-block;width:98%">
<div id="gmail-m_2566262313982254463x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Reid Kleckner <<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>><br>
<b>Sent:</b> Thursday, March 28, 2019 03:08 PM<br>
<b>To:</b> Billy O'Neal (VC LIBS); Friedman, Eli; Grang, Mandeep Singh<br>
<b>Cc:</b> JF Bastien; cfe-dev<br>
<b>Subject:</b> Re: [cfe-dev] FYI, Intel folks might be looking to add the __iso_volatile_Xxx family for MSVC STL <atomic> soon</font>
<div> </div>
</div>
<div>
<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-m_2566262313982254463x_x_gmail_quote">
<blockquote class="gmail-m_2566262313982254463x_x_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_2566262313982254463x_x_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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fllvm.org%2Fdocs%2FLangRef.html%23volatile-memory-accesses&data=02%7C01%7Cbion%40microsoft.com%7Cd5b30d145383489891e408d6b3ca007c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636894077507135048&sdata=73J4avlu9DO5KGNF%2BONhLJVMzl1Z%2F0TMDnJE9fpUkmY%3D&reserved=0" target="_blank">https://llvm.org/docs/LangRef.html#volatile-memory-accesses</a> </div>
<div><a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fllvm.org%2Fdocs%2FLangRef.html%23memory-model-for-concurrent-operations&data=02%7C01%7Cbion%40microsoft.com%7Cd5b30d145383489891e408d6b3ca007c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636894077507145039&sdata=Vn1qu6pXuqVPMWJtclPUeFOYp5eK4aLRf2WWJxpzEfA%3D&reserved=0" 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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD52807&data=02%7C01%7Cbion%40microsoft.com%7Cd5b30d145383489891e408d6b3ca007c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636894077507145039&sdata=RroXJ2BBZBaTokulSxd2PTOUor6e56Gm0ii%2Bau9om%2Bw%3D&reserved=0" target="_blank">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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FP8137&data=02%7C01%7Cbion%40microsoft.com%7Cd5b30d145383489891e408d6b3ca007c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636894077507155032&sdata=UORTGrVnJ%2F8IcsoG1Bezaov2sSB7WrXejzHGyCTH4Cc%3D&reserved=0" 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>
</div>
</div>
</div>
</blockquote></div>