<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, Oct 31, 2018 at 7:39 AM <<a href="mailto:paul.robinson@sony.com">paul.robinson@sony.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div class="m_-465925786703764835WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Also, don't we usually put ABI changes under an ABI compatibility check? This would be making Clang incompatible with itself.</span> </p></div></div></blockquote><div><br></div><div>I don't think it's worth it because nobody can use i128 in a meaningful way on x64 Windows right now. If you do i128 multiplication or division, LLVM will generate calls to the *ti* (for "tetrainteger") runtime functions in compiler-rt. Without this change, clang compiles those *ti* functions with a different calling convention from the one that LLVM uses, because LLVM is trying to match libgcc's calling conventions.</div><div><br></div><div>Clang is in a situation where all language extensions are supported by default, all the time, for all targets, whether or not anyone has thought about ABI stability for that particular feature and target combination. I don't think it's really tenable to promise ABI stability for every feature on every platform, ever. We have to give ourselves the flexibility to implement one feature (e.g. i128) generically for all targets, and then fill in the target specific blanks later. For example, I have no idea about the status of OpenMP or ObjC on Windows right now, and I certainly hope that we do not promise ABI stability with whatever our current emergent behavior is.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div class="m_-465925786703764835WordSection1">
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> cfe-commits [mailto:<a href="mailto:cfe-commits-bounces@lists.llvm.org" target="_blank">cfe-commits-bounces@lists.llvm.org</a>]
<b>On Behalf Of </b>Richard Trieu via cfe-commits<br>
<b>Sent:</b> Tuesday, October 30, 2018 10:16 PM<br>
<b>To:</b> Reid Kleckner<br>
<b>Cc:</b> cfe-commits<br>
<b>Subject:</b> Re: r345676 - [Win64] Handle passing i128 by value<u></u><u></u></span></p>
</div>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<div>
<div>
<div>
<p class="MsoNormal">I have reverted this in r345691 because it caused test CodeGen/mingw-long-double.c to start failing.</p></div></div></div></div></div></div></div></div></blockquote><div><br></div><div>Thanks!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div class="m_-465925786703764835WordSection1"><div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt"><div><blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in"><p class="MsoNormal">
+ case BuiltinType::LongDouble:<br>
+ // Mingw64 GCC uses the old 80 bit extended precision floating point<br>
+ // unit. It passes them indirectly through memory.<br>
+ if (IsMingw64) {<br>
+ const llvm::fltSemantics *LDF = &getTarget().getLongDoubleFormat();<br>
+ if (LDF == &llvm::APFloat::x87DoubleExtended())<br>
+ return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);<br>
+ break;<br>
+ }<br></p></blockquote></div></div></div></div></blockquote><div>The bug is that I rewrote my patch at the last minute to use a switch here, and I forgot the break. -Wimplicit-fallthrough would've caught this. I can't find a bug for enabling this on our codebase, but I know there have been cleanup efforts in the past.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div class="m_-465925786703764835WordSection1"><div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt"><div><blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in"><p class="MsoNormal">
+ case BuiltinType::Int128:<br>
+ case BuiltinType::UInt128:<br>
+ // If it's a parameter type, the normal ABI rule is that arguments larger<br>
+ // than 8 bytes are passed indirectly. GCC follows it. We follow it too,<br>
+ // even though it isn't particularly efficient.<br>
+ if (!IsReturnType)<br>
+ return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);<br>
+<br>
+ // Mingw64 GCC returns i128 in XMM0. Coerce to v2i64 to handle that.<br>
+ // Clang matches them for compatibility.<br>
+ return ABIArgInfo::getDirect(<br>
+ llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()), 2));<br>
+<br>
+ default:<br>
+ break;<br>
+ }<br>
}<br><u></u></p>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote></div></div>