Recent assembler sign extension patch

Recent assembler sign extension patch

[Matt Fleming]

Hey Stuart,

Your recent patch "sh: Remove implicit sign extension from assembler
immediates" (fea966f7564205fcf5919af9bde031e753419c96 in Paul's tree)
causes gas to error out for me. I'm using today's version of the
binutils assembler, GNU assembler version 2.19.51 (sh4-linux) using BFD
version (GNU Binutils) 2.19.51.20090827.

arch/sh/kernel/cpu/sh4/../sh3/entry.S:260: Error: offset out of range
arch/sh/kernel/cpu/sh4/../sh3/entry.S:260: Error: value of 4294967280 too large for field of 2 bytes at 160
arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:60: Error: offset out of range
arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:60: Error: value of 4294967280 too large for field of 2 bytes at 248
arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:172: Error: offset out of range
arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:172: Error: value of 4294967280 too large for field of 2 bytes at 516
arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:320: Error: offset out of range
arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:320: Error: value of 4294967280 too large for field of 2 bytes at 820
make[3]: *** [arch/sh/kernel/cpu/sh4/../sh3/entry.o] Error 1
make[2]: *** [arch/sh/kernel/cpu/sh4] Error 2
make[1]: *** [arch/sh/kernel/cpu] Error 2
make: *** [arch/sh/kernel] Error 2

I can't really understand the motivation for the change, specifically
the changelog for the patch states that the assembler was getting the
sign extension wrong in one case, but I can't see where. All the uses of
the "mov" and "and" instructions look sane to me. Could you explain the
issues you were seeing in a bit more detail, please?

I could be missing something blatantly obvious, however ;-)

Re: Recent assembler sign extension patch

[Paul Mundt]

On Thu, Aug 27, 2009 at 02:28:13PM +0100, Matt Fleming wrote:
> Hey Stuart,
> 
> Your recent patch "sh: Remove implicit sign extension from assembler
> immediates" (fea966f7564205fcf5919af9bde031e753419c96 in Paul's tree)
> causes gas to error out for me. I'm using today's version of the
> binutils assembler, GNU assembler version 2.19.51 (sh4-linux) using BFD
> version (GNU Binutils) 2.19.51.20090827.
> 
> arch/sh/kernel/cpu/sh4/../sh3/entry.S:260: Error: offset out of range
> arch/sh/kernel/cpu/sh4/../sh3/entry.S:260: Error: value of 4294967280 too large for field of 2 bytes at 160
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:60: Error: offset out of range
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:60: Error: value of 4294967280 too large for field of 2 bytes at 248
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:172: Error: offset out of range
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:172: Error: value of 4294967280 too large for field of 2 bytes at 516
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:320: Error: offset out of range
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:320: Error: value of 4294967280 too large for field of 2 bytes at 820
> make[3]: *** [arch/sh/kernel/cpu/sh4/../sh3/entry.o] Error 1
> make[2]: *** [arch/sh/kernel/cpu/sh4] Error 2
> make[1]: *** [arch/sh/kernel/cpu] Error 2
> make: *** [arch/sh/kernel] Error 2
> 
> I can't really understand the motivation for the change, specifically
> the changelog for the patch states that the assembler was getting the
> sign extension wrong in one case, but I can't see where. All the uses of
> the "mov" and "and" instructions look sane to me. Could you explain the
> issues you were seeing in a bit more detail, please?
> 
> I could be missing something blatantly obvious, however ;-)

Incidentally, this produces the proper results with the version of
binutils I am using, so it seems there are two separate issues, and that
the sign extension issue in question is likely already fixed by more
recent binutils versions.

It looks like we will have to version it and bury the sign extension in a
macro to deal with the different binutils versions. Your version of
binutils looks like it is doing the right thing, but I am concerned about
the other versions that require the explicit sign extension.

Re: Recent assembler sign extension patch

[Stuart MENEFY]

Hi Matt

