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

Martell Malone via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 20 15:57:11 PDT 2015


>
> 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_32TargetInfo 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 both 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
>>>>> <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/8bc67802/attachment-0001.html>


More information about the cfe-commits mailing list