For reference It was raised again here also https://github.com/Alexpux/MINGW-packages/issues/722 <https://github.com/Alexpux/MINGW-packages/issues/722#issuecomment-141642907>
"that is extended from X86TargetInfo Which sets LongDoubleFormat = &llvm::APFloat::x87DoubleExtended;" On Saturday, September 19, 2015, Yaron Keren <yaron.ke...@gmail.com <javascript:_e(%7B%7D,'cvml','yaron.ke...@gmail.com');>> wrote: > Thanks, I have replied there. > > 2015-09-19 13:33 GMT+03:00 Hal Finkel <hfin...@anl.gov>: > >> FYI: https://llvm.org/bugs/show_bug.cgi?id=24398 was just reopened >> pointing to a lack of resolution here. >> >> -Hal >> >> ----- Original Message ----- >> > From: "Yaron Keren via cfe-commits" <cfe-commits@lists.llvm.org> >> > To: "Martell Malone" <martellmal...@gmail.com> >> > Cc: "Richard Smith" <rich...@metafoo.co.uk>, "cfe-commits" < >> cfe-commits@lists.llvm.org> >> > Sent: Friday, August 21, 2015 2:47:50 AM >> > Subject: Re: r245459 - According to i686 ABI, long double size on x86 >> is 12 bytes not 16 bytes. >> > >> > >> > >> > >> > The testcase from r245459 was not reverted and still in SVN. >> > >> > >> > >> > >> > >> > 2015-08-21 2:05 GMT+03:00 Martell Malone < martellmal...@gmail.com > >> > : >> > >> > >> > >> > >> > >> > I feel very silly now. >> > After testing the testcase again on svn it still works. >> > It appears the OP was looking for this patch to go onto the 3.6 >> > branch and was applying my patch to that. >> > >> > >> > I'll know in future to recheck the testcase afterwards myself in >> > future. >> > >> > >> > Apologies for the noise guys. >> > >> > >> > Yaron I think the test case from r245459 would be useful to ensure it >> > is never broken in the future? >> > >> > Would you be able to recommit the test case? >> > >> > >> > >> > Kind Regards >> > >> > Martell >> > >> > >> > >> > >> > >> > >> > >> > On Thu, Aug 20, 2015 at 3:57 PM, Martell Malone < >> > martellmal...@gmail.com > wrote: >> > >> > >> > >> > >> > >> > There is no testcase for PR24398 nor the OP reporting the problem was >> > actually solved. Martell? >> > I'm just re-looking through it now. >> > >> > >> > X86TargetInfo sets LongDoubleFormat = >> > &llvm::APFloat::x87DoubleExtended; >> > X86_64TargetInfo then sets LongDoubleWidth = LongDoubleAlign = 128 ; >> > X86_32 TargetInfo then sets LongDoubleWidth = 96 ; LongDoubleAlign = >> > 32 ; >> > >> > >> > From this I can see that the patch I committed actually doesn't >> > change anything but only breaks mingw x86. >> > >> > >> > I can only see these values changed in Microsoft*TargetInfo classes >> > which is not a parent of MINGW >> > >> > >> > When I submitted the patch I just wanted to explicitly set the values >> > in MinGWX86_64TargetInfo to ensure it was in fact that. >> > >> > It seemed as though it was not inheriting the value from the root >> > parent class somehow. >> > >> > >> > >> > I was told on irc that it did fix the bug which is even stranger. >> > I'm actually at a bit of a loss as to what the proper fix to this is >> > then. >> > >> > Apologies for breaking mingw i686 long double. >> > >> > >> > I will do up a test case and try to find the actual cause of the long >> > double bug and reopen the issue. >> > >> > >> > >> > >> > >> > On Thu, Aug 20, 2015 at 3:09 PM, Yaron Keren < yaron.ke...@gmail.com >> > > wrote: >> > >> > >> > >> > >> > Hi, I've just done this exactly this in r245618 (32 bit) and r245620 >> > (64 bits). >> > >> > >> > mingw i686 long double values were correct before r245084 and wrong >> > after it. >> > mingw x86_64 long double values were not modified at all by r245084 >> > for the reason you stated, so I agree and do not see how this >> > non-change can solve anything . There is no testcase for PR24398 nor >> > the OP reporting the problem was actually solved. Martell? >> > >> > >> > About PR24398, long double support was in clang long ago and b oth >> > code examples compile and run correctly with current svn (without >> > r245084) and gcc version 5.1.0 (i686-posix-dwarf-rev0, Built by >> > MinGW-W64 project). >> > It's not x86_64 but as said r245084 didn't actually modify x86_64 >> > configuration. >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > 2015-08-21 0:52 GMT+03:00 Richard Smith < rich...@metafoo.co.uk > : >> > >> > >> > OK, so here's the problem: >> > >> > >> > 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. >> > >> > >> > 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)? >> > >> > >> > >> > >> > >> > On Thu, Aug 20, 2015 at 2:27 PM, Yaron Keren < yaron.ke...@gmail.com >> > > wrote: >> > >> > >> > >> > >> > OK, based on testing, mingw i686 aligns long doubles to 4 bytes: >> > >> > >> > >> > >> > sh-4.3$ cat < a.cpp >> > #include <iostream> >> > int main() { >> > struct { >> > char c[1]; >> > long double d; >> > } s; >> > std::cout<<&s.c<<std::endl; >> > std::cout<<&s.d<<std::endl; >> > } >> > sh-4.3$ g++ a.cpp&&./a.exe >> > 0x28fea0 >> > 0x28fea4 >> > >> > >> > I'll fix that. >> > >> > >> > >> > >> > >> > >> > >> > 2015-08-21 0:13 GMT+03:00 Richard Smith < rich...@metafoo.co.uk > : >> > >> > >> > >> > >> > >> > >> > On Wed, Aug 19, 2015 at 11:42 AM, Yaron Keren < yaron.ke...@gmail.com >> > > wrote: >> > >> > >> > >> > >> > Yes, it looks like a legacy issue. Documentation says so: >> > >> > >> > >> > >> https://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/i386-and-x86-64-Options.html >> > >> > >> > >> > -m96bit-long-double -m128bit-long-double These switches control the >> > size of long double type. The i386 application binary interface >> > specifies the size to be 96 bits, so -m96bit-long-double is the >> > default in 32-bit mode. >> > >> > Modern architectures (Pentium and newer) prefer long double to be >> > aligned to an 8- or 16-byte boundary. In arrays or structures >> > conforming to the ABI, this is not possible. So specifying >> > -m128bit-long-double aligns long double to a 16-byte boundary by >> > padding the long double with an additional 32-bit zero. >> > >> > In the x86-64 compiler, -m128bit-long-double is the default choice as >> > its ABI specifies that long double is aligned on 16-byte boundary. >> > >> > Notice that neither of these options enable any extra precision over >> > the x87 standard of 80 bits for a long double . >> > >> > Warning: if you override the default value for your target ABI, this >> > changes the size of structures and arrays containing long double >> > variables, as well as modifying the function calling convention for >> > functions taking long double . Hence they are not binary-compatible >> > with code compiled without that switch. >> > >> > >> > And practical testing agrees: >> > >> > >> > >> > >> > sh-4.3$ cat < a.cpp >> > #include <iostream> >> > int main() { >> > long double a; >> > std::cout<<sizeof(a)<<std::endl; >> > } >> > sh-4.3$ g++ -v >> > Using built-in specs. >> > COLLECT_GCC=C:\mingw32\bin\g++.exe >> > >> COLLECT_LTO_WRAPPER=C:/mingw32/bin/../libexec/gcc/i686-w64-mingw32/5.1.0/lto-wrapper.exe >> > Target: i686-w64-mingw32 >> > 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= http://sourceforge.net/projects/mingw-w64 >> > 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' >> > Thread model: posix >> > gcc version 5.1.0 (i686-posix-dwarf-rev0, Built by MinGW-W64 project) >> > >> > >> > sh-4.3$ g++ a.cpp >> > sh-4.3$ ./a.exe >> > >> > 12 >> > >> > >> > Without the patch clang outputs 16 and seg faults on a boost::math >> > example. >> > >> > >> > 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? >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > 2015-08-19 21:29 GMT+03:00 Richard Smith < rich...@metafoo.co.uk > : >> > >> > >> > >> > >> > >> > >> > On Wed, Aug 19, 2015 at 10:02 AM, Yaron Keren via cfe-commits < >> > cfe-commits@lists.llvm.org > wrote: >> > >> > >> > Author: yrnkrn >> > Date: Wed Aug 19 12:02:32 2015 >> > New Revision: 245459 >> > >> > URL: http://llvm.org/viewvc/llvm-project?rev=245459&view=rev >> > Log: >> > According to i686 ABI, long double size on x86 is 12 bytes not 16 >> > bytes. >> > See >> > https://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/i386-and-x86-64-Options.html >> > >> > >> > Added: >> > cfe/trunk/test/CodeGen/mingw-long-double-size.c >> > Modified: >> > cfe/trunk/lib/Basic/Targets.cpp >> > >> > Modified: cfe/trunk/lib/Basic/Targets.cpp >> > URL: >> > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=245459&r1=245458&r2=245459&view=diff >> > >> ============================================================================== >> > --- cfe/trunk/lib/Basic/Targets.cpp (original) >> > +++ cfe/trunk/lib/Basic/Targets.cpp Wed Aug 19 12:02:32 2015 >> > @@ -3785,7 +3785,8 @@ class MinGWX86_32TargetInfo : public Win >> > public: >> > MinGWX86_32TargetInfo(const llvm::Triple &Triple) >> > : WindowsX86_32TargetInfo(Triple) { >> > - LongDoubleWidth = LongDoubleAlign = 128; >> > + LongDoubleWidth = 96; >> > + LongDoubleAlign = 128; >> > >> > >> > >> > Is this really correct? It's deeply suspicious that the size is not a >> > multiple of the alignment. >> > >> > >> > LongDoubleFormat = &llvm::APFloat::x87DoubleExtended; >> > } >> > void getTargetDefines(const LangOptions &Opts, >> > >> > Added: cfe/trunk/test/CodeGen/mingw-long-double-size.c >> > URL: >> > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/mingw-long-double-size.c?rev=245459&view=auto >> > >> ============================================================================== >> > --- cfe/trunk/test/CodeGen/mingw-long-double-size.c (added) >> > +++ cfe/trunk/test/CodeGen/mingw-long-double-size.c Wed Aug 19 >> > 12:02:32 2015 >> > @@ -0,0 +1,5 @@ >> > +// RUN: %clang_cc1 -triple i686-pc-windows-gnu -S %s -o - | >> > FileCheck %s -check-prefix=CHECK_I686 >> > +// CHECK_I686: lda,12 >> > +// RUN: %clang_cc1 -triple x86_64-pc-windows-gnu -S %s -o - | >> > FileCheck %s -check-prefix=CHECK_X86_64 >> > +// CHECK_X86_64: lda,16 >> > +long double lda; >> > >> > >> > _______________________________________________ >> > cfe-commits mailing list >> > cfe-commits@lists.llvm.org >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > >> > >> > >> > >> > >> > >> > _______________________________________________ >> > cfe-commits mailing list >> > cfe-commits@lists.llvm.org >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > >> >> -- >> Hal Finkel >> Assistant Computational Scientist >> Leadership Computing Facility >> Argonne National Laboratory >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
