This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PATCH: PR target/45213: "suffix or operands invalid for `push'" triggered by optimisations on x86_64
On Sat, Aug 7, 2010 at 10:54 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Sat, 2010-08-07 at 08:52 -0700, H.J. Lu wrote:
>> On Sat, Aug 7, 2010 at 3:45 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> > On Sat, Aug 7, 2010 at 12:11 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> >
>> >>> "pushq $imm32S" only takes 32bit signed extended immediate. You can't push
>> >>> 0xbf800000. Instead, you push -1082130432 or 0xffffffffbf800000. ?This
>> >>> patch makes it signed. ?OK for trunk/4.5/4.4?
>> >>
>> >> No, see the comment in real.h:
>> >>
>> >> /* IN is a REAL_VALUE_TYPE. ?OUT is a long. ?*/
>> >> #define REAL_VALUE_TO_TARGET_SINGLE(IN, OUT) \
>> >> ?((OUT) = real_to_target (NULL, &(IN), mode_for_size (32, MODE_FLOAT, 0)))
>> >
>> > IMO, this is correct patch, to also generate correct extension on ILP32 hosts.
>> >
>> > Index: i386.c
>> > ===================================================================
>> > --- i386.c ? ? ?(revision 162975)
>> > +++ i386.c ? ? ?(working copy)
>> > @@ -12921,7 +12921,7 @@
>> >
>> > ? ? ? if (ASSEMBLER_DIALECT == ASM_ATT)
>> > ? ? ? ?putc ('$', file);
>> > - ? ? ?fprintf (file, "0x%08lx", (long unsigned int) l);
>> > + ? ? ?fprintf (file, "0x%08llx", (unsigned long long) (int) l);
>> > ? ? }
>> >
>> > ? /* These float cases don't actually occur as immediate operands. ?*/
>> >
>>
>> SF is 4 bytes, the same as SI. pushq takes 32bit signed extended immediate.
>> It is safe for pushq since the upper 4 bytes, which are all 1s, are unused.
>> For everything else, we use 32bit unsigned int. ?We don't want
>>
>> movl ?$0xffffffffbf800000, (%rsp)
>>
>> How does this patch look?
>
> Based on your patch, I think this is what we want:
>
> Index: i386.c
> ===================================================================
> --- i386.c ? ? ?(revision 162975)
> +++ i386.c ? ? ?(working copy)
> @@ -12921,7 +12921,11 @@
>
> ? ? ? if (ASSEMBLER_DIALECT == ASM_ATT)
> ? ? ? ?putc ('$', file);
> - ? ? ?fprintf (file, "0x%08lx", (long unsigned int) l);
> + ? ? ?/* For 64bit ABI sign extend 32bit immediate to 8 bytes. ?*/
> + ? ? ?if (code == 'q')
> + ? ? ? fprintf (file, "0x%08llx", (unsigned long long) (int) l);
> + ? ? ?else
> + ? ? ? fprintf (file, "0x%08x", (unsigned int) l);
> ? ? }
>
That has nothing to do with 64bit ABI. It is due to how pushq works
in hardware. We do wants to generate a 4byte immediate, not 8byte.
--
H.J.