[PATCH] fix wrong code for push of long double

Uros Bizjak ubizjak@gmail.com
Tue Aug 4 17:48:00 GMT 2009


On 08/04/2009 07:47 PM, Uros Bizjak wrote:
> Hello!
>
>> This patch fixes incorrect code being generated for push long double.
>>
>> Tested on i686. I compiled and ran succesfully mpfr testsuite where the
>> bug originally showed up.
>>
>> The patch should be applied to 4.4 branch and likely to trunk as well (I
>> checked it only on 4.4.1 release).
>>
>> 2009-08-04  Mikulas Patocka<mikulas@artax.karlin.mff.cuni.cz>
>>
>>     PR target/40906
>>     * config/i386/i386.c (ix86_split_long_move): Fix push of long
>>     double
>>     * testsuite/gcc.dg/pr40906-m128bit-long-double.c: New test
>>     * testsuite/gcc.dg/pr40906.c: New test
>>
>> diff -urpN gcc-4.4.1/gcc/config/i386/i386.c 
>> gcc-4.4.1-fix-push/gcc/config/i386/i386.c
>> --- gcc-4.4.1/gcc/config/i386/i386.c    2009-07-21 09:22:51.000000000 
>> +0200
>> +++ gcc-4.4.1-fix-push/gcc/config/i386/i386.c    2009-07-29 
>> 03:10:26.000000000 +0200
>> @@ -16307,10 +16307,18 @@ ix86_split_long_move (rtx operands[])
>>     /* When emitting push, take care for source operands on the 
>> stack.  */
>>     if (push&&  MEM_P (operands[1])
>> &&  reg_overlap_mentioned_p (stack_pointer_rtx, operands[1]))
>> -    for (i = 0; i<  nparts - 1; i++)
>> -      part[1][i] = change_address (part[1][i],
>> -                   GET_MODE (part[1][i]),
>> -                   XEXP (part[1][i + 1], 0));
>> +    {
>> +      if (!TARGET_64BIT&&  TARGET_128BIT_LONG_DOUBLE&&  mode == XFmode)
>> +    {
>> +      /* Compensate for the stack decrement by 4 */
>> +      for (i = 0; i<  nparts; i++)
>> +        part[1][i] = change_address (part[1][i], GET_MODE 
>> (part[1][i]), plus_constant(XEXP (part[1][i], 0), 4));
>> +    }
>> +      for (i = nparts - 2; i>= 0; i--)
>> +    part[1][i] = change_address (part[1][i],
>> +                     GET_MODE (part[1][i]),
>> +                     XEXP (part[1][i + 1], 0));
>
> This part is in fact copying the offset from the last element to all 
> elements that are part of the source operand. As evident from the 
> generated source, we are dealing with a strange situation, where 
> destination _AND_ source of the push instruction are automatically 
> updated:
>
> g:
>     subl    $16, %esp
>     pushl    28(%esp)
>     pushl    28(%esp)
>     pushl    28(%esp)
>     call    f
>     xorl    %eax, %eax
>     addl    $28, %esp
>     ret
>
> Actually, by copying the %esp-relative address of the topmost element 
> to all elements, we are doing the right thing. %esp will be decreased 
> by the push, so all data will be copied from one place to another.
>
>> +    }
>>
>>     /* We need to do copy in the right order in case an address register
>>        of the source overlaps the destination.  */
>> diff -urpN 
>> gcc-4.4.1/gcc/testsuite/gcc.dg/pr40906-m128bit-long-double.c 
>> gcc-4.4.1-fix-push/gcc/testsuite/gcc.dg/pr40906-m128bit-long-double.c
>> --- gcc-4.4.1/gcc/testsuite/gcc.dg/pr40906-m128bit-long-double.c    
>> 1970-01-01 01:00:00.000000000 +0100
>> +++ 
>> gcc-4.4.1-fix-push/gcc/testsuite/gcc.dg/pr40906-m128bit-long-double.c    
>> 2009-08-04 14:19:09.000000000 +0200
>
> This test belongs to gcc.target/i386. Add "dg-require-effective-target 
> ilp32", since it is valid only on x86_32.
>
>> @@ -0,0 +1,24 @@
>> +/* PR target/40906 */
>> +/* Test for push of long double */
>> +
>> +/* { dg-do run { target { { i?86-*-* x86_64-*-* }&&  ilp32 } } } */
>> +/* { dg-options "-O2 -fomit-frame-pointer -fno-inline -mpush-args 
>> -mno-accumulate-outgoing-args -m128bit-long-double" } */
>> +
>> +extern void abort (void);
>> +
>> +void f(long double a)
>> +{
>> +    if (a != 1.0) abort();
>> +}
>> +
>> +int g(long double b)
>> +{
>> +    f(b);
>> +    return 0;
>> +}
>> +
>> +int main(void)
>> +{
>> +    g(1.0);
>> +    return 0;
>> +}
>> diff -urpN gcc-4.4.1/gcc/testsuite/gcc.dg/pr40906.c 
>> gcc-4.4.1-fix-push/gcc/testsuite/gcc.dg/pr40906.c
>> --- gcc-4.4.1/gcc/testsuite/gcc.dg/pr40906.c    1970-01-01 
>> 01:00:00.000000000 +0100
>> +++ gcc-4.4.1-fix-push/gcc/testsuite/gcc.dg/pr40906.c    2009-08-04 
>> 15:06:42.000000000 +0200
>> @@ -0,0 +1,25 @@
>> +/* PR target/40906 */
>> +/* Test for push of long double */
>> +
>> +/* { dg-do run } */
>> +/* { dg-options "-O2 -fomit-frame-pointer -fno-inline" } */
>> +/* { dg-options "-O2 -fomit-frame-pointer -fno-inline -mpush-args 
>> -mno-accumulate-outgoing-args" { target { i?86-*-* x86_64-*-* } } } */
>
> This test is not valid on x86_64, so it should be limited to ilp32 
> targets. Usually, __attribute__((noinline)) is used instead of 
> -fno-inline.
>
>> +
>> +extern void abort (void);
>> +
>> +void f(long double a)
>> +{
>> +    if (a != 1.0) abort();
>> +}
>> +
>> +int g(long double b)
>> +{
>> +    f(b);
>> +    return 0;
>> +}
>> +
>> +int main(void)
>> +{
>> +    g(1.0);
>> +    return 0;
>> +}
>>
>
> Can you please try if attached patch fixes your problem? (I'm 
> currently testing this patch on x86_64-pc-linux-gnu {,-m32}).
>
> Thanks,
> Uros.
>

... now with the patch ...

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: p.diff.txt
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20090804/17b0fe41/attachment.txt>


More information about the Gcc-patches mailing list