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

Thu Aug 20 14:52:15 PDT 2015

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
>>> <https://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/i386-and-x86-64-Options.html>*
>>>
>>> -m96bit-long-double-m128bit-long-doubleThese 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
>>>>>
>>>>
>>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150820/63e24f4d/attachment.html>