Re: PATCH: PR target/39082: union with long double doesn't follow x86-64 psABI

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Thu, Feb 5, 2009 at 8:12 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Feb 05, 2009 at 08:24:50AM +0100, Uros Bizjak wrote:
>> On Wed, Feb 4, 2009 at 7:26 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> >>> I am not convinced such ABI changes should be done without adding
>> >>> a diagnostic for the cases that changed.  And I agree that release branches
>> >>> should not be touched.
>> >>>
>> >>
>> >> This problem happens deep in the backend. Adding a proper diagnostic
>> >> may not be easy without FE help. I will see what I can do.
>> >>
>> >
>> > Here is the updated patch with warning for ABI change. OK for trunk?
>>
>> Warning w.r.t. to ABI changes was discussed in [1] when the meaning of
>> sync nand builtins was changed. Mark suggested that instead of a
>> warning, a note should be emitted once per compilation file and there
>> should be a way to disable this note/warning.
>>
>> Also, I think we should say at which point the ABI has changed: "The
>> ABI of passing union with long double has changed in GCC 4.4".
>>
>> All these suggestions were implemented in [2]. I think that patch at
>> [2] is a good and accepted example of this approach.
>>
>> [1] http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00760.html
>> [2] http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00806.html
>
> Is this patch OK for trunk?
>
> Thanks.
>
>
> H.J.
> ----
> gcc/
>
> 2009-02-05  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR target/39082
>        * config/i386/i386.c (classify_argument): Pass an aggregate in
>        memory if X87UP class isn't preceded by X87 class.   Warn once
>        about the ABI change when passing union with long double.
>
>        * config/i386/i386.opt (Wunion-with-long-double): New.
>
>        * doc/invoke.texi: Document -Wunion-with-long-double.
>
> gcc/testsuite/
>
> 2009-02-05  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR target/39082
>        * gcc.target/i386/pr39082-1.c: New.
>
>        gcc.target/x86_64/abi/abi-x86_64.exp: Use glob instead of
>        find.
>        (additional_flags): Add -Wno-union-with-long-double".
>
>        * gcc.target/x86_64/abi/args.h (XMM_T): Add _m64 and _m128 if
>        CHECK_M64_M128 is defined.
>        (check_f_arguments): Add "do".
>        (check_vector_arguments): New.
>        (check_m64_arguments): Likewise.
>        (check_m128_arguments): Likewise.
>
>        * gcc.target/x86_64/abi/defines.h: Include <xmmintrin.h>.
>        (CHECK_M64_M128): Define.
>
>        * gcc.target/x86_64/abi/test_m64m128_returning.c: New.  Based
>        on abitest.
>        * gcc.target/x86_64/abi/test_passing_m64m128.c: Likewise.
>
>        * gcc.target/x86_64/abi/test_passing_structs.c: Define __m128
>        tests only if CHECK_M64_M128 is defined.
>
>        * gcc.target/x86_64/abi/test_passing_structs.c (m128_struct): New.
>        (m128_2_struct): Likewise.
>        (check_struct_passing5): Likewise.
>        (check_struct_passing6): Likewise.
>        (main): Test struct with __m128 if CHECK_M64_M128 is defined.
>
>        * gcc.target/x86_64/abi/test_passing_unions.c (un4): New.
>        (un5): Likewise.
>        (check_union_passing4): Likewise.
>        (main): Test union with __m128 if CHECK_M64_M128 is defined.
>

Here is the updated patch. I added

	* g++.dg/compat/struct-layout-1_generate.c (dg_options): Add
	-Wno-union-with-long-double for x86.
	* gcc.dg/compat/struct-layout-1_generate.c (dg_options): Likewise.

Otherwise, the ABI change message will cause those generated
files fail to compile.  OK for trunk if all tests passes on Linux/x86-64?

Thanks.

-- 
H.J.

Attachment: gcc-pr39082-4.patch
Description: Text document