<div dir="rtl"><div dir="ltr">The testcase from <span style="font-size:12.8000001907349px">r245459 was not reverted and still in SVN.</span></div><div dir="ltr"><span style="font-size:12.8000001907349px"><br></span></div></div><div class="gmail_extra"><div dir="ltr"><br><div class="gmail_quote">2015-08-21 2:05 GMT+03:00 Martell Malone <span dir="ltr"><<a href="mailto:martellmalone@gmail.com" target="_blank">martellmalone@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 .8ex;border-left:1px #ccc solid;border-right:1px #ccc solid;padding-left:1ex;padding-right:1ex"><div><div><div>I feel very silly now.<br></div>After testing the testcase again on svn it still works.<br></div>It appears the OP was looking for this patch to go onto the 3.6 branch and was applying my patch to that.<br><br></div><div>I'll know in future to recheck the testcase afterwards myself in future.<br><br></div><div>Apologies for the noise guys.<br><br></div><div>Yaron I think the test case from r245459 would be useful to ensure it is never broken in the future?<br></div><div>Would you be able to recommit the test case?<br></div><div><br></div><div>Kind Regards<span class="HOEnZb"><font color="#888888"><br></font></span></div><span class="HOEnZb"><font color="#888888"><div>Martell<br></div><div><br></div></font></span></blockquote></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Aug 20, 2015 at 3:57 PM, Martell Malone <span dir="ltr"><<a href="mailto:martellmalone@gmail.com" target="_blank">martellmalone@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">There is no testcase for <span style="font-size:12.8000001907349px">PR24398 </span>nor the OP reporting the problem was actually solved. Martell?</blockquote></span><div>I'm just re-looking through it now.<br></div><div><span><br>X86TargetInfo sets </span>LongDoubleFormat = &llvm::APFloat::x87DoubleExtended;<br><span>X86_64TargetInfo then</span><span> sets </span><span>LongDoubleWidth =</span>
LongDoubleAlign = <span>128</span>;<br>X86_32<span>TargetInfo then</span><span> sets </span><span>LongDoubleWidth =</span><span> 96</span>;
LongDoubleAlign = <span>32</span>;<br></div><div><br>From this I can see that the patch I committed actually doesn't change anything but only breaks mingw x86.<br></div><div><div>I can only see these values changed in <span>Microsoft*TargetInfo classes which is not a parent of MINGW<br></span></div><br></div><div>When I submitted the patch I just wanted to explicitly set the values in <span>MinGWX86_64TargetInfo to ensure it was in fact that.<br></span></div><div><span>It seemed as though it was not inheriting the value from the root parent class somehow.<br><br></span></div><div><div>I was told on irc that it did fix the bug which is even stranger.<br></div>I'm actually at a bit of a loss as to what the proper fix to this is then.<br><br>Apologies for breaking mingw i686 long double.<br><br></div><div>I will do up a test case and try to find the actual cause of the long double bug and reopen the issue.<br></div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Aug 20, 2015 at 3:09 PM, Yaron Keren <span dir="ltr"><<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div dir="ltr">Hi, I've just done this exactly this in r245618 (32 bit) and r245620 (64 bits).</div><div dir="ltr"><br></div><div dir="ltr">mingw i686 long double values were correct before <span style="font-size:12.8000001907349px">r245084 </span>and wrong after<span style="font-size:12.8000001907349px"> it.</span></div><div dir="ltr"><span style="font-size:12.8000001907349px">mingw x86_64 long double values were not modified at all by </span><span style="font-size:12.8000001907349px">r245084 for the reason you stated, so I agree and do not see how this non-change can solve anything</span>. There is no testcase for <span style="font-size:12.8000001907349px">PR24398 </span>nor the OP reporting the problem was actually solved. Martell?</div><div dir="ltr"><br></div><div dir="ltr"><span style="font-size:12.8000001907349px">About PR24398, long double support was in clang long ago and b</span><span style="font-size:12.8000001907349px">oth code examples compile and run correctly with current svn (without </span><span style="font-size:12.8000001907349px">r245084) and </span><span style="font-size:12.8000001907349px">gcc version 5.1.0 (i686-posix-dwarf-rev0, Built by MinGW-W64 project).</span></div><div dir="ltr"><span style="font-size:12.8000001907349px">It's not x86_64 but as said </span><span style="font-size:12.8000001907349px">r245084</span><span style="font-size:12.8000001907349px"> didn't actually modify x86_64 configuration.</span></div><div dir="ltr"><br></div><div dir="ltr"><br></div><div dir="ltr"><span style="font-size:12.8000001907349px"><br></span></div></div><div><div><div class="gmail_extra"><div dir="ltr"><br><div class="gmail_quote">2015-08-21 0:52 GMT+03:00 Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 .8ex;border-left:1px #ccc solid;border-right:1px #ccc solid;padding-left:1ex;padding-right:1ex">OK, so here's the problem:<div><br></div><div>The right way to fix this seems to be to delete the assignments to LongDouble* from the MinGWX86_32TargetInfo constructor; the X86_32TargetInfo and X86TargetInfo base classes already set them to the right values. Likewise we can delete the assignments to LongDouble* from the MinGWX86_64TargetInfo constructor for the same reason.</div><div><br></div><div>But... that completely reverts Martell's r245084, which apparently fixed PR24398. So you two need to figure out what the actual problem is here, and what the right fix is. r245084 didn't provide any test cases, and had no apparent effect other than to break long double for mingw32; did it really fix PR24398 (and if so, how)?<div><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Aug 20, 2015 at 2:27 PM, Yaron Keren <span dir="ltr"><<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div dir="ltr">OK, based on testing, mingw i686 aligns long doubles to 4 bytes:<br></div><div dir="ltr"><br></div><div dir="ltr"><span><div dir="ltr">sh-4.3$ cat < a.cpp</div><div dir="ltr">#include <iostream></div><div dir="ltr">int main() {</div></span><div dir="ltr"> struct {</div><div dir="ltr"> char c[1];</div><div dir="ltr"> long double d;</div><div dir="ltr"> } s;</div><div dir="ltr"> std::cout<<&s.c<<std::endl;</div><div dir="ltr"> std::cout<<&s.d<<std::endl;</div><div dir="ltr">}</div><div dir="ltr">sh-4.3$ g++ a.cpp&&./a.exe</div><div dir="ltr">0x28fea0</div><div dir="ltr">0x28fea4</div><div dir="ltr"><br></div><div>I'll fix that.</div><div><br></div></div></div><div><div><div class="gmail_extra"><div dir="ltr"><br><div class="gmail_quote">2015-08-21 0:13 GMT+03:00 Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 .8ex;border-left:1px #ccc solid;border-right:1px #ccc solid;padding-left:1ex;padding-right:1ex"><div class="gmail_extra"><div class="gmail_quote"><div><div>On Wed, Aug 19, 2015 at 11:42 AM, Yaron Keren <span dir="ltr"><<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div dir="ltr">Yes, it looks like a legacy issue. Documentation says so:</div><div dir="ltr"><br></div><div dir="ltr"><div dir="ltr"><font color="#1155cc"><span style="font-size:12.8000001907349px"><u><a href="https://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/i386-and-x86-64-Options.html" target="_blank">https://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/i386-and-x86-64-Options.html</a></u></span></font><br></div><div dir="ltr" style="font-size:12.8000001907349px"><br></div><div dir="ltr" style="font-size:12.8000001907349px"><dt style="color:rgb(0,0,0);font-family:'Times New Roman';font-size:medium"><code>-m96bit-long-double</code></dt><dt style="color:rgb(0,0,0);font-family:'Times New Roman';font-size:medium"><code>-m128bit-long-double</code></dt><dd style="color:rgb(0,0,0);font-family:'Times New Roman';font-size:medium"><a name="14f4d5bdd143106e_14f4d549052b6f7d_14f4d2931f3300ac_14f4d19291ec8c03_14f4d02fe2807b01_14f4cf579915b31d_14f47455fcf8cc4c_14f4743761dc8e58_index-m96bit-long-double-1451"></a><a name="14f4d5bdd143106e_14f4d549052b6f7d_14f4d2931f3300ac_14f4d19291ec8c03_14f4d02fe2807b01_14f4cf579915b31d_14f47455fcf8cc4c_14f4743761dc8e58_index-m128bit-long-double-1452"></a>These switches control the size of <code>long double</code> type. The i386 application binary interface specifies the size to be 96 bits, so <samp><span>-m96bit-long-double</span></samp> is the default in 32-bit mode.<p>Modern architectures (Pentium and newer) prefer <code>long double</code> to be aligned to an 8- or 16-byte boundary. In arrays or structures conforming to the ABI, this is not possible. So specifying <samp><span>-m128bit-long-double</span></samp> aligns <code>long double</code> to a 16-byte boundary by padding the <code>long double</code> with an additional 32-bit zero.</p><p>In the x86-64 compiler, <samp><span>-m128bit-long-double</span></samp> is the default choice as its ABI specifies that <code>long double</code> is aligned on 16-byte boundary.</p><p>Notice that neither of these options enable any extra precision over the x87 standard of 80 bits for a <code>long double</code>.</p><p><strong>Warning:</strong> if you override the default value for your target ABI, this changes the size of structures and arrays containing <code>long double</code> variables, as well as modifying the function calling convention for functions taking <code>long double</code>. Hence they are not binary-compatible with code compiled without that switch. </p></dd></div></div><div dir="ltr"><br></div><div dir="ltr">And practical testing agrees:<br></div><div dir="ltr"><br></div><div dir="ltr"><div dir="ltr">sh-4.3$ cat < a.cpp</div><div dir="ltr">#include <iostream></div><div dir="ltr">int main() {</div><div dir="ltr"> long double a;</div><div dir="ltr"> std::cout<<sizeof(a)<<std::endl;</div><div dir="ltr">}</div><div dir="ltr">sh-4.3$ g++ -v</div><div dir="ltr">Using built-in specs.</div><div dir="ltr">COLLECT_GCC=C:\mingw32\bin\g++.exe</div><div dir="ltr">COLLECT_LTO_WRAPPER=C:/mingw32/bin/../libexec/gcc/i686-w64-mingw32/5.1.0/lto-wrapper.exe</div><div dir="ltr">Target: i686-w64-mingw32</div><div dir="ltr">Configured with: ../../../src/gcc-5.1.0/configure --host=i686-w64-mingw32 --build=i686-w64-mingw32 --target=i686-w64-mingw32 --prefix=/mingw32 --with-sysroot=/c/mingw510/i686-510-posix-dwarf-rt_v4-rev0/mingw32 --with-gxx-include-dir=/mingw32/i686-w64-mingw32/include/c++ --enable-shared --enable-static --disable-multilib --enable-languages=c,c++,fortran,objc,obj-c++,lto --enable-libstdcxx-time=yes --enable-threads=posix --enable-libgomp --enable-libatomic --enable-lto --enable-graphite --enable-checking=release --enable-fully-dynamic-string --enable-version-specific-runtime-libs --disable-sjlj-exceptions --with-dwarf2 --disable-isl-version-check --disable-libstdcxx-pch --disable-libstdcxx-debug --enable-bootstrap --disable-rpath --disable-win32-registry --disable-nls --disable-werror --disable-symvers --with-gnu-as --with-gnu-ld --with-arch=i686 --with-tune=generic --with-libiconv --with-system-zlib --with-gmp=/c/mingw510/prerequisites/i686-w64-mingw32-static --with-mpfr=/c/mingw510/prerequisites/i686-w64-mingw32-static --with-mpc=/c/mingw510/prerequisites/i686-w64-mingw32-static --with-isl=/c/mingw510/prerequisites/i686-w64-mingw32-static --with-pkgversion='i686-posix-dwarf-rev0, Built by MinGW-W64 project' --with-bugurl=<a href="http://sourceforge.net/projects/mingw-w64" target="_blank">http://sourceforge.net/projects/mingw-w64</a> CFLAGS='-O2 -pipe -I/c/mingw510/i686-510-posix-dwarf-rt_v4-rev0/mingw32/opt/include -I/c/mingw510/prerequisites/i686-zlib-static/include -I/c/mingw510/prerequisites/i686-w64-mingw32-static/include' CXXFLAGS='-O2 -pipe -I/c/mingw510/i686-510-posix-dwarf-rt_v4-rev0/mingw32/opt/include -I/c/mingw510/prerequisites/i686-zlib-static/include -I/c/mingw510/prerequisites/i686-w64-mingw32-static/include' CPPFLAGS= LDFLAGS='-pipe -L/c/mingw510/i686-510-posix-dwarf-rt_v4-rev0/mingw32/opt/lib -L/c/mingw510/prerequisites/i686-zlib-static/lib -L/c/mingw510/prerequisites/i686-w64-mingw32-static/lib -Wl,--large-address-aware'</div><div dir="ltr">Thread model: posix</div><div dir="ltr">gcc version 5.1.0 (i686-posix-dwarf-rev0, Built by MinGW-W64 project)</div><div dir="ltr"><br></div><div dir="ltr">sh-4.3$ g++ a.cpp</div><div dir="ltr">sh-4.3$ ./a.exe<br></div><div dir="ltr">12</div><div dir="ltr"><br></div><div>Without the patch clang outputs 16 and seg faults on a boost::math example.</div></div></div></blockquote><div><br></div></div></div><div>None of that answers my question. Our default GCC-compatible behavior for x86_32 long double is to give it 4-byte alignment, 12-byte size, and this matches the behavior I observe with GCC. Does MinGW /really/ deviate from this and give long double a 16-byte alignment?</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div><div><div class="gmail_extra"><div dir="ltr"><div class="gmail_quote">2015-08-19 21:29 GMT+03:00 Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><div class="gmail_quote"><div><div>On Wed, Aug 19, 2015 at 10:02 AM, Yaron Keren via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: yrnkrn<br>
Date: Wed Aug 19 12:02:32 2015<br>
New Revision: 245459<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=245459&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=245459&view=rev</a><br>
Log:<br>
According to i686 ABI, long double size on x86 is 12 bytes not 16 bytes.<br>
See<br>
<a href="https://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/i386-and-x86-64-Options.html" rel="noreferrer" target="_blank">https://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/i386-and-x86-64-Options.html</a><br>
<br>
<br>
Added:<br>
cfe/trunk/test/CodeGen/mingw-long-double-size.c<br>
Modified:<br>
cfe/trunk/lib/Basic/Targets.cpp<br>
<br>
Modified: cfe/trunk/lib/Basic/Targets.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=245459&r1=245458&r2=245459&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=245459&r1=245458&r2=245459&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Basic/Targets.cpp (original)<br>
+++ cfe/trunk/lib/Basic/Targets.cpp Wed Aug 19 12:02:32 2015<br>
@@ -3785,7 +3785,8 @@ class MinGWX86_32TargetInfo : public Win<br>
public:<br>
MinGWX86_32TargetInfo(const llvm::Triple &Triple)<br>
: WindowsX86_32TargetInfo(Triple) {<br>
- LongDoubleWidth = LongDoubleAlign = 128;<br>
+ LongDoubleWidth = 96;<br>
+ LongDoubleAlign = 128;<br></blockquote><div><br></div></div></div><div>Is this really correct? It's deeply suspicious that the size is not a multiple of the alignment.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
LongDoubleFormat = &llvm::APFloat::x87DoubleExtended;<br>
}<br>
void getTargetDefines(const LangOptions &Opts,<br>
<br>
Added: cfe/trunk/test/CodeGen/mingw-long-double-size.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/mingw-long-double-size.c?rev=245459&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/mingw-long-double-size.c?rev=245459&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGen/mingw-long-double-size.c (added)<br>
+++ cfe/trunk/test/CodeGen/mingw-long-double-size.c Wed Aug 19 12:02:32 2015<br>
@@ -0,0 +1,5 @@<br>
+// RUN: %clang_cc1 -triple i686-pc-windows-gnu -S %s -o - | FileCheck %s -check-prefix=CHECK_I686<br>
+// CHECK_I686: lda,12<br>
+// RUN: %clang_cc1 -triple x86_64-pc-windows-gnu -S %s -o - | FileCheck %s -check-prefix=CHECK_X86_64<br>
+// CHECK_X86_64: lda,16<br>
+long double lda;<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></span></div><br></div></blockquote></div></div></div></div></div></div>
</blockquote></div></div></div><br></div></blockquote></div></div></div>
</div></div></blockquote></div><br></div></div></div></div></blockquote></div></div></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></div></div>