Matt Fleming wrote:
> Hey Stuart,
> 
> Your recent patch "sh: Remove implicit sign extension from assembler
> immediates" (fea966f7564205fcf5919af9bde031e753419c96 in Paul's tree)
> causes gas to error out for me. I'm using today's version of the
> binutils assembler, GNU assembler version 2.19.51 (sh4-linux) using BFD
> version (GNU Binutils) 2.19.51.20090827.
> 
> arch/sh/kernel/cpu/sh4/../sh3/entry.S:260: Error: offset out of range
> arch/sh/kernel/cpu/sh4/../sh3/entry.S:260: Error: value of 4294967280 too large for field of 2 bytes at 160
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:60: Error: offset out of range
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:60: Error: value of 4294967280 too large for field of 2 bytes at 248
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:172: Error: offset out of range
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:172: Error: value of 4294967280 too large for field of 2 bytes at 516
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:320: Error: offset out of range
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:320: Error: value of 4294967280 too large for field of 2 bytes at 820
> make[3]: *** [arch/sh/kernel/cpu/sh4/../sh3/entry.o] Error 1
> make[2]: *** [arch/sh/kernel/cpu/sh4] Error 2
> make[1]: *** [arch/sh/kernel/cpu] Error 2
> make: *** [arch/sh/kernel] Error 2

Strange, this works OK for me although I'm using a slightly older
as (2.18.50.0.8.20090602). However I did see this error with gas
built on x86-64 - there is a bug in the assembler in this area
which assumes longs are 32 bit (from memory). Could that be why
you're seeing this?

> I can't really understand the motivation for the change, specifically
> the changelog for the patch states that the assembler was getting the
> sign extension wrong in one case, but I can't see where. All the uses of
> the "mov" and "and" instructions look sane to me. Could you explain the
> issues you were seeing in a bit more detail, please?

Its one of the things which has always bugged me about the SH assembler,
that its checks on the immediate argument don't take into account
whether the instruction sign extends its argument. To me more precise,
it checks all immediates are in the range -256 to +255.

So I modified binutils to take into account whether the instruction
sign extends or not, and found that entry.S had a bug:

@@ -676,8 +676,9 @@ need_resched:

        mov     #OFF_SR, r0
        mov.l   @(r0,r15), r0           ! get status register
-       and     #0xf0, r0               ! interrupts off (exception path)?
-       cmp/eq  #0xf0, r0
+       shlr    r0
+       and     #(0xf0>>1), r0
+       cmp/eq  #(0xf0>>1), r0          ! interrupts off (exception path)?
        bt      noresched
        mov.l   3f, r0
        jsr     @r0                     ! call preempt_schedule_irq

Here the "and" doesn't sign extend, but the "cmp/eq" does.

All the other changes are to clarify when sign extension is actually
taking place. Personally I'd rather see
  add #0xffffffe0, r0
than
  add #0xe0, r0
and have to remember that sign extension is taking place. This also happens
to be what my modified assembler requires!

Stuart

Re: Recent assembler sign extension patch

[Matt Fleming]

On Thu, Aug 27, 2009 at 03:39:36PM +0100, Stuart MENEFY wrote:
> 
> Strange, this works OK for me although I'm using a slightly older
> as (2.18.50.0.8.20090602). However I did see this error with gas
> built on x86-64 - there is a bug in the assembler in this area
> which assumes longs are 32 bit (from memory). Could that be why
> you're seeing this?
> 

Bingo! Good call there, Stuart. It is indeed a 64-bit host bug in the
assembler. The bug is still present in the latest binutils sources. I've
attached a patch that I will push towards the binutils folks.

> 
> Its one of the things which has always bugged me about the SH assembler,
> that its checks on the immediate argument don't take into account
> whether the instruction sign extends its argument. To me more precise,
> it checks all immediates are in the range -256 to +255.
> 
> So I modified binutils to take into account whether the instruction
> sign extends or not, and found that entry.S had a bug:
> 

Are you planning on submitting these assembler changes upstream? It'd be
cool if you can.

> @@ -676,8 +676,9 @@ need_resched:
> 
>         mov     #OFF_SR, r0
>         mov.l   @(r0,r15), r0           ! get status register
> -       and     #0xf0, r0               ! interrupts off (exception path)?
> -       cmp/eq  #0xf0, r0
> +       shlr    r0
> +       and     #(0xf0>>1), r0
> +       cmp/eq  #(0xf0>>1), r0          ! interrupts off (exception path)?
>         bt      noresched
>         mov.l   3f, r0
>         jsr     @r0                     ! call preempt_schedule_irq
> 
> Here the "and" doesn't sign extend, but the "cmp/eq" does.
> 
> All the other changes are to clarify when sign extension is actually
> taking place. Personally I'd rather see
>   add #0xffffffe0, r0
> than
>   add #0xe0, r0
> and have to remember that sign extension is taking place. This also happens
> to be what my modified assembler requires!
> 
> Stuart

