| Message ID | 1281203653.883.2.camel@localhost |
|---|---|
| State | New |
| Headers | show |
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.
On Sat, Aug 7, 2010 at 8:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> 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. OK will say "Sign extend 32bit signed immediate to 8 bytes. 0xff.ff.ff.ff.80.00.00.00 So, 8 bytes. Uros.
On Sat, Aug 7, 2010 at 12:04 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Sat, Aug 7, 2010 at 8:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > >>>> 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. > > OK will say "Sign extend 32bit signed immediate to 8 bytes. > > 0xff.ff.ff.ff.80.00.00.00 > > So, 8 bytes. Only pushq sign extends to 8 bytes and we don't care about the upper 4 bytes here. Everything else is 4 byte.
On Sat, 2010-08-07 at 14:09 -0700, H.J. Lu wrote: > >>>> 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. > > > > OK will say "Sign extend 32bit signed immediate to 8 bytes. > > > > 0xff.ff.ff.ff.80.00.00.00 > > > > So, 8 bytes. > > Only pushq sign extends to 8 bytes and we don't care about > the upper 4 bytes here. Everything else is 4 byte. You have to feed pushq with 8 bytes, and we do care for upper 4 bytes since they should all be 0xff to correctly sign-extend the 32bit immediate. pushq mnemonic _REQUIRES_ 8 bytes, the top 4 must be explicitly set in the asm source. Just try to assemble "pushq 0xfeffffff80000000". Uros.
On Sat, Aug 7, 2010 at 2:23 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Sat, 2010-08-07 at 14:09 -0700, H.J. Lu wrote: > >> >>>> 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. >> > >> > OK will say "Sign extend 32bit signed immediate to 8 bytes. >> > >> > 0xff.ff.ff.ff.80.00.00.00 >> > >> > So, 8 bytes. >> >> Only pushq sign extends to 8 bytes and we don't care about >> the upper 4 bytes here. Everything else is 4 byte. > > You have to feed pushq with 8 bytes, and we do care for upper 4 bytes > since they should all be 0xff to correctly sign-extend the 32bit > immediate. > > pushq mnemonic _REQUIRES_ 8 bytes, the top 4 must be explicitly set in > the asm source. Just try to assemble "pushq 0xfeffffff80000000". > This is probably too much details. pushq puts 8byte, which is sign extended from 32bit immediate operand, on stack. pushq takes INT_MIN to INT_MAX. You can express them in either 8byte hex or 4byte signed decimal. They are equivalent as long as 8byte hex is within INT_MIN and INT_MAX. Since SF is 4byte and the size of each argument gets rounded up to eightbytes, it is OK to use pushq to put SF on stack when passing function parameter. The upper 4bytes of "pushq" target are unused by callee.
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); } /* These float cases don't actually occur as immediate operands. */