<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:"Yu Gothic";
panose-1:2 11 4 0 0 0 0 0 0 0;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"\@Yu Gothic";
panose-1:2 11 4 0 0 0 0 0 0 0;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style>
<div class="WordSection1">
<p class="MsoNormal">> we can worry about optimization, perhaps using _Atomic, later</p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">We can’t use _Atomic because it would not be ABI compatible with our std::atomic; in particular, because we put the spinlock for non-lock-free atomics inside the atomic, for instance. And because that isn’t a thing for some of our supported
frontends.</p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">It is possible that there are different intrinsics we could call for Clang that would make you folks happier, but we don’t know what those are or even if they exist at present.</p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">> I think the only downside to using _ReadWriteBarrier, which compiles to an LLVM IR fence, and volatile memory operations</p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Also be aware that on Windows there is a lot of code from before the C++ memory model existed, that treats volatile loads as having acquire semantics, hence our `/volatile:ms` switch, which is the default on the platforms that do this for
`free` (x86 and amd64).</p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Billy3</p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Reid Kleckner <rnk@google.com><br>
<b>Sent:</b> Tuesday, July 30, 2019 4:10:28 PM<br>
<b>To:</b> Billy O'Neal (VC LIBS) <bion@microsoft.com><br>
<b>Cc:</b> Friedman, Eli <efriedma@codeaurora.org>; Grang, Mandeep Singh <mgrang@codeaurora.org>; JF Bastien <jfbastien@apple.com>; cfe-dev <cfe-dev@lists.llvm.org><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">Apparently I did this in March, and then completely forgot about it: <a href="https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Freviews.llvm.org%2FrC357220&data=02%7C01%7Cbion%40microsoft.com%7C5340e785a6a44de6422b08d715432513%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637001250443295750&sdata=oqdHoI3TSxD2RAyN42UGY8b601x7M9i1a3U8HpOHw40%3D&reserved=0" originalsrc="http://reviews.llvm.org/rC357220" shash="ElrJeoSXt0MDF6IRg4I9yiQGSSxBWnsu14ukAaq0oSXoa0MlzbCuo4CFc4GGCpdJGlvPZTa6dCKAsWovByq740Ed6P+T7hh0EKPmmZNzKrYmPbw62KEspR6c7/CPI1qywgHddqfDYJHtmKVo6HGLn7sPaCowpRjSc4qaBBvtbD0=">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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdevelopercommunity.visualstudio.com%2Fcontent%2Fproblem%2F274938%2Fvs2017-1567158p2-stdatomicload-causes-write-access.html&data=02%7C01%7Cbion%40microsoft.com%7C5340e785a6a44de6422b08d715432513%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637001250443305744&sdata=NCklr3zO%2BMnFYDlUpS8cZJKwKomznWv9tNOHTb5bibg%3D&reserved=0" originalsrc="https://developercommunity.visualstudio.com/content/problem/274938/vs2017-1567158p2-stdatomicload-causes-write-access.html" shash="m2nLfD+CIeL8Cu1Hm4YAW8MNj85/+bADVyXEGJIfmj3fLwIllaRRcvuGLPw2YJnGh3N7RqGzS211Qd/1FPKgZc/DrZ+35cfKuhBJbBepb9pVYTgRW0MLaCnsF75xdIep4haPK3atexniN7lgdVd141vzg/lOPlpgValn6ZeJuaM=" 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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fcpp%2Fintrinsics%2Freadwritebarrier%3Fview%3Dvs-2017&data=02%7C01%7Cbion%40microsoft.com%7C5340e785a6a44de6422b08d715432513%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637001250443305744&sdata=q62Fw5gzx0524CAIkVOVk%2FjxaVlzdW%2FivxB2RE9bZ0k%3D&reserved=0" originalsrc="https://docs.microsoft.com/en-us/cpp/intrinsics/readwritebarrier?view=vs-2017" shash="wE1x7hsKEEgVZ7jFGyuTnSA6mlqZH2Qg9LtyLnaW8YScUKf4IuBYINwdv0Wn6jSAtawBo9UkGc5B022ltqyrY5phfWBY/F/sa2/HmXa2XUpS06/WkLG5L7npM1ryLGPpL3mNLvjMw/vWw00klaSvqh/KCBho3OCukyUU3fMP9Qs=" 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%7C5340e785a6a44de6422b08d715432513%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637001250443315740&sdata=7FGbuLn5GZB%2B65qLlsOA29rYjXlvIENbsk1zzjRYAFo%3D&reserved=0" originalsrc="https://llvm.org/docs/LangRef.html#volatile-memory-accesses" shash="mXTrv1+tOpu71ao2mS1mmqDFe/kZl2MiT2KL2FHqT3Qgiwf+e/JjtmjGTr7A5C6TcaX9YBLQmSCU2uoYFnlPBncB56iWmwzgI/LrtH4v0mjhbdG9TBgELMUEIC/bgrVobDc1D0MaDHJLuLpeWxEPp9XXKgY7kVr/t2MXE/nk1hU=" 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%7C5340e785a6a44de6422b08d715432513%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637001250443315740&sdata=ytOttLd9m3RsN9VMBP04voImbNeFHYcfUGB5EQUjyWw%3D&reserved=0" originalsrc="https://llvm.org/docs/LangRef.html#memory-model-for-concurrent-operations" shash="vBnqfCP535CUN7aR8/gh/wl4Kjs2Ek+IU2rsmoepwVCSiRRUKnL2Zee6j5a6R2rFOuTKvk+Ju+NgMeOAlEq3kJ0RwqWMzSRcNTAnE6/eB8cxdil+2v3Nz6vEcUJtx6XCsSTUvaowqs7D9TRKDn+3e35PZY3/VudIP/t9EIPT68M=" 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%7C5340e785a6a44de6422b08d715432513%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637001250443325736&sdata=B%2BR32BFNQ57PKyFAKly9624V5Jr02mgIxnvfrMyW%2F2s%3D&reserved=0" originalsrc="https://reviews.llvm.org/D52807" shash="mWLYJ+EE0v3GE3r4nByYPf6VJqL+5VnFfVZb+PxoMnIhmSCHl1usjPf4nGb7iA0GMcagziEHRH3l4djKHvjAwDfRx3KXtkIHKIY9C4XPC7wq/3+Rp+iL/AZ/4jzQeZIj8yk4vkWTeWovU4WXPz9rmu21c07GUwpVcEyXaDagMPI=" 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%7C5340e785a6a44de6422b08d715432513%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637001250443325736&sdata=HipedgnoAk4vkWevTvUdjtfvPyjMuoOnWXtSNK909pE%3D&reserved=0" originalsrc="https://reviews.llvm.org/P8137" shash="lgMwZOWhAkHWiu7r1Hse4HeR1O9iagjoDVmaJOw8gAnFZSPZVGxP/4OMVubb/a0h6Fa24iRA1QAgBtnRNxDcUxfl8r2Yrt/cep0Grs25Ygsnq5Y7o1tx89FPeDMS2DGNeiIzCCBChmpvHvnAf5AlgVpU3ilN+jPpdFynwmzJ8oM=" 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>
</div>
</body>
</html>