r245459 - According to i686 ABI, long double size on x86 is 12 bytes not 16 bytes.

Sat Sep 19 05:29:39 PDT 2015

I can't reproduce sizeof = 12 when targetting 64 bits, it is 16
(LongDoubleWidth = 128) and have been this way since r55036.  It may be
that both 32 bit mingw and 64 bit mingw are installed and clang is
targetting the 32 bits while gcc targets 64 bits.

2015-09-19 15:24 GMT+03:00 Martell Malone <martellmalone at gmail.com>:

> 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.keren at gmail.com>
> wrote:
>
>> Thanks, I have replied there.
>>
>> 2015-09-19 13:33 GMT+03:00 Hal Finkel <hfinkel at 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 at lists.llvm.org>
>>> > To: "Martell Malone" <martellmalone at gmail.com>
>>> > Cc: "Richard Smith" <richard at metafoo.co.uk>, "cfe-commits" <
>>> cfe-commits at 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 < martellmalone at 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 <
>>> > martellmalone at 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.keren at 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 < richard at 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.keren at 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 < richard at metafoo.co.uk > :
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > On Wed, Aug 19, 2015 at 11:42 AM, Yaron Keren < yaron.keren at 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 < richard at metafoo.co.uk > :
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > On Wed, Aug 19, 2015 at 10:02 AM, Yaron Keren via cfe-commits <
>>> > cfe-commits at 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 at lists.llvm.org
>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > _______________________________________________
>>> > cfe-commits mailing list
>>> > cfe-commits at lists.llvm.org
>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>> >
>>>
>>> --
>>> Hal Finkel
>>> Assistant Computational Scientist
>>> Leadership Computing Facility
>>> Argonne National Laboratory
>>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150919/e719123b/attachment-0001.html>