more m68k breakage on m68k-linux

more m68k breakage on m68k-linux

[Bernardo Innocenti]

Hello,

I'm now testing on m68k-linux as well as m68k-netbsdelf and m68k-uclinux.
I see a new ICE during bootstrap, even with Andreas patch installed.

It happens *only* on m68k-linux, but the checkous have been done a few
days from each other.


./xgcc -B./ -B/warez/gcc/m68k-linux-HEAD-install/m68k-unknown-linux-gnu/bin/ -isystem /warez/gcc/m68k-linux-HEAD-install/m68k-unknown-linux-gnu/include -isystem /warez/gcc/m68k-linux-HEAD-install/m68k-unknown-linux-gnu/sys-include -L/home/bernie/src/m68k-linux-HEAD-build/gcc/../ld -O2  -DIN_GCC    -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition  -isystem ./include  -fPIC -g  -DIN_LIBGCC2 -D__GCC_FLOAT_NOT_NEEDED  -I. -I. -I../../combined-HEAD/gcc -I../../combined-HEAD/gcc/. -I../../combined-HEAD/gcc/../include   -DL_cmpdi2 -c ../../combined-HEAD/gcc/libgcc2.c -o libgcc/./_cmpdi2.o
../../combined-HEAD/gcc/libgcc2.c: In function `__cmpdi2':
../../combined-HEAD/gcc/libgcc2.c:1081: error: unrecognizable insn:
(insn 34 27 28 2 ../../combined-HEAD/gcc/libgcc2.c:1075 (set (strict_low_part (reg:QI 2 %d2))
       (const_int 2 [0x2])) -1 (nil)
   (expr_list:REG_EQUAL (const_int 2 [0x2])
       (nil)))
../../combined-HEAD/gcc/libgcc2.c:1081: internal compiler error: in extract_insn, at recog.c:2083

-- 
 // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: more m68k breakage on m68k-linux

[Bernardo Innocenti]

Bernardo Innocenti wrote:
> Hello,
> 
> I'm now testing on m68k-linux as well as m68k-netbsdelf and m68k-uclinux.
> I see a new ICE during bootstrap, even with Andreas patch installed.
> 
> It happens *only* on m68k-linux, but the checkous have been done a few
> days from each other.
> 
> ./xgcc -B./ 
> -B/warez/gcc/m68k-linux-HEAD-install/m68k-unknown-linux-gnu/bin/ 
> -isystem 
> /warez/gcc/m68k-linux-HEAD-install/m68k-unknown-linux-gnu/include 
> -isystem 
> /warez/gcc/m68k-linux-HEAD-install/m68k-unknown-linux-gnu/sys-include 
> -L/home/bernie/src/m68k-linux-HEAD-build/gcc/../ld -O2  -DIN_GCC    -W 
> -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes 
> -Wold-style-definition  -isystem ./include  -fPIC -g  -DIN_LIBGCC2 
> -D__GCC_FLOAT_NOT_NEEDED  -I. -I. -I../../combined-HEAD/gcc 
> -I../../combined-HEAD/gcc/. -I../../combined-HEAD/gcc/../include   
> -DL_cmpdi2 -c ../../combined-HEAD/gcc/libgcc2.c -o libgcc/./_cmpdi2.o
> ../../combined-HEAD/gcc/libgcc2.c: In function `__cmpdi2':
> ../../combined-HEAD/gcc/libgcc2.c:1081: error: unrecognizable insn:
> (insn 34 27 28 2 ../../combined-HEAD/gcc/libgcc2.c:1075 (set 
> (strict_low_part (reg:QI 2 %d2))
>       (const_int 2 [0x2])) -1 (nil)
>   (expr_list:REG_EQUAL (const_int 2 [0x2])
>       (nil)))
> ../../combined-HEAD/gcc/libgcc2.c:1081: internal compiler error: in 
> extract_insn, at recog.c:2083

Correction: this test system must have thermal problems or some memory
issue.  I keep getting spurious, unreproducible failures all over
libgcc... only when make has been running for a while :-)

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: more m68k breakage on m68k-linux

[Matthias Klose]

Bernardo Innocenti writes:
> Hello,
> 
> I'm now testing on m68k-linux as well as m68k-netbsdelf and m68k-uclinux.
> I see a new ICE during bootstrap, even with Andreas patch installed.
> 
> It happens *only* on m68k-linux, but the checkous have been done a few
> days from each other.

unable to reproduce this. Just finished a testsuite run for
m68k-linux, ignoring the bootstrap comparision failure, described in
http://gcc.gnu.org/ml/gcc/2004-01/msg00929.html

See http://gcc.gnu.org/ml/gcc-testresults/2004-01/msg00720.html for
the test results.

	Matthias

Re: more m68k breakage on m68k-linux

[Bernardo Innocenti]

Matthias Klose wrote:

>>I'm now testing on m68k-linux as well as m68k-netbsdelf and m68k-uclinux.
>>I see a new ICE during bootstrap, even with Andreas patch installed.
>>
>>It happens *only* on m68k-linux, but the checkous have been done a few
>>days from each other.
> 
> unable to reproduce this.

Sorry, it was a bogus report.  The machine I was testing with
most probably had some overheating problems.

> Just finished a testsuite run for
> m68k-linux, ignoring the bootstrap comparision failure, described in
> http://gcc.gnu.org/ml/gcc/2004-01/msg00929.html
> 
> See http://gcc.gnu.org/ml/gcc-testresults/2004-01/msg00720.html for
> the test results.

Hmmm... looks we're in a pretty good shape, except for some EH problems.

