This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: Testing m68k changes on AmigaOS and Linux/m68k
On Wed, Oct 15, 2003 at 08:53:58PM +0200, Bernardo Innocenti wrote:
> Gunther Nikl wrote:
> > 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...
When I first inspected your patch I noticed that I had to remove the
ARG_POINTER_REGNUM redefine but then I forgot doing that when your
patch was committed :-/
> 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.
I know that behaviour. Thats why I used __regargs :)
> The offsets are all correct, but I wonder why the FP can't be eliminated
> for this simple case.
Yes, with framepointer it was ok. I guess that the FP can't be eliminated
because that would change the offset into the frame and tracking that is
probably hard.
> There could be something wrong in ELIMINABLE_REGS or CAN_ELIMINATE...
I would like to see FP eliminated all the time when -fomit-frame-pointer
is used ;-)
> > 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).
I didn't inspect the former version in great detail thus I missed that
in this part the normal mask was computed from the rev_mask for COLDFIRE.
> In the 680x0, we need the reversed mask when storing and the straight one
> for restoring.
Good, but I fear in that case your patch is completely broken.
The FPU case seems to be messed up too...
> >@@ -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.
Now I see, you changed the test compared with the old code. I looked how
the mask used here was computed before and that was
mask |= 1 << (15 - regno);
which is
(reg_rev_mask =) rmask |= 1 << (15 - regno);
in your patch. Thus the new code is correct. Hm, maybe it should be as
before and use reg_rev_mask for consistency within the prologue function
(except COLDFIRE ;-) I changed it back to use reg_rev_mask.
The same applies to the fpu prologue generation. The fpu epilogue seems
to be broken as well.
Please take a look at diff between 1.107 and 1.108 to see what I mean.
> What's the push order of movem on the 68000? If it pushes registers from
> D0 to A7, then the offset is also fine.
I believe that part is fine.
Gunther
PS: I also moved some comments around to the new places they belong to.
--cut--
--- m68k.c_ Mon Oct 13 14:39:45 2003
+++ m68k.c Thu Oct 16 14:40:41 2003
@@ -614,6 +614,9 @@ look_for_reg:
return 0;
}
+/* Note that the order of the bit mask for fmovem is the opposite
+ of the order for movem! */
+
static void
m68k_compute_frame_layout (void)
{
@@ -682,7 +685,12 @@ m68k_initial_elimination_offset (int fro
abort();
}
-/* Return true if we need to save REGNO. */
+/* Refer to the array `regs_ever_live' to determine which registers
+ to save; `regs_ever_live[I]' is nonzero if register number I
+ is ever used in the function. This function is responsible for
+ knowing which registers should not be saved even if used.
+ Return true if we need to save REGNO. */
+
static bool
m68k_save_reg (unsigned int regno, bool interrupt_handler)
{
@@ -736,15 +744,7 @@ m68k_save_reg (unsigned int regno, bool
/* This function generates the assembly code for function entry.
STREAM is a stdio stream to output the code to.
- SIZE is an int: how many units of temporary storage to allocate.
- Refer to the array `regs_ever_live' to determine which registers
- to save; `regs_ever_live[I]' is nonzero if register number I
- is ever used in the function. This function is responsible for
- knowing which registers should not be saved even if used. */
-
-
-/* Note that the order of the bit mask for fmovem is the opposite
- of the order for movem! */
+ SIZE is an int: how many units of temporary storage to allocate. */
static void
m68k_output_function_prologue (FILE *stream, HOST_WIDE_INT size ATTRIBUTE_UNUSED)
@@ -925,12 +925,12 @@ m68k_output_function_prologue (FILE *str
if (TARGET_68881)
{
- if (current_frame.fpu_mask)
+ if (current_frame.fpu_rev_mask)
{
#ifdef MOTOROLA
- asm_fprintf (stream, "\tfmovm %I0x%x,-(%Rsp)\n", current_frame.fpu_mask);
+ asm_fprintf (stream, "\tfmovm %I0x%x,-(%Rsp)\n", current_frame.fpu_rev_mask);
#else
- asm_fprintf (stream, "\tfmovem %I0x%x,%Rsp@-\n", current_frame.fpu_mask);
+ asm_fprintf (stream, "\tfmovem %I0x%x,%Rsp@-\n", current_frame.fpu_rev_mask);
#endif
if (dwarf2out_do_frame ())
{
@@ -941,7 +941,7 @@ m68k_output_function_prologue (FILE *str
if (! frame_pointer_needed)
dwarf2out_def_cfa (l, STACK_POINTER_REGNUM, cfa_offset);
for (regno = 16, n_regs = 0; regno < 24; regno++)
- if (current_frame.fpu_mask & (1 << (regno - 16)))
+ if (current_frame.fpu_rev_mask & (1 << (regno - 16)))
dwarf2out_reg_save (l, regno,
-cfa_offset + n_regs++ * 12);
}
@@ -966,7 +966,7 @@ m68k_output_function_prologue (FILE *str
warning ("stack limit expression is not supported");
}
- if (num_saved_regs <= 2)
+ if (current_frame.reg_no <= 2)
{
/* Store each separately in the same order moveml uses.
Using two movel instructions instead of a single moveml
@@ -1015,9 +1015,9 @@ m68k_output_function_prologue (FILE *str
else
{
#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 ())
@@ -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 << (15 - regno)))
dwarf2out_reg_save (l, regno,
-cfa_offset + n_regs++ * 4);
}
@@ -1293,7 +1293,7 @@ m68k_output_function_epilogue (FILE *str
}
}
}
- if (current_frame.fpu_rev_mask)
+ if (current_frame.fpu_mask)
{
if (big)
{
@@ -1301,22 +1301,22 @@ m68k_output_function_epilogue (FILE *str
asm_fprintf (stream, "\tfmovm -%wd(%s,%Ra1.l),%I0x%x\n",
current_frame.foffset + fsize,
reg_names[FRAME_POINTER_REGNUM],
- current_frame.fpu_rev_mask);
+ current_frame.fpu_mask);
#else
asm_fprintf (stream, "\tfmovem %s@(-%wd,%Ra1:l),%I0x%x\n",
reg_names[FRAME_POINTER_REGNUM],
current_frame.foffset + fsize,
- current_frame.fpu_rev_mask);
+ current_frame.fpu_mask);
#endif
}
else if (restore_from_sp)
{
#ifdef MOTOROLA
asm_fprintf (stream, "\tfmovm (%Rsp)+,%I0x%x\n",
- current_frame.fpu_rev_mask);
+ current_frame.fpu_mask);
#else
asm_fprintf (stream, "\tfmovem %Rsp@+,%I0x%x\n",
- current_frame.fpu_rev_mask);
+ current_frame.fpu_mask);
#endif
}
else
@@ -1330,7 +1330,7 @@ m68k_output_function_epilogue (FILE *str
asm_fprintf (stream, "\tfmovem %s@(-%wd),%I0x%x\n",
reg_names[FRAME_POINTER_REGNUM],
current_frame.foffset + fsize,
- current_frame.fpu_rev_mask);
+ current_frame.fpu_mask);
#endif
}
}
--cut--