Ah right, I understand now. That makes sense.


Index: gas/config/tc-sh.c
=================================RCS file: /cvs/src/src/gas/config/tc-sh.c,v
retrieving revision 1.132
diff -u -r1.132 tc-sh.c
--- gas/config/tc-sh.c	24 Jul 2009 11:45:00 -0000	1.132
+++ gas/config/tc-sh.c	27 Aug 2009 20:09:14 -0000
@@ -156,6 +156,11 @@
 #define ENCODE_RELAX(what,length) (((what) << 4) + (length))
 #define GET_WHAT(x) ((x>>4))
 
+/* Truncate and sign-extend at 32 bits, so that building on a 64-bit host
+   gives identical results to a 32-bit host.  */
+#define TRUNC(X)	((long) (X) & 0xffffffff)
+#define SEXT(X)		((TRUNC (X) ^ 0x80000000) - 0x80000000)
+
 /* These are the three types of relaxable instruction.  */
 /* These are the types of relaxable instructions; except for END which is
    a marker.  */
@@ -4183,7 +4188,7 @@
 	val = ((val >> shift)
 	       | ((long) -1 & ~ ((long) -1 >> shift)));
     }
-  if (max != 0 && (val < min || val > max))
+  if (max != 0 && (SEXT(val) < min || SEXT(val) > max))
     as_bad_where (fixP->fx_file, fixP->fx_line, _("offset out of range"));
   else if (max != 0)
     /* Stop the generic code from trying to overlow check the value as well.

Re: Recent assembler sign extension patch

[Nobuhiro Iwamatsu]

Hi,

2009/8/28 Matt Fleming <matt@console-pimps.org>:
> On Thu, Aug 27, 2009 at 03:39:36PM +0100, Stuart MENEFY wrote:
>>
>> Strange, this works OK for me although I'm using a slightly older
>> as (2.18.50.0.8.20090602). However I did see this error with gas
>> built on x86-64 - there is a bug in the assembler in this area
>> which assumes longs are 32 bit (from memory). Could that be why
>> you're seeing this?
>>
>
> Bingo! Good call there, Stuart. It is indeed a 64-bit host bug in the
> assembler. The bug is still present in the latest binutils sources. I've
> attached a patch that I will push towards the binutils folks.
>
>>
>> Its one of the things which has always bugged me about the SH assembler,
>> that its checks on the immediate argument don't take into account
>> whether the instruction sign extends its argument. To me more precise,
>> it checks all immediates are in the range -256 to +255.
>>
>> So I modified binutils to take into account whether the instruction
>> sign extends or not, and found that entry.S had a bug:
>>
>
> Are you planning on submitting these assembler changes upstream? It'd be
> cool if you can.
>
>> @@ -676,8 +676,9 @@ need_resched:
>>
>>         mov     #OFF_SR, r0
>>         mov.l   @(r0,r15), r0           ! get status register
>> -       and     #0xf0, r0               ! interrupts off (exception path)?
>> -       cmp/eq  #0xf0, r0
>> +       shlr    r0
>> +       and     #(0xf0>>1), r0
>> +       cmp/eq  #(0xf0>>1), r0          ! interrupts off (exception path)?
>>         bt      noresched
>>         mov.l   3f, r0
>>         jsr     @r0                     ! call preempt_schedule_irq
>>
>> Here the "and" doesn't sign extend, but the "cmp/eq" does.
>>
>> All the other changes are to clarify when sign extension is actually
>> taking place. Personally I'd rather see
>>   add #0xffffffe0, r0
>> than
>>   add #0xe0, r0
>> and have to remember that sign extension is taking place. This also happens
>> to be what my modified assembler requires!
>>
>> Stuart
>
> Ah right, I understand now. That makes sense.
>
>
> Index: gas/config/tc-sh.c
> =================================> RCS file: /cvs/src/src/gas/config/tc-sh.c,v
> retrieving revision 1.132
> diff -u -r1.132 tc-sh.c
> --- gas/config/tc-sh.c  24 Jul 2009 11:45:00 -0000      1.132
> +++ gas/config/tc-sh.c  27 Aug 2009 20:09:14 -0000
> @@ -156,6 +156,11 @@
>  #define ENCODE_RELAX(what,length) (((what) << 4) + (length))
>  #define GET_WHAT(x) ((x>>4))
>
> +/* Truncate and sign-extend at 32 bits, so that building on a 64-bit host
> +   gives identical results to a 32-bit host.  */
> +#define TRUNC(X)       ((long) (X) & 0xffffffff)
> +#define SEXT(X)                ((TRUNC (X) ^ 0x80000000) - 0x80000000)
> +
>  /* These are the three types of relaxable instruction.  */
>  /* These are the types of relaxable instructions; except for END which is
>    a marker.  */
> @@ -4183,7 +4188,7 @@
>        val = ((val >> shift)
>               | ((long) -1 & ~ ((long) -1 >> shift)));
>     }
> -  if (max != 0 && (val < min || val > max))
> +  if (max != 0 && (SEXT(val) < min || SEXT(val) > max))
>     as_bad_where (fixP->fx_file, fixP->fx_line, _("offset out of range"));
>   else if (max != 0)
>     /* Stop the generic code from trying to overlow check the value as well.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

I think that I should cope in a kernel, because this problem is thing
by the limit of mov.
# mov of this case supports only 8bit.

diff --git a/arch/sh/include/asm/entry-macros.S
b/arch/sh/include/asm/entry-macros.S
index cc43a55..64fd0de 100644
--- a/arch/sh/include/asm/entry-macros.S
+++ b/arch/sh/include/asm/entry-macros.S
@@ -7,7 +7,7 @@
    .endm

    .macro  sti
-   mov #0xfffffff0, r11
+   mov #0xf0, r11
    extu.b  r11, r11
    not r11, r11
    stc sr, r10
diff --git a/arch/sh/kernel/cpu/sh3/entry.S b/arch/sh/kernel/cpu/sh3/entry.S
index 0151933..4e3dc2d 100644
--- a/arch/sh/kernel/cpu/sh3/entry.S
+++ b/arch/sh/kernel/cpu/sh3/entry.S
@@ -260,7 +260,7 @@ restore_all:
    !
    ! Calculate new SR value
    mov k3, k2          ! original SR value
-   mov #0xfffffff0, k1
+   mov #0xf0, k1
    extu.b  k1, k1
    not k1, k1
    and k1, k2          ! Mask original SR value
diff --git a/arch/sh/lib/__clear_user.S b/arch/sh/lib/__clear_user.S
index db1dca7..31b692e 100644
--- a/arch/sh/lib/__clear_user.S
+++ b/arch/sh/lib/__clear_user.S
@@ -11,7 +11,7 @@
 ENTRY(__clear_user)
    !
    mov #0, r0
-   mov #0xffffffe0, r1
+   mov #0xe0, r1
    !
    ! r4..(r4+31)&~32      -------- not aligned [ Area 0 ]
    ! (r4+31)&~32..(r4+r5)&~32 -------- aligned [ Area 1 ]

Best regards,
  Nobuhiro
-- 
Nobuhiro Iwamatsu

Re: Recent assembler sign extension patch

[Paul Mundt]

On Mon, Sep 14, 2009 at 02:12:34PM +0900, Nobuhiro Iwamatsu wrote:
> 2009/8/28 Matt Fleming <matt@console-pimps.org>:
> > Index: gas/config/tc-sh.c
> > =================================> > RCS file: /cvs/src/src/gas/config/tc-sh.c,v
> > retrieving revision 1.132
> > diff -u -r1.132 tc-sh.c
> > --- gas/config/tc-sh.c ?24 Jul 2009 11:45:00 -0000 ? ? ?1.132
> > +++ gas/config/tc-sh.c ?27 Aug 2009 20:09:14 -0000
> > @@ -156,6 +156,11 @@
> > ?#define ENCODE_RELAX(what,length) (((what) << 4) + (length))
> > ?#define GET_WHAT(x) ((x>>4))
> >
> > +/* Truncate and sign-extend at 32 bits, so that building on a 64-bit host
> > + ? gives identical results to a 32-bit host. ?*/
> > +#define TRUNC(X) ? ? ? ((long) (X) & 0xffffffff)
> > +#define SEXT(X) ? ? ? ? ? ? ? ?((TRUNC (X) ^ 0x80000000) - 0x80000000)
> > +
> > ?/* These are the three types of relaxable instruction. ?*/
> > ?/* These are the types of relaxable instructions; except for END which is
> > ? ?a marker. ?*/
> > @@ -4183,7 +4188,7 @@
> > ? ? ? ?val = ((val >> shift)
> > ? ? ? ? ? ? ? | ((long) -1 & ~ ((long) -1 >> shift)));
> > ? ? }
> > - ?if (max != 0 && (val < min || val > max))
> > + ?if (max != 0 && (SEXT(val) < min || SEXT(val) > max))
> > ? ? as_bad_where (fixP->fx_file, fixP->fx_line, _("offset out of range"));
> > ? else if (max != 0)
> > ? ? /* Stop the generic code from trying to overlow check the value as well.
> 
> I think that I should cope in a kernel, because this problem is thing
> by the limit of mov.
> # mov of this case supports only 8bit.
> 
The whole reason that the toolchain patch is necessary is because 64-bit
hosts weren't doing sign extension on a truncated 32-bit value, which is
a legitimate bug.

As far as the mov size, a manually sign extended value is perfectly
acceptable, and any toolchain should be intelligent enough to get the
size calculation right. It's obviously not entirely elegant, but it's
much less confusing than running in to situations where the same
immediate is used and it may or may not be sign extended, depending on
which instruction it gets used with. That ambiguity is what this approach
solves, and I think it is definitely worthwhile trying to keep it as
simple as possible to prevent sign extension bugs from popping up.

If you've ever had to chase a sign extension bug, you will appreciate the
value of this (especially given the sometimes rather inconsistent
instruction set).

Re: Recent assembler sign extension patch

[Rob Landley]

On Monday 25 January 2010 21:22:08 you wrote:
> On Sat, Jan 23, 2010 at 06:16:06AM -0600, Rob Landley wrote:
> > I get this error:
> >
> > arch/sh/kernel/cpu/sh4/../sh3/entry.S: Assembler messages:
> > arch/sh/kernel/cpu/sh4/../sh3/entry.S:260: Error: value of
> > 00000000fffffff0 too large for field of 2 bytes at 00000000000000a0
> > arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:60: Error: value of
> > 00000000fffffff0 too large for field of 2 bytes at 00000000000000c8
> > arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:172: Error: value of
> > 00000000fffffff0 too large for field of 2 bytes at 000000000000013e
> > arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:320: Error: value of
> > 00000000fffffff0 too large for field of 2 bytes at 00000000000001da
> > make[3]: *** [arch/sh/kernel/cpu/sh4/../sh3/entry.o] Error 1
>
> See the http://www.spinics.net/lists/linux-sh/msg03499.html thread.

I'm sorry, what was the resolution?  Changing the compiler means the kernel no 
longer compiles with any existing gcc release from here forward.  Also, I 
applied the patch from http://www.spinics.net/lists/linux-sh/msg03505.html to 
binutils 2.17 (the last gplv2 release), which changed the error to:

  AS      arch/sh/kernel/cpu/sh4/../sh3/entry.o
arch/sh/kernel/cpu/sh4/../sh3/entry.S: Assembler messages:
arch/sh/kernel/cpu/sh4/../sh3/entry.S:261: Error: value of 4294967280 too 
large for field of 2 bytes at 168
arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:60: Error: value of 
4294967280 too large for field of 2 bytes at 208
arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:172: Error: value of 
4294967280 too large for field of 2 bytes at 326
arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:320: Error: value of 
4294967280 too large for field of 2 bytes at 482

Which is complaining in write.c (generic code).  (The patch doesn't seem to 
adjust val, it just changes the error test...)

Rob
-- 
Latency is more important than throughput. It's that simple. - Linus Torvalds

Re: Recent assembler sign extension patch

[Paul Mundt]

On Wed, Jan 27, 2010 at 01:29:36AM -0600, Rob Landley wrote:
> On Monday 25 January 2010 21:22:08 you wrote:
> > On Sat, Jan 23, 2010 at 06:16:06AM -0600, Rob Landley wrote:
> > > I get this error:
> > >
> > > arch/sh/kernel/cpu/sh4/../sh3/entry.S: Assembler messages:
> > > arch/sh/kernel/cpu/sh4/../sh3/entry.S:260: Error: value of
> > > 00000000fffffff0 too large for field of 2 bytes at 00000000000000a0
> > > arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:60: Error: value of
> > > 00000000fffffff0 too large for field of 2 bytes at 00000000000000c8
> > > arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:172: Error: value of
> > > 00000000fffffff0 too large for field of 2 bytes at 000000000000013e
> > > arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:320: Error: value of
> > > 00000000fffffff0 too large for field of 2 bytes at 00000000000001da
> > > make[3]: *** [arch/sh/kernel/cpu/sh4/../sh3/entry.o] Error 1
> >
> > See the http://www.spinics.net/lists/linux-sh/msg03499.html thread.
> 
> I'm sorry, what was the resolution?  Changing the compiler means the kernel no 
> longer compiles with any existing gcc release from here forward.  Also, I 
> applied the patch from http://www.spinics.net/lists/linux-sh/msg03505.html to 
> binutils 2.17 (the last gplv2 release), which changed the error to:
> 
At what point in this thread was the compiler even mentioned? The entire
topic of the thread relates only to gas, kindly refrain from making up
imaginary problems, thanks.

>   AS      arch/sh/kernel/cpu/sh4/../sh3/entry.o
> arch/sh/kernel/cpu/sh4/../sh3/entry.S: Assembler messages:
> arch/sh/kernel/cpu/sh4/../sh3/entry.S:261: Error: value of 4294967280 too 
> large for field of 2 bytes at 168
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:60: Error: value of 
> 4294967280 too large for field of 2 bytes at 208
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:172: Error: value of 
> 4294967280 too large for field of 2 bytes at 326
> arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:320: Error: value of 
> 4294967280 too large for field of 2 bytes at 482
> 
> Which is complaining in write.c (generic code).  (The patch doesn't seem to 
> adjust val, it just changes the error test...)
> 
It's a bug in gas on 64-bit hosts, which has subsequently been fixed
upstream:

	http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/config/tc-sh.c.diff?r1=1.132&r2=1.133&cvsroot=src

If you wish to use an outdated and buggy version of binutils for personal
reasons, you can of course backport away at your leisure, but in that
case there's no point in submitting bug reports unless they are still
relevant to current versions.

Note that this particular bug was quickly identified by Matt and Stuart
while being subsequently fixed upstream by Kojima-san all within a 2 day
window.

If the bug persists for you with current versions, then we need to take
aother look at the binutils changes and see if there is anything
outstanding.

Re: Recent assembler sign extension patch

[Rob Landley]

On Wednesday 27 January 2010 01:55:42 Paul Mundt wrote:
> On Wed, Jan 27, 2010 at 01:29:36AM -0600, Rob Landley wrote:
> > On Monday 25 January 2010 21:22:08 you wrote:
> > > On Sat, Jan 23, 2010 at 06:16:06AM -0600, Rob Landley wrote:
> > > > I get this error:
> > > >
> > > > arch/sh/kernel/cpu/sh4/../sh3/entry.S: Assembler messages:
> > > > arch/sh/kernel/cpu/sh4/../sh3/entry.S:260: Error: value of
> > > > 00000000fffffff0 too large for field of 2 bytes at 00000000000000a0
> > > > arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:60: Error: value
> > > > of 00000000fffffff0 too large for field of 2 bytes at
> > > > 00000000000000c8
> > > > arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:172: Error: value
> > > > of 00000000fffffff0 too large for field of 2 bytes at
> > > > 000000000000013e
> > > > arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:320: Error: value
> > > > of 00000000fffffff0 too large for field of 2 bytes at
> > > > 00000000000001da make[3]: *** [arch/sh/kernel/cpu/sh4/../sh3/entry.o]
> > > > Error 1
> > >
> > > See the http://www.spinics.net/lists/linux-sh/msg03499.html thread.
> >
> > I'm sorry, what was the resolution?  Changing the compiler means the
> > kernel no longer compiles with any existing gcc release from here
> > forward.  Also, I applied the patch from
> > http://www.spinics.net/lists/linux-sh/msg03505.html to binutils 2.17 (the
> > last gplv2 release), which changed the error to:
>
> At what point in this thread was the compiler even mentioned? The entire
> topic of the thread relates only to gas, kindly refrain from making up
> imaginary problems, thanks.

You're right, the compiler never seems to emit code that triggers this bug, 
you have to hand-write assembly to do it, and intentionally ask the assembler 
to do something it can't do.

> >   AS      arch/sh/kernel/cpu/sh4/../sh3/entry.o
> > arch/sh/kernel/cpu/sh4/../sh3/entry.S: Assembler messages:
> > arch/sh/kernel/cpu/sh4/../sh3/entry.S:261: Error: value of 4294967280 too
> > large for field of 2 bytes at 168
> > arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:60: Error: value of
> > 4294967280 too large for field of 2 bytes at 208
> > arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:172: Error: value of
> > 4294967280 too large for field of 2 bytes at 326
> > arch/sh/kernel/cpu/sh4/../sh3/../../entry-common.S:320: Error: value of
> > 4294967280 too large for field of 2 bytes at 482
> >
> > Which is complaining in write.c (generic code).  (The patch doesn't seem
> > to adjust val, it just changes the error test...)
>
> It's a bug in gas on 64-bit hosts, which has subsequently been fixed
> upstream:
>
> 	http://sourceware.org/cgi-bin/cvsweb.cgi/src/gas/config/tc-sh.c.diff?r1=1.
>132&r2=1.133&cvsroot=src

The date on that commit is August 29.

So the official position of the maintainer of the Super Hitachi port is that any 
toolchain older than October 16th (the binutils 2.20 release) is too old to 
build the Linux kernel.  (With the corollary that in the 5 years since the PC 
went 64 bit, apparently nobody ever actually tried to build anything for Super 
Hitachi?)

Meanwhile Documentation/Changes still says binutils 2.12, and other 
architectures _never_ work around bugs in older toolchains (*cough* git 
2a4b9c5af8203 *cough*)...

*shrug*  I'm going to go locally revert the "cleanup" patch that asked the 
assembler to do something it can't do.  That fixes it for me.

Rob
-- 
Latency is more important than throughput. It's that simple. - Linus Torvalds

Re: Recent assembler sign extension patch

[Paul Mundt]

On Fri, Jan 29, 2010 at 01:53:59AM -0600, Rob Landley wrote:
> On Wednesday 27 January 2010 01:55:42 Paul Mundt wrote:
> > At what point in this thread was the compiler even mentioned? The entire
> > topic of the thread relates only to gas, kindly refrain from making up
> > imaginary problems, thanks.
> 
> You're right, the compiler never seems to emit code that triggers this bug, 
> you have to hand-write assembly to do it, and intentionally ask the assembler 
> to do something it can't do.
> 
The only reason the assembler choked on it was because the assembler
was buggy, is that so hard for you to get your head around? And
unsurprisingly, other bugs in the kernel were caught and fixed as a
result of this change.

> So the official position of the maintainer of the Super Hitachi port is that any 
> toolchain older than October 16th (the binutils 2.20 release) is too old to 
> build the Linux kernel.  (With the corollary that in the 5 years since the PC 
> went 64 bit, apparently nobody ever actually tried to build anything for Super 
> Hitachi?)
> 
For starters, the only Super Hitachi in existence is a train, which has
nothing to do with me or with my employer.

Anything older than the 2.20 binutils release will have this particular
bug with sign extension on 64-bit hosts. The fact we only recently
introduced a change that triggered this bug in the assembler doesn't
change the fact that it was always there, and remains a legitimate
usecase. Having said that, we could work around it, yes, but so far there
hasn't been sufficient reason to do so. 

If you'd like to cite a technical reason for why you are unable to move
your version of binutils forward, then we can look in to that. If you're
primarily intent on whining and posturing, don't bother Ccing me.

> Meanwhile Documentation/Changes still says binutils 2.12, and other 
> architectures _never_ work around bugs in older toolchains (*cough* git 
> 2a4b9c5af8203 *cough*)...
> 
Documentation/Changes also only states "minimal" requirements, which are
obviously not going to be consistent across platforms. If you think it
means anything other than that, you are sorely misinformed. Most
platforms can't build the kernel with the toolchain requirements outlined
therein today, something that is abundantly obvious to anyone that spends
even a cursory amount of time actually working on the code.