I've just finished another testsuite on m68k-netbsd, this time using
the GNU as and ld.  Failures dropped from 170 to about 100.  It's very
sad, altough most tests seem to be failing because of OS specific
issues.

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: more m68k breakage on m68k-linux

[Andreas Schwab]

Matthias Klose <doko@cs.tu-berlin.de> writes:

> See http://gcc.gnu.org/ml/gcc-testresults/2004-01/msg00720.html for
> the test results.

Could you please try to analyze the failures?  That is probably
easier than tracking down the comparison failures.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: more m68k breakage on m68k-linux

[Bernardo Innocenti]

Andreas Schwab wrote:
> Matthias Klose <doko@cs.tu-berlin.de> writes:
> 
> 
>>See http://gcc.gnu.org/ml/gcc-testresults/2004-01/msg00720.html for
>>the test results.
> 
> 
> Could you please try to analyze the failures?  That is probably
> easier than tracking down the comparison failures.

I think I can take the EH related ones.  My guess is that we have
an off-by-one bug in dw2 unwinding information generated by
m68k_output_function_prologue().  I'm also aware that offsets
should be computed backwards for ColdFire frames when using movem
with no pre-decrement.

My m68k-linux test box is currently being upgraded by the Debian
people.  I'll have to wait until they're done before I can start
analyzying the failures.

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: more m68k breakage on m68k-linux

[Richard Zidlicky]

Hi,

Appears we have an off by 1 error in m68k.h. Any idea how the old
m68k CPU magically gained another hard register?

gcc-3.2: #define FIRST_PSEUDO_REGISTER 24
gcc-3.4: #define FIRST_PSEUDO_REGISTER 25

Richard

Re: more m68k breakage on m68k-linux

[Richard Zidlicky]

On Mon, Mar 01, 2004 at 09:05:53PM +0100, Richard Zidlicky wrote:
> Hi,
> 
> Appears we have an off by 1 error in m68k.h. Any idea how the old
> m68k CPU magically gained another hard register?
> 
> gcc-3.2: #define FIRST_PSEUDO_REGISTER 24
> gcc-3.4: #define FIRST_PSEUDO_REGISTER 25

Apparently it is this change:

