<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=us-ascii">
<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;}
/* 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:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle19
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle20
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle21
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle23
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
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="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal">Yes, that’s essentially what I’m thinking.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I’d be okay with ignoring the crash issue, if someone comes up with a patch for the missed optimization. (But we’d still need to evaluate the optimization on its own merits.)<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">-Eli <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> Seth Brenith <Seth.Brenith@microsoft.com> <br>
<b>Sent:</b> Wednesday, October 30, 2019 8:52 AM<br>
<b>To:</b> Eli Friedman <efriedma@quicinc.com>; LLVM Dev <llvm-dev@lists.llvm.org><br>
<b>Subject:</b> [EXT] RE: [llvm-dev] unnecessary reload of 8-byte struct on i386<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Hi Eli,<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks for the thoughtful response! To check my understanding, is the following statement true? Given infinite time/resources and a clearer understanding of the exact trigger for this CPU bug, we would like two separate changes: one change
in Clang to emit different IR for x86 like we do for ARM, and a second change to search for this instruction sequence and transform it into something harmless.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">However, I don’t really know what “something harmless” would be, and this issue affects only a tiny minority of machines, and it feels a little silly to stuff an increasing number of nops in between those instructions until we stop getting
these crash reports, so I think the reasonable next step is to leave it alone. Please let me know if you think otherwise, and thanks again!<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">-Seth<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> Eli Friedman <<a href="mailto:efriedma@quicinc.com">efriedma@quicinc.com</a>>
<br>
<b>Sent:</b> Monday, October 28, 2019 4:34 PM<br>
<b>To:</b> Seth Brenith <<a href="mailto:Seth.Brenith@microsoft.com">Seth.Brenith@microsoft.com</a>>; LLVM Dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>><br>
<b>Subject:</b> [EXTERNAL] RE: [llvm-dev] unnecessary reload of 8-byte struct on i386<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">It’s not unheard of for the compiler to work around CPU bugs… but generally, we try to do it in a more disciplined way: with a code generation pass that actually detects the bad sequence in question. I’m not really happy with trying to
“get lucky” here to avoid a bug.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">This particular missed optimization is a known issue with the LLVM IR representation of “byval”; there’s an implied copy that can’t be easily optimized away at the IR level due to calling convention rules. For ARM targets, clang works
around this issue by changing the IR it generates; see ARMABIInfo::classifyArgumentType in clang/lib/CodeGen/TargetInfo.cpp .<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">-Eli<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> llvm-dev <<a href="mailto:llvm-dev-bounces@lists.llvm.org">llvm-dev-bounces@lists.llvm.org</a>>
<b>On Behalf Of </b>Seth Brenith via llvm-dev<br>
<b>Sent:</b> Friday, October 25, 2019 2:29 PM<br>
<b>To:</b> <a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
<b>Subject:</b> [EXT] [llvm-dev] unnecessary reload of 8-byte struct on i386<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Hello folks,<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I’ve recently been looking at the generated code for a few functions in Chromium while investigating crashes, and I came across a curious pattern. A smallish repro case is available at
<a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgodbolt.org%2Fz%2FDsu1WI&data=02%7C01%7CSeth.Brenith%40microsoft.com%7C3fbf0efccbcd4bb3518c08d75bff49e5%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637079025113565811&sdata=N4cEn%2BsUBdqAr6uhQHDaAnxqlyBk1hfcRGj%2FkO%2F%2Fns4%3D&reserved=0">
https://godbolt.org/z/Dsu1WI</a> . In that case, the function Assembler::emit_arith receives a struct (Operand) by value and passes it by value to another function. That struct is 8 bytes long, so the -O3 generated code uses movsd to copy it up the stack. However,
we end up with some loads that aren’t needed, as in the following chunk:<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span style="color:blue">movsd</span><span style="color:black">
</span><span style="color:#4864AA">xmm0</span><span style="color:black">, </span>
<span style="color:teal">qword</span><span style="color:black"> </span><span style="color:teal">ptr</span><span style="color:black"> [</span><span style="color:#4864AA">ecx</span><span style="color:black">]
</span><span style="color:green"># xmm0 = mem[0],zero</span><o:p></o:p></p>
<p class="MsoNormal"><span style="color:blue">mov</span><span style="color:black">
</span><span style="color:teal">dword</span><span style="color:black"> </span><span style="color:teal">ptr</span><span style="color:black"> [</span><span style="color:#4864AA">esp</span><span style="color:black"> +
</span><span style="color:#09885A">24</span><span style="color:black">], </span><span style="color:#4864AA">edx</span><o:p></o:p></p>
<p class="MsoNormal"><span style="color:blue">movsd</span><span style="color:black">
</span><span style="color:teal">qword</span><span style="color:black"> </span><span style="color:teal">ptr</span><span style="color:black"> [</span><span style="color:#4864AA">esp</span><span style="color:black"> +
</span><span style="color:#09885A">40</span><span style="color:black">], </span><span style="color:#4864AA">xmm0</span><o:p></o:p></p>
<p class="MsoNormal"><span style="color:blue">movsd</span><span style="color:black">
</span><span style="color:#4864AA">xmm0</span><span style="color:black">, </span>
<span style="color:teal">qword</span><span style="color:black"> </span><span style="color:teal">ptr</span><span style="color:black"> [</span><span style="color:#4864AA">esp</span><span style="color:black"> +
</span><span style="color:#09885A">40</span><span style="color:black">] </span><span style="color:green"># xmm0 = mem[0],zero</span><o:p></o:p></p>
<p class="MsoNormal"><span style="color:blue">movsd</span><span style="color:black">
</span><span style="color:teal">qword</span><span style="color:black"> </span><span style="color:teal">ptr</span><span style="color:black"> [</span><span style="color:#4864AA">esp</span><span style="color:black"> +
</span><span style="color:#09885A">8</span><span style="color:black">], </span><span style="color:#4864AA">xmm0</span><o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">As far as I can tell, the fourth line has no effect. On its own, that seems like a small missed opportunity for optimization. However, this sequence of instructions also appears to trigger a hardware bug on a small fraction of devices which
sometimes end up storing zero at esp+8. A more in-depth discussion of that issue can be found here:
<a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.chromium.org%2Fp%2Fv8%2Fissues%2Fdetail%3Fid%3D9774&data=02%7C01%7CSeth.Brenith%40microsoft.com%7C3fbf0efccbcd4bb3518c08d75bff49e5%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637079025113575803&sdata=x1t77z5dpjMuQW3xawNP7jbxMcS12ZlhUAvZRAbBwbQ%3D&reserved=0">
https://bugs.chromium.org/p/v8/issues/detail?id=9774</a> .<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I’m hoping that getting rid of the second load in the sequence above would appease these misbehaving machines (though of course I don’t know that it would), as well as making the code a little smaller for everybody else. Does that sound
like a reasonable idea? Would LLVM be interested in a patch related to eliminating reloads like this? Does anybody have advice about where I should start looking, or any reasons it would be very hard to achieve the result I’m hoping for?<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
<p class="MsoNormal">Seth<o:p></o:p></p>
</div>
</div>
</div>
</body>
</html>