Testing m68k changes on AmigaOS and Linux/m68k

[Bernardo Innocenti]

Gunther Nikl wrote:
> On Wed, Oct 15, 2003 at 11:32:47AM +0200, Gunther Nikl wrote:
> 
>>>The compiler used to work fine with m68k-elf and m68k-uclinux last week
>>>(last tested from CVS sources on 20031011).
>>
>>  Are you sure? ;-)
> 
>   Argl! Turns out that bug was a bug in my patches which still redefined
>   ARG_POINTER_REGNUM to FRAME_POINTER_REGNUM :-/ Sorry for the false report
>   about that issue.

Feew... I was sweating quite a lot trying to guess
what could possibly got broken...

This is what I got from your test case with -O1
-fomit-frame-pointer:

foo:
        link.w %a6,#-4
        pea -4(%a6)
        pea 8(%a6)
        jbsr bar
        addq.l #8,%sp
        unlk %a6
        rts

As you can see, it's using the frame pointer even though it's
been disabled. The offsets are all correct, but I wonder why
the FP can't be eliminated for this simple case.

There could be something wrong in ELIMINABLE_REGS or
CAN_ELIMINATE...


>   However, I found a bug that was hidden by the wrong test when saving
>   registers. The wrong mask is used when saving multiple registers in
>   m68k_output_function_prologue(). The diff includes the original change.
>   No ChangeLog entry this time.


IIRC, the mask to be used for movem in the ColdFire is
the same for both saving and restoring registers (there's
no post-increment/pre-decrement in movem).

In the 680x0, we need the reversed mask when storing
and the straight one for restoring.

> @@ -1007,17 +1007,17 @@ m68k_output_function_prologue (FILE *str
>  	     the fsize_with_regs amount.  */
>  
>  #ifdef MOTOROLA
> -	  asm_fprintf (stream, "\tmovm.l %I0x%x,(%Rsp)\n", current_frame.reg_mask);
> +	  asm_fprintf (stream, "\tmovm.l %I0x%x,(%Rsp)\n", current_frame.reg_rev_mask);
>  #else
> -	  asm_fprintf (stream, "\tmoveml %I0x%x,%Rsp@\n", current_frame.reg_mask);
> +	  asm_fprintf (stream, "\tmoveml %I0x%x,%Rsp@\n", current_frame.reg_rev_mask);
>  #endif

...so, this code was correct for the ColdFire and shouldn't
be changed (I wouldn't get a working Linux kernel otherwise).


>  #ifdef MOTOROLA
> -	  asm_fprintf (stream, "\tmovm.l %I0x%x,-(%Rsp)\n", current_frame.reg_mask);
> +	  asm_fprintf (stream, "\tmovm.l %I0x%x,-(%Rsp)\n", current_frame.reg_rev_mask);
>  #else
> -	  asm_fprintf (stream, "\tmoveml %I0x%x,%Rsp@-\n", current_frame.reg_mask);
> +	  asm_fprintf (stream, "\tmoveml %I0x%x,%Rsp@-\n", current_frame.reg_rev_mask);
>  #endif
>  	}
>        if (dwarf2out_do_frame ())

For this, please accept a mea culpa, mea culpa,
mea maxima culpa :-)


> @@ -1029,7 +1029,7 @@ m68k_output_function_prologue (FILE *str
>  	  if (! frame_pointer_needed)
>  	    dwarf2out_def_cfa (l, STACK_POINTER_REGNUM, cfa_offset);
>  	  for (regno = 0, n_regs = 0; regno < 16; regno++)
> -	    if (current_frame.reg_mask & (1 << regno))
> +	    if (current_frame.reg_rev_mask & (1 << regno))
>  	      dwarf2out_reg_save (l, regno,
>  				  -cfa_offset + n_regs++ * 4);
>  	}

Are you sure about this? I'm pretty sure that when regno
is n, the correct bit to test with (1<<n) would be in
the straight mask.

What's the push order of movem on the 68000? If it
pushes registers from D0 to A7, then the offset is
also fine.

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

Please don't send Word attachments - http://www.gnu.org/philosophy/no-word-attachments.html