<<
2003-09-08  Bernardo Innocenti  <bernie@develer.com>
            Peter Barada <peter@baradas.org>

        * config/m68k/coff.h (REGISTER_NAMES): Add fake register `argptr'
        * config/m68k/hp320.h (REGISTER_NAMES): Likewise.
        * config/m68k/linux.h (REGISTER_NAMES): Likewise.
        * config/m68k/m68kelf.h (REGISTER_NAMES): Likewise.
        * gcc/config/m68k/sgs.h (REGISTER_NAMES): Likewise.
        * config/m68k/m68k-protos.h (m68k_initial_elimination_offset): Add prot
otype.
        * config/m68k/m68k.c (m68k_frame): New struct, simular to ix86 back-end
.
        (m68k_compute_frame_layout): New function.
        (m68k_initial_elimination_offset): New function.
        (m68k_output_function_prologue): ColdFire-specific movem handling.
        (m68k_output_function_epilogue): Likewise.
        * config/m68k/m68k.h (FIRST_PSEOUDO_REGISTER): Make room for argptr reg
.
        (ARG_POINTER_REGNUM): Add new definition.
        (INITIAL_FRAME_POINTER_OFFSET): Remove macro.
        (ELIMINABLE_REGS): Define new macro, like in ix86 back-end.
        (CAN_ELIMINATE): Likewise.
        (INITIAL_ELIMINATION_OFFSET): Likewise.  
>>

Note the typo "FIRST_PSEOUDO_REGISTER".. made it somewhat hard to
find the change.

This looks highly bogus to me: You create a new "hard" register but
dont init it anywhere (FIXED_REGISTERS,...). You change ARG_POINTER_REGNUM
from 14 to 24 claiming in the comment that it is not a hard register..
well it is still < FIRST_PSEOUDO_REGISTER so it is a hard register.

Last not least there is 

#define ELIMINABLE_REGS					\
{{ ARG_POINTER_REGNUM, STACK_POINTER_REGNUM },		\
 { ARG_POINTER_REGNUM, FRAME_POINTER_REGNUM },	\
 { FRAME_POINTER_REGNUM, STACK_POINTER_REGNUM }}

and it is not clear to me how this should work if ARG_POINTER_REGNUM
were a pseudo register.

Richard

Re: more m68k breakage on m68k-linux

[Andreas Schwab]

Richard Zidlicky <rz@linux-m68k.org> writes:

> Hi,
>
> Appears we have an off by 1 error in m68k.h. Any idea how the old
> m68k CPU magically gained another hard register?

It's called argptr.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: more m68k breakage on m68k-linux

[Bernardo Innocenti]

Richard Zidlicky wrote:
> Hi,
> 
> Appears we have an off by 1 error in m68k.h. Any idea how the old
> m68k CPU magically gained another hard register?
> 
> gcc-3.2: #define FIRST_PSEUDO_REGISTER 24
> gcc-3.4: #define FIRST_PSEUDO_REGISTER 25
> 

Yeah: register 25 is a pseudo register that is only being
used in RTL. It stands for the "argument pointer" register,
pointing to a location on the stack from which arguments
can be retrieved using a constant offset.

Some CPUs really have such a register.  On the m68k, it
is of course always eliminated by replacing it with the
frame pointer or the stack pointer.

Previously, the argptr was defined to be the same number
of FP, but this made it impossible to do the right thing
in register elimination for the ColdFire.

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: more m68k breakage on m68k-linux

[Bernardo Innocenti]

Richard Zidlicky wrote:

> Note the typo "FIRST_PSEOUDO_REGISTER".. made it somewhat hard to
> find the change.

Oops!  That's very unfortunate.  I've fixed it in the ChangeLog.


> This looks highly bogus to me: You create a new "hard" register but
> dont init it anywhere (FIXED_REGISTERS,...). You change ARG_POINTER_REGNUM
> from 14 to 24 claiming in the comment that it is not a hard register..
> well it is still < FIRST_PSEOUDO_REGISTER so it is a hard register.

All registers >= FIRST_PSEUDO_REGISTER are free for use in
RTL expressions as temporaries before the register allocator
pass moves them to a hard register.

If I didn't increment FIRST_PSEUDO_REGISTER, register 25 would
be used both as the argptr and as a placeholder for some
temporary register, with very interesting consequences :-)

> Last not least there is 
> 
> #define ELIMINABLE_REGS					\
> {{ ARG_POINTER_REGNUM, STACK_POINTER_REGNUM },		\
>  { ARG_POINTER_REGNUM, FRAME_POINTER_REGNUM },	\
>  { FRAME_POINTER_REGNUM, STACK_POINTER_REGNUM }}
> 
> and it is not clear to me how this should work if ARG_POINTER_REGNUM
> were a pseudo register.

This basically says that the arg pointer can be eliminated
against either SP or FP.  Additionally, FP can be
eliminated with SP.

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: more m68k breakage on m68k-linux

[Richard Zidlicky]

On Tue, Mar 02, 2004 at 01:18:51AM +0100, Bernardo Innocenti wrote:
> Richard Zidlicky wrote:
> 
> >Note the typo "FIRST_PSEOUDO_REGISTER".. made it somewhat hard to
> >find the change.
> 
> Oops!  That's very unfortunate.  I've fixed it in the ChangeLog.

thanks.. I should have used agrep or something :)

> >This looks highly bogus to me: You create a new "hard" register but
> >dont init it anywhere (FIXED_REGISTERS,...). You change ARG_POINTER_REGNUM
> >from 14 to 24 claiming in the comment that it is not a hard register..
> >well it is still < FIRST_PSEOUDO_REGISTER so it is a hard register.
> 
> All registers >= FIRST_PSEUDO_REGISTER are free for use in
> RTL expressions as temporaries before the register allocator
> pass moves them to a hard register.
> 
> If I didn't increment FIRST_PSEUDO_REGISTER, register 25 would
> be used both as the argptr and as a placeholder for some
> temporary register, with very interesting consequences :-)

hm.. the problem I see is that the register allocator is initialised
with random garbage. Eg FIXED_REGISTERS is 24 elts big, thus 
initial_fixed_regs[] is also only 24 elts and in init_reg_sets() we
copy a random 25th elt into fixed_regs[] and risk segfault.
I am pretty sure more it results in more damage later on.

Richard

Re: more m68k breakage on m68k-linux

[Gunther Nikl]

On Tue, Mar 02, 2004 at 09:53:05AM +0100, Richard Zidlicky wrote:
> On Tue, Mar 02, 2004 at 01:18:51AM +0100, Bernardo Innocenti wrote:
> > If I didn't increment FIRST_PSEUDO_REGISTER, register 25 would
> > be used both as the argptr and as a placeholder for some
> > temporary register, with very interesting consequences :-)
> 
> hm.. the problem I see is that the register allocator is initialised
> with random garbage. Eg FIXED_REGISTERS is 24 elts big, thus 
> initial_fixed_regs[] is also only 24 elts and in init_reg_sets() we
> copy a random 25th elt into fixed_regs[] and risk segfault.
> I am pretty sure more it results in more damage later on.

  Yes, that looks like a bug. i386.h has "argptr" included in
  FIXED_REGISTERS and gives it a nonzero value.

  Gunther
    

Re: more m68k breakage on m68k-linux

[Richard Henderson]

On Tue, Mar 02, 2004 at 11:43:00AM +0100, Gunther Nikl wrote:
> > I am pretty sure more it results in more damage later on.
> 
>   Yes, that looks like a bug. i386.h has "argptr" included in
>   FIXED_REGISTERS and gives it a nonzero value.

Yes.  There are three such arrays that *must* include *all*
registers, including such special purpose registers.


r~

Re: more m68k breakage on m68k-linux

[Roman Zippel]

Hi,

Richard Zidlicky wrote:

> hm.. the problem I see is that the register allocator is initialised
> with random garbage. Eg FIXED_REGISTERS is 24 elts big, thus 
> initial_fixed_regs[] is also only 24 elts and in init_reg_sets() we
> copy a random 25th elt into fixed_regs[] and risk segfault.
> I am pretty sure more it results in more damage later on.

I can confirm that it causes problems, after fixing FIXED_REGISTERS and 
CALL_USED_REGISTERS I can finally bootstrap gcc again, but my tree has 
another fix for a subtle miscompilation, which I still have to test 
separately. reg_class and all related macros still have to be fixed though.

bye, Roman

Re: more m68k breakage on m68k-linux

[Richard Henderson]

On Tue, Mar 02, 2004 at 08:32:44PM +0100, Roman Zippel wrote:
> I can confirm that it causes problems, after fixing FIXED_REGISTERS and 
> CALL_USED_REGISTERS ...

You also need REG_ALLOC_ORDER.


r~

Re: more m68k breakage on m68k-linux

[Richard Zidlicky]

On Tue, Mar 02, 2004 at 08:32:44PM +0100, Roman Zippel wrote:
> Hi,
> 
> Richard Zidlicky wrote:
> 
> >hm.. the problem I see is that the register allocator is initialised
> >with random garbage. Eg FIXED_REGISTERS is 24 elts big, thus 
> >initial_fixed_regs[] is also only 24 elts and in init_reg_sets() we
> >copy a random 25th elt into fixed_regs[] and risk segfault.
> >I am pretty sure more it results in more damage later on.
> 
> I can confirm that it causes problems, after fixing FIXED_REGISTERS and 
> CALL_USED_REGISTERS I can finally bootstrap gcc again, 

same here.

> but my tree has 
> another fix for a subtle miscompilation, which I still have to test 
> separately. reg_class and all related macros still have to be fixed though.

I have another few miscompilations so I am awaiting your patches :)

Richard

Re: more m68k breakage on m68k-linux

[Bernardo Innocenti]

Richard Zidlicky wrote:

>>>Note the typo "FIRST_PSEOUDO_REGISTER".. made it somewhat hard to
>>>find the change.
>>
>>Oops!  That's very unfortunate.  I've fixed it in the ChangeLog.
> 
> thanks.. I should have used agrep or something :)

What's that?  Is it perhaps an heuristic version of grep?
I'd badly need something like that! :-)


>>If I didn't increment FIRST_PSEUDO_REGISTER, register 25 would
>>be used both as the argptr and as a placeholder for some
>>temporary register, with very interesting consequences :-)
> 
> hm.. the problem I see is that the register allocator is initialised
> with random garbage. Eg FIXED_REGISTERS is 24 elts big, thus 
> initial_fixed_regs[] is also only 24 elts and in init_reg_sets() we
> copy a random 25th elt into fixed_regs[] and risk segfault.
> I am pretty sure more it results in more damage later on.

That's a nice catch, and it would explain all the strange
bootstrap failures we've seen, but... I can't see this bug
in current mainline code:

void
init_reg_sets (void)
{
  int i, j;

  /* First copy the register information from the initial int form into
     the regsets.  */

  for (i = 0; i < N_REG_CLASSES; i++)
    {
      CLEAR_HARD_REG_SET (reg_class_contents[i]);

      /* Note that we hard-code 32 here, not HOST_BITS_PER_INT.  */
      for (j = 0; j < FIRST_PSEUDO_REGISTER; j++)
        if (int_reg_class_contents[i][j / 32]
            & ((unsigned) 1 << (j % 32)))
          SET_HARD_REG_BIT (reg_class_contents[i], j);
    }

  memcpy (fixed_regs, initial_fixed_regs, sizeof fixed_regs);
  memcpy (call_used_regs, initial_call_used_regs, sizeof call_used_regs);
  memset (global_regs, 0, sizeof global_regs);

  /* Do any additional initialization regsets may need.  */
  INIT_ONCE_REG_SET ();

#ifdef REG_ALLOC_ORDER
  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
    inv_reg_alloc_order[reg_alloc_order[i]] = i;
#endif
}

Moreover, CVS annotate shows that this function has not been
changed significantly since year 2000.

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: more m68k breakage on m68k-linux

[Bernardo Innocenti]

Roman Zippel wrote:

>> hm.. the problem I see is that the register allocator is initialised
>> with random garbage. Eg FIXED_REGISTERS is 24 elts big, thus 
>> initial_fixed_regs[] is also only 24 elts and in init_reg_sets() we
>> copy a random 25th elt into fixed_regs[] and risk segfault.
>> I am pretty sure more it results in more damage later on.
> 
> 
> I can confirm that it causes problems, after fixing FIXED_REGISTERS and 
> CALL_USED_REGISTERS I can finally bootstrap gcc again, but my tree has 
> another fix for a subtle miscompilation, which I still have to test 
> separately. reg_class and all related macros still have to be fixed though.

Well done!

I'm convinced that argptr missing from FIXED_REGISTERS was an anomaly,
but I still don't quite understand how it would affect init_reg_sets().
Could you elaborate?

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: more m68k breakage on m68k-linux

[Roman Zippel]

Hi,

On Wed, 3 Mar 2004, Bernardo Innocenti wrote:

> I'm convinced that argptr missing from FIXED_REGISTERS was an anomaly,
> but I still don't quite understand how it would affect init_reg_sets().
> Could you elaborate?

I compared the test results between the stage1 and stage2 compiler and
checked the differences. When fixed_regs is initialized from
initial_fixed_regs the last value is practically random, which e.g.
produces wrong results in assign_parms() (extra copy of
virtual_incoming_args_rtx). A bit later gcc produces a ICE,
I haven't checked what exactly went wrong further, but fixing the
initialization also fixed the test case.

bye, Roman

Re: more m68k breakage on m68k-linux

[Andreas Schwab]

Bernardo Innocenti <bernie@develer.com> writes:

> I'm convinced that argptr missing from FIXED_REGISTERS was an anomaly,
> but I still don't quite understand how it would affect init_reg_sets().
> Could you elaborate?

  memcpy (fixed_regs, initial_fixed_regs, sizeof fixed_regs);
  memcpy (call_used_regs, initial_call_used_regs, sizeof call_used_regs);

sizeof fixed_regs > sizeof initial_fixed_regs
sizeof call_used_regs > sizeof initial_call_used_regs

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: more m68k breakage on m68k-linux

[Richard Zidlicky]

On Wed, Mar 03, 2004 at 09:22:08AM +0100, Bernardo Innocenti wrote:
> Richard Zidlicky wrote:
> 
> >>>Note the typo "FIRST_PSEOUDO_REGISTER".. made it somewhat hard to
> >>>find the change.
> >>
> >>Oops!  That's very unfortunate.  I've fixed it in the ChangeLog.
> >
> >thanks.. I should have used agrep or something :)
> 
> What's that?  Is it perhaps an heuristic version of grep?
> I'd badly need something like that! :-)

http://laurikari.net/tre/

I have just installed it a few days ago so not many experiences
yet.

 agrep -2 FIRST_PSEUDO_REGISTER ChangeLog*

> >>If I didn't increment FIRST_PSEUDO_REGISTER, register 25 would
> >>be used both as the argptr and as a placeholder for some
> >>temporary register, with very interesting consequences :-)
> >
> >hm.. the problem I see is that the register allocator is initialised
> >with random garbage. Eg FIXED_REGISTERS is 24 elts big, thus 
> >initial_fixed_regs[] is also only 24 elts and in init_reg_sets() we
> >copy a random 25th elt into fixed_regs[] and risk segfault.
> >I am pretty sure more it results in more damage later on.
> 
> That's a nice catch, and it would explain all the strange
> bootstrap failures we've seen, but... I can't see this bug
> in current mainline code:
> 
> void
> init_reg_sets (void)
> {
>  int i, j;
> 
>  /* First copy the register information from the initial int form into
>     the regsets.  */
> 
>  for (i = 0; i < N_REG_CLASSES; i++)
>    {
>      CLEAR_HARD_REG_SET (reg_class_contents[i]);
> 
>      /* Note that we hard-code 32 here, not HOST_BITS_PER_INT.  */
>      for (j = 0; j < FIRST_PSEUDO_REGISTER; j++)
>        if (int_reg_class_contents[i][j / 32]
>            & ((unsigned) 1 << (j % 32)))
>          SET_HARD_REG_BIT (reg_class_contents[i], j);
>    }
> 
>  memcpy (fixed_regs, initial_fixed_regs, sizeof fixed_regs);


  sizeof fixed_regs != sizeof initial_fixed_regs

<rant>
I vote for rewriting gcc in Lisp, Ocaml or Prolog. It does already
have two garbage collectors and implements a large superset of every
Lisp interpreter out there. It can only get faster if implemented
in a higher level PL. Many Lisp dialects (and not only those) offer
far superior HW independence and portability compared to C and we
do still spend too much time hunting bugs that would be outright
impossible in a higher level language.
</rant>

Richard

Re: more m68k breakage on m68k-linux

[Bernardo Innocenti]

Andreas Schwab wrote:
> Bernardo Innocenti <bernie@develer.com> writes:
> 
> 
>>I'm convinced that argptr missing from FIXED_REGISTERS was an anomaly,
>>but I still don't quite understand how it would affect init_reg_sets().
>>Could you elaborate?
> 
> 
>   memcpy (fixed_regs, initial_fixed_regs, sizeof fixed_regs);
>   memcpy (call_used_regs, initial_call_used_regs, sizeof call_used_regs);
> 
> sizeof fixed_regs > sizeof initial_fixed_regs
> sizeof call_used_regs > sizeof initial_call_used_regs

Oh, I see.  I had been thinking about memory trashing,
not uninitialized memory.

I apologize for being the root cause of such a subtle
bug.

To make sure this particular case will never show up again
on any backend, I'd like to add some static sanity checks.
We can't use the preprocessor because it doesn't know about
structure sizes.

Woundn't it be nice to have something like BOOST's static
assert for C?  I've come up with this hack:

#define __STATIC_ASSERT_PASTE2(x,y) x ## y
#define __STATIC_ASSEET_PASTE(x,y) __PASTE2(x,y)

#define STATIC_ASSERT(EXPR) \
    typedef struct { \
         char ASSERTION_FAILED[(EXPR) ? 1 : -1]; \
    } __STATIC_ASSERT_PASTE(__assertion_, __LINE__)


Here is a test case to show how it works:

 int main(void)
 {
     static const int a = 42;

     STATIC_ASSERT(sizeof(short) == sizeof(long));  // ERROR!
     STATIC_ASSERT(sizeof(short) * 2 == sizeof(long)); OK
     STATIC_ASSERT(10 > 30); // ERROR!
     STATIC_ASSERT(a > 10); // OK
     STATIC_ASSERT(a > 50); // OK??? Is this a GCC bug?
     return 0;
 }


Do you think we could include something like this
somewhere in GCC?

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: more m68k breakage on m68k-linux

[Richard Henderson]

On Wed, Mar 03, 2004 at 11:38:38PM +0100, Bernardo Innocenti wrote:
> Woundn't it be nice to have something like BOOST's static
> assert for C?  I've come up with this hack:

Why bother?  Just make it a regular abort() check somewhere.


r~

Re: more m68k breakage on m68k-linux

[Gunther Nikl]

On Wed, Mar 03, 2004 at 04:35:46PM -0800, Richard Henderson wrote:
> On Wed, Mar 03, 2004 at 11:38:38PM +0100, Bernardo Innocenti wrote:
> > Woundn't it be nice to have something like BOOST's static
> > assert for C?  I've come up with this hack:
> 
> Why bother?  Just make it a regular abort() check somewhere.

  Wrapped into ENABLE_CHECKING?

  Gunther

Re: more m68k breakage on m68k-linux

[Gunther Nikl]

On Tue, Mar 02, 2004 at 08:32:44PM +0100, Roman Zippel wrote:
> 
> Richard Zidlicky wrote:
> 
> >hm.. the problem I see is that the register allocator is initialised
> >with random garbage. Eg FIXED_REGISTERS is 24 elts big, thus 
> >initial_fixed_regs[] is also only 24 elts and in init_reg_sets() we
> >copy a random 25th elt into fixed_regs[] and risk segfault.
> >I am pretty sure more it results in more damage later on.
> 
> I can confirm that it causes problems, after fixing FIXED_REGISTERS and 
> CALL_USED_REGISTERS I can finally bootstrap gcc again, but my tree has 
> another fix for a subtle miscompilation, which I still have to test 
> separately. reg_class and all related macros still have to be fixed though.

  Can you please post the patch that corrects the FIXED_REGISTERS problem?

  Gunther

Re: more m68k breakage on m68k-linux

[Bernardo Innocenti]

Gunther Nikl wrote:

>>I can confirm that it causes problems, after fixing FIXED_REGISTERS and 
>>CALL_USED_REGISTERS I can finally bootstrap gcc again, but my tree has 
>>another fix for a subtle miscompilation, which I still have to test 
>>separately. reg_class and all related macros still have to be fixed though.
> 
>   Can you please post the patch that corrects the FIXED_REGISTERS problem?

I will.  Please allow some time since I'm on a tight work schedule
right now.  I'll regtest the patch on m68k-linux and m68k-netbsd.
Additionally, I'll build the m68k-uclinux kernel & userland.

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: more m68k breakage on m68k-linux

[Roman Zippel]

Hi,

On Thu, 4 Mar 2004, Bernardo Innocenti wrote:

> >   Can you please post the patch that corrects the FIXED_REGISTERS problem?
>
> I will.  Please allow some time since I'm on a tight work schedule
> right now.  I'll regtest the patch on m68k-linux and m68k-netbsd.
> Additionally, I'll build the m68k-uclinux kernel & userland.

Does your patch look very different from the one below?
That's the one I'm testing for m68k-linux right now.

bye, Roman

Index: config/m68k/m68k.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/m68k/m68k.c,v
retrieving revision 1.128
diff -u -r1.128 m68k.c
--- config/m68k/m68k.c	17 Feb 2004 20:24:44 -0000	1.128
+++ config/m68k/m68k.c	4 Mar 2004 11:25:54 -0000
@@ -44,6 +44,18 @@
 #include "debug.h"
 #include "flags.h"

+char regno_reg_class[] =
+{
+  DATA_REGS, DATA_REGS, DATA_REGS, DATA_REGS,
+  DATA_REGS, DATA_REGS, DATA_REGS, DATA_REGS,
+  ADDR_REGS, ADDR_REGS, ADDR_REGS, ADDR_REGS,
+  ADDR_REGS, ADDR_REGS, ADDR_REGS, ADDR_REGS,
+  FP_REGS, FP_REGS, FP_REGS, FP_REGS,
+  FP_REGS, FP_REGS, FP_REGS, FP_REGS,
+  ADDR_REGS
+};
+
+
 /* The ASM_DOT macro allows easy string pasting to handle the differences
    between MOTOROLA and MIT syntaxes in asm_fprintf(), which doesn't
    support the %. option.  */
Index: config/m68k/m68k.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/m68k/m68k.h,v
retrieving revision 1.109
diff -u -r1.109 m68k.h
--- config/m68k/m68k.h	15 Feb 2004 17:46:02 -0000	1.109
+++ config/m68k/m68k.h	4 Mar 2004 11:25:55 -0000
@@ -486,7 +486,10 @@
                                \
   /* Floating point registers  \
      (if available).  */       \
-  0, 0, 0, 0, 0, 0, 0, 0 }
+  0, 0, 0, 0, 0, 0, 0, 0,      \
+                               \
+  /* Arg pointer.  */          \
+  1 }

 /* 1 for registers not available across function calls.
    These must include the FIXED_REGISTERS and also any
@@ -497,7 +500,18 @@
 #define CALL_USED_REGISTERS \
  {1, 1, 0, 0, 0, 0, 0, 0,   \
   1, 1, 0, 0, 0, 0, 0, 1,   \
-  1, 1, 0, 0, 0, 0, 0, 0 }
+  1, 1, 0, 0, 0, 0, 0, 0, 1 }
+
+#define REG_ALLOC_ORDER		\
+{ /* d0/d1/a0/a1 */		\
+  0, 1, 8, 9,			\
+  /* d2-d7 */			\
+  2, 3, 4, 5, 6, 7,		\
+  /* a2-a7/arg */		\
+  10, 11, 12, 13, 14, 15, 24,	\
+  /* fp0-fp7 */			\
+  16, 17, 18, 19, 20, 21, 22, 23\
+}


 /* Make sure everything's fine if we *don't* have a given processor.
@@ -636,12 +650,12 @@
 {					\
   {0x00000000},  /* NO_REGS */		\
   {0x000000ff},  /* DATA_REGS */	\
-  {0x0000ff00},  /* ADDR_REGS */	\
+  {0x0100ff00},  /* ADDR_REGS */	\
   {0x00ff0000},  /* FP_REGS */		\
-  {0x0000ffff},  /* GENERAL_REGS */	\
+  {0x0100ffff},  /* GENERAL_REGS */	\
   {0x00ff00ff},  /* DATA_OR_FP_REGS */	\
-  {0x00ffff00},  /* ADDR_OR_FP_REGS */	\
-  {0x00ffffff},  /* ALL_REGS */		\
+  {0x01ffff00},  /* ADDR_OR_FP_REGS */	\
+  {0x01ffffff},  /* ALL_REGS */		\
 }

 /* The same information, inverted:
@@ -649,7 +663,8 @@
    reg number REGNO.  This could be a conditional expression
    or could index an array.  */

-#define REGNO_REG_CLASS(REGNO) (((REGNO)>>3)+1)
+extern char regno_reg_class[];
+#define REGNO_REG_CLASS(REGNO) ((enum reg_class)regno_reg_class[(REGNO)])

 /* The class value for index registers, and the one for base regs.  */

Re: more m68k breakage on m68k-linux

[Gunther Nikl]

Hello Roman,

On Thu, Mar 04, 2004 at 02:44:26PM +0100, Roman Zippel wrote:
> 
> On Thu, 4 Mar 2004, Bernardo Innocenti wrote:
> 
> > >   Can you please post the patch that corrects the FIXED_REGISTERS problem?
> >
> > I will.  Please allow some time since I'm on a tight work schedule
> > right now.  I'll regtest the patch on m68k-linux and m68k-netbsd.
> > Additionally, I'll build the m68k-uclinux kernel & userland.
> 
> Does your patch look very different from the one below?
> That's the one I'm testing for m68k-linux right now.

  Thanks for providing something to look at!

> Index: config/m68k/m68k.c
> ===================================================================
> RCS file: /cvsroot/gcc/gcc/gcc/config/m68k/m68k.c,v
> retrieving revision 1.128
> diff -u -r1.128 m68k.c
> --- config/m68k/m68k.c	17 Feb 2004 20:24:44 -0000	1.128
> +++ config/m68k/m68k.c	4 Mar 2004 11:25:54 -0000
> @@ -44,6 +44,18 @@
>  #include "debug.h"
>  #include "flags.h"
> 
> +char regno_reg_class[] =
> +{
> +  DATA_REGS, DATA_REGS, DATA_REGS, DATA_REGS,
> +  DATA_REGS, DATA_REGS, DATA_REGS, DATA_REGS,
> +  ADDR_REGS, ADDR_REGS, ADDR_REGS, ADDR_REGS,
> +  ADDR_REGS, ADDR_REGS, ADDR_REGS, ADDR_REGS,
> +  FP_REGS, FP_REGS, FP_REGS, FP_REGS,
> +  FP_REGS, FP_REGS, FP_REGS, FP_REGS,
> +  ADDR_REGS
> +};

  Wouldn't it be better to make this "const enum reg_class"? Then you don't
  need to cast in REGNO_REG_CLASS. I guess using char[] saves space?

  Gunther

Re: more m68k breakage on m68k-linux

[Richard Zidlicky]

On Wed, Mar 03, 2004 at 11:38:38PM +0100, Bernardo Innocenti wrote:

> To make sure this particular case will never show up again
> on any backend, I'd like to add some static sanity checks.
> We can't use the preprocessor because it doesn't know about
> structure sizes.

actually, in this particular case the compiler has all
necessary information to catch the error, any chance
gcc could grasp simple memcpy overruns when the size
of both operands is known?

Adding yet another check for an extremely unlikely condition
may not be a good tradeoff at this place.

Richard

Re: more m68k breakage on m68k-linux

[Andreas Schwab]

Richard Zidlicky <rz@linux-m68k.org> writes:

> Adding yet another check for an extremely unlikely condition
> may not be a good tradeoff at this place.

On the other hand, the check is very cheap (can be optimized away
completely by the compiler) and can help catching future bugs.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: more m68k breakage on m68k-linux

[Bernardo Innocenti]

Roman Zippel wrote:
> Hi,
> 
> On Thu, 4 Mar 2004, Bernardo Innocenti wrote:
> 
> 
>>>  Can you please post the patch that corrects the FIXED_REGISTERS problem?
>>
>>I will.  Please allow some time since I'm on a tight work schedule
>>right now.  I'll regtest the patch on m68k-linux and m68k-netbsd.
>>Additionally, I'll build the m68k-uclinux kernel & userland.
> 
> 
> Does your patch look very different from the one below?
> That's the one I'm testing for m68k-linux right now.

It's similar, but I didn't notice REGNO_REG_CLASS also
needed fiddling, so your patch is superior.

Do you have a copyright assignment on file?  If so, once
it's properly regtested we could apply your patch with
the improvement suggested by Gunther.

Otherwise, I'll use Vulcan mind meld to forget your
patch and then redo it from scratch ;-)

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: more m68k breakage on m68k-linux

[Gunther Nikl]

On Sun, Mar 07, 2004 at 04:05:50PM +0100, Bernardo Innocenti wrote:
> Roman Zippel wrote:
> >
> >Does your patch look very different from the one below?
> >That's the one I'm testing for m68k-linux right now.
> 
> It's similar, but I didn't notice REGNO_REG_CLASS also needed fiddling,
> so your patch is superior.

  Why are so many changes required to fix the breakage caused by adding
  argptr?

  Gunther
  

Re: more m68k breakage on m68k-linux

[Bernardo Innocenti]

Gunther Nikl wrote:
> On Sun, Mar 07, 2004 at 04:05:50PM +0100, Bernardo Innocenti wrote:
> 
>>Roman Zippel wrote:
>>
>>>Does your patch look very different from the one below?
>>>That's the one I'm testing for m68k-linux right now.
>>
>>It's similar, but I didn't notice REGNO_REG_CLASS also needed fiddling,
>>so your patch is superior.
> 
> 
>   Why are so many changes required to fix the breakage caused by adding
>   argptr?

Because the target configuration infrastructure used by
GCC is a hell ofpreprocessor macros with very weak
consistency checking? :-)

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: You ucLinux patches break RTEMS building

[Bernardo Innocenti]

Peter Barada wrote:
> Bernie,
> 
> The patches you came up with for ucLinux interact with the patches for
> rtems and it looks like the two conflict.

Do you mean on 3.4?

> I remember you were talking about putting together a config target
> (m68k-uclinux) to prevent your changes from conflicting with the raw
> target (m68k-elf).
>
> I don't remember if you've done that yet.  Any progress?

I already did.  As far as I can remember, I didn't change the
behavior of m68k-elf.  Any changes to the headers should be
cleanups.

Please note that we've just spotted a serious source of
breakage on the m68k backend that was causing several
significant regressions.  It was caused by my followups
of your ColdFire patches.

Please see the thread with subject "more m68k breakage on
m68k-linux".

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Re: more m68k breakage on m68k-linux

[Gunther Nikl]

Hello Bernado!

On Mon, Mar 08, 2004 at 10:38:03PM +0100, Bernardo Innocenti wrote:
> Gunther Nikl wrote:
> >On Sun, Mar 07, 2004 at 04:05:50PM +0100, Bernardo Innocenti wrote:
> >
> >>Roman Zippel wrote:
> >>
> >>>Does your patch look very different from the one below?
> >>>That's the one I'm testing for m68k-linux right now.
> >>
> >>It's similar, but I didn't notice REGNO_REG_CLASS also needed fiddling,
> >>so your patch is superior.
> >
> >  Why are so many changes required to fix the breakage caused by adding
> >  argptr?
> 
> Because the target configuration infrastructure used by GCC is a hell
> ofpreprocessor macros with very weak consistency checking? :-)

  No, that wasn't my real question ;-) I thought that just adding argptr
  to FIXED_REGISTERS+CALL_USED_REGISTERS would be enough. But the patch
  adds REG_ALLOC_ORDER and modifies REG_CLASS_CONTENTS+REGNO_REG_CLASS.
  I checked the documenation but it wasn't that helpful regarding
  REG_ALLOC_ORDER. Why is that one now needed?
  BTW, the patch should be posted quickly because  according to
  http://gcc.gnu.org/ml/gcc-patches/2004-03/msg00513.html 3.4.0 is
  scheduled for its final code freeze soon. IMHO, this patch must be
  in 3.4.0.

  Thank you,
  Gunther

Re: more m68k breakage on m68k-linux

[Bernardo Innocenti]

Gunther Nikl wrote:
> Hello Bernado!
> 
> On Mon, Mar 08, 2004 at 10:38:03PM +0100, Bernardo Innocenti wrote:
> 
>>Gunther Nikl wrote:
>>
>>>On Sun, Mar 07, 2004 at 04:05:50PM +0100, Bernardo Innocenti wrote:
>>>
>>>
>>>>Roman Zippel wrote:
>>>>
>>>>
>>>>>Does your patch look very different from the one below?
>>>>>That's the one I'm testing for m68k-linux right now.
>>>>
>>>>It's similar, but I didn't notice REGNO_REG_CLASS also needed fiddling,
>>>>so your patch is superior.
>>>
>>> Why are so many changes required to fix the breakage caused by adding
>>> argptr?
>>
>>Because the target configuration infrastructure used by GCC is a hell
>>ofpreprocessor macros with very weak consistency checking? :-)



> 
>   No, that wasn't my real question ;-) I thought that just adding argptr
>   to FIXED_REGISTERS+CALL_USED_REGISTERS would be enough. But the patch
>   adds REG_ALLOC_ORDER and modifies REG_CLASS_CONTENTS+REGNO_REG_CLASS.

Well, the REGNO_REG_CLASS is needed for correctness in case the middle-end
invokes it on the argument pointer.  I dunno if it really does, but better
safe than sorry.  The divide-by-8 trick doesn't work any more, so Roman
converted the clever formula to a table.

For other's reference:

`REGNO_REG_CLASS (REGNO)'
     A C expression whose value is a register class containing hard
     register REGNO.  In general there is more than one such class;
     choose a class which is "minimal", meaning that no smaller class
     also contains the register.

Roman's changes:

 -#define REGNO_REG_CLASS(REGNO) (((REGNO)>>3)+1)
 +extern char regno_reg_class[];
 +#define REGNO_REG_CLASS(REGNO) ((enum reg_class)regno_reg_class[(REGNO)])

 ...

 +char regno_reg_class[] =
 +{
 +  DATA_REGS, DATA_REGS, DATA_REGS, DATA_REGS,
 +  DATA_REGS, DATA_REGS, DATA_REGS, DATA_REGS,
 +  ADDR_REGS, ADDR_REGS, ADDR_REGS, ADDR_REGS,
 +  ADDR_REGS, ADDR_REGS, ADDR_REGS, ADDR_REGS,
 +  FP_REGS, FP_REGS, FP_REGS, FP_REGS,
 +  FP_REGS, FP_REGS, FP_REGS, FP_REGS,
 +  ADDR_REGS
 +};


>   I checked the documenation but it wasn't that helpful regarding
>   REG_ALLOC_ORDER. Why is that one now needed?

I'm puzzled too... I think Roman wanted to make sure argptr wasn't
being ever allocated, but this is easly accomplished by setting it
to 1 in FIXED_REGISTERS.

All code paths for targets that don't define REG_ALLOC_ORDER look
safe to me even for the argument pointer fake register.


>   BTW, the patch should be posted quickly because  according to
>   http://gcc.gnu.org/ml/gcc-patches/2004-03/msg00513.html 3.4.0 is
>   scheduled for its final code freeze soon. IMHO, this patch must be
>   in 3.4.0.

Yeah.  Sorry for the delay, I'll see if I can work on it later this
week.

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/