<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<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:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Verdana;
panose-1:2 11 6 4 3 5 4 4 2 4;}
@font-face
{font-family:"Lucida Console";
panose-1:2 11 6 9 4 5 4 2 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Verdana",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><a name="_MailEndCompose">On Tue, Nov 25, 2014 at 5:16 AM, Cownie, James H <</a><a href="mailto:james.h.cownie@intel.com" target="_blank">james.h.cownie@intel.com</a>> wrote:<o:p></o:p></p>
<p class="MsoNormal">From a runtime point of view we need to preserve backwards binary compatibility, so we can't change the interface to the current interface function to introduce a count (because that old code won't set it).<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span style="color:#984807;mso-style-textfill-fill-color:#984807;mso-style-textfill-fill-alpha:100.0%">Okay. I wasn't sure if you were guaranteeing backwards binary compatibility with existing compiles or not. The way that I understand
you're currently deploying the runtime — linking in a static library that's distributed with the compiler — does not actually require backwards binary compatibility as far as I can tell. But if you're making that guarantee, then I agree we can't break it.<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D">You missed that we don’t normally build a static library at all, but rather a dynamic one. Therefore we need to maintain backwards binary compatibility
so that we can link code compiled by an old compiler against a library provided by a new one.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D">Static-linking the OpenMP runtime is a very bad idea, since it can lead to multiple copies of the runtime in the same process (when a shared library
is linked statically against one OpenMP runtime and another shared library, or the executable, is linked against another). That, in turn, can lead to over-subscription (each rutime creates its own thread pool) with associated poor performance, or, even incorrect
behaviour (</span><span lang="EN-GB" style="font-size:10.0pt;font-family:"Lucida Console";color:#1F497D">#pragma omp critical</span><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D"> not enforcing atomicity between
the different libraries).<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D">This is one of the main reasons why we have the libgomp entrypoints, since without them you would necessarily have two OpenMP runtimes in the process
when you linked gcc compiled OpenMP code and clang compiled OpenMP code. That would lead to all the issues outlined above.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D">The only sane approach is to attempt to ensure that there’s only a single copy of the OpenMP runtime in any process. As soon as you statically link
an OpenMP runtime into some component (executable, shared-library) that becomes impossible to guarantee in general.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#984807;mso-style-textfill-fill-color:#984807;mso-style-textfill-fill-alpha:100.0%">It sounds like the flush operation is really designed for the benefit of OpenMP implementations that aren't just using CPU threads. <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D">You clearly need the flush operation even when compiling for CPUs in a cache coherent node to ensure that the compiler has moved data associated
with the flushed variables from registers into store, and that an appropriate memory fence is executed. The real issue is whether, once the compiler has issued the necessary stores, the runtime can do anything smarter than use a full memory-fence if it knows
which variables were flushed. Clearly that has never proved to be the case yet!<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D">We actually implemented “Cluster OpenMP” (you can probably Google it if you care), which implemented OpenMP on a “shared nothing” cluster. It exploited
OpenMP’s relaxed memory consistency models and a Treadmarks derived shared virtual memory implementation, but clearly even that didn’t need to pass any extra information into the flush function in the runtime, since otherwise this interface would already have
been fixed!<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D">Anyway, we’ll have a patch to change the interface specification in the next few weeks, I hope.
<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D">-- Jim<br>
<br>
James Cownie <james.h.cownie@intel.com><br>
SSG/DPD/TCAR (Technical Computing, Analyzers and Runtimes)<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D">Tel: +44 117 9071438</span><span lang="EN-GB" style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:10.0pt;font-family:"Verdana",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> John McCall [mailto:rjmccall@gmail.com]
<br>
<b>Sent:</b> Wednesday, November 26, 2014 5:31 AM<br>
<b>To:</b> Cownie, James H<br>
<b>Cc:</b> Hal Finkel; reviews+D6292+public+0f122151bdc64d55@reviews.llvm.org; cfe-commits@cs.uiuc.edu; a bataev; fraggamuffin@gmail.com; ejstotzer@gmail.com<br>
<b>Subject:</b> Re: [PATCH] [OPENMP] Codegen for "omp flush" directive.<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On Tue, Nov 25, 2014 at 5:16 AM, Cownie, James H <<a href="mailto:james.h.cownie@intel.com" target="_blank">james.h.cownie@intel.com</a>> wrote:<o:p></o:p></p>
<div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm">
<p class="MsoNormal">From a runtime point of view we need to preserve backwards binary compatibility, so we can't change the interface to the current interface function to introduce a count (because that old code won't set it).<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Okay. I wasn't sure if you were guaranteeing backwards binary compatibility with existing compiles or not. The way that I understand you're currently deploying the runtime — linking in a static library that's distributed with the compiler
— does not actually require backwards binary compatibility as far as I can tell. But if you're making that guarantee, then I agree we can't break it.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm">
<p class="MsoNormal">I would therefore prefer to do this<br>
<br>
1) Change the prototype for the existing function so that it becomes a function with no arguments. (Effectively your #1). Since no existing code passes arguments, this is fine.<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Changing from a variadic to a non-variadic function is technically allowed to break the ABI, e.g. by passing the non-optional arguments in a different way. I don't know of any platforms where it does actually change for the standard C
calling convention, though — I know of platforms (like MIPS and Apple's ARM64) that change how variadic arguments are passed, but not of any that affect earlier arguments. And I *think* there's no issue with new compilations calling old runtimes on platforms
like x86-64 where variadic functions expect extra information, because I don't know of any compilers that actually crash on an out-of-range value in %al.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">So this is probably fine, but if you want to be 100% paranoid, the safest thing to do is to leave it variadic but document that the variadic arguments are useless.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm">
<p class="MsoNormal">2) If/when we decide that it is useful to pass extra information, we design that as a new function. I can't see a need for that in a cache-coherent system, and in a non-coherent system the interface needs to be more complicated than just
a set of pointers (because you also need to know how big the object at the target of the pointer is). So this gets complicated, but, since we're not going to implement it until we need it and understand the problem better, that’s fine. <o:p></o:p></p>
</blockquote>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm">
<p class="MsoNormal"><br>
I'm avoiding your #2 because I don't believe it is sufficient.<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I'm fine with that decision; it's your call. I hadn't looked to see what these pointers were supposed to be; I guess I was assuming that they were variable descriptors or something, not literally just pointers to objects of unknown size.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">It sounds like the flush operation is really designed for the benefit of OpenMP implementations that aren't just using CPU threads. Given that your runtime's API is really only designed to be good enough for CPU threads, there's no reason
to future proof it, because current CPUs don't let us do efficient partial memory barriers anyway. If somebody comes out with a CPU that does, we can embellish the API then.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">John.<o:p></o:p></p>
</div>
</div>
</div>
</div>
</div>
<p>---------------------------------------------------------------------<br>
Intel Corporation (UK) Limited<br>
Registered No. 1134945 (England)<br>
Registered Office: Pipers Way, Swindon SN3 1RJ<br>
VAT No: 860 2173 47</p>
<p>This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.</p></body>
</html>