RFA: recognize new instructions in rs6000 prologues

RFA: recognize new instructions in rs6000 prologues

[Jim Blandy]

These prologues are generated by a not-yet-released compiler, but the
test suite does catch the problem.

2004-02-25  Jim Blandy  <jimb@redhat.com>

	* rs6000-tdep.c (skip_prologue): Recognize moves from argument
	registers to temp register r0 and byte stores as prologue
	instructions.

*** gdb/rs6000-tdep.c.~2~	2004-02-25 15:14:13.000000000 -0500
--- gdb/rs6000-tdep.c	2004-02-25 15:15:43.000000000 -0500
***************
*** 772,777 ****
--- 772,785 ----
  
  	  /* store parameters in stack */
  	}
+       /* Move parameters from argument registers to temporary register.  */
+       else if ((op & 0xfc0007fe) == 0x7c000378 &&	/* mr(.)  Rx,Ry */
+                (((op >> 21) & 31) >= 3) &&              /* R3 >= Ry >= R10 */
+                (((op >> 21) & 31) <= 10) &&
+                (((op >> 16) & 31) == 0)) /* Rx: scratch register r0 */
+         {
+           continue;
+         }
        else if ((op & 0xfc1f0003) == 0xf8010000 ||	/* std rx,NUM(r1) */
  	       (op & 0xfc1f0000) == 0xd8010000 ||	/* stfd Rx,NUM(r1) */
  	       (op & 0xfc1f0000) == 0xfc010000)		/* frsp, fp?,NUM(r1) */
***************
*** 781,790 ****
  	  /* store parameters in stack via frame pointer */
  	}
        else if (framep &&
! 	       ((op & 0xfc1f0000) == 0x901f0000 ||	/* st rx,NUM(r1) */
! 		(op & 0xfc1f0000) == 0xd81f0000 ||	/* stfd Rx,NUM(r1) */
! 		(op & 0xfc1f0000) == 0xfc1f0000))
! 	{			/* frsp, fp?,NUM(r1) */
  	  continue;
  
  	  /* Set up frame pointer */
--- 789,799 ----
  	  /* store parameters in stack via frame pointer */
  	}
        else if (framep &&
! 	       ((op & 0xfc1f0000) == 0x901f0000 ||     /* st rx,NUM(r31) */
!                 (op & 0xfc1f0000) == 0x981f0000 ||     /* stb Rx,NUM(r31) */
! 		(op & 0xfc1f0000) == 0xd81f0000 ||     /* stfd Rx,NUM(r31) */
! 		(op & 0xfc1f0000) == 0xfc1f0000))      /* frsp, fp?,NUM(r31) */
!         {
  	  continue;
  
  	  /* Set up frame pointer */

Re: RFA: recognize new instructions in rs6000 prologues

[Kevin Buettner]

On 24 Mar 2004 10:10:04 -0500
Jim Blandy <jimb@redhat.com> wrote:

> 
> These prologues are generated by a not-yet-released compiler, but the
> test suite does catch the problem.
> 
> 2004-02-25  Jim Blandy  <jimb@redhat.com>
> 
> 	* rs6000-tdep.c (skip_prologue): Recognize moves from argument
> 	registers to temp register r0 and byte stores as prologue
> 	instructions.
> 
> *** gdb/rs6000-tdep.c.~2~	2004-02-25 15:14:13.000000000 -0500
> --- gdb/rs6000-tdep.c	2004-02-25 15:15:43.000000000 -0500
> ***************
> *** 772,777 ****
> --- 772,785 ----
>   
>   	  /* store parameters in stack */
>   	}
> +       /* Move parameters from argument registers to temporary register.  */
> +       else if ((op & 0xfc0007fe) == 0x7c000378 &&	/* mr(.)  Rx,Ry */
> +                (((op >> 21) & 31) >= 3) &&              /* R3 >= Ry >= R10 */
> +                (((op >> 21) & 31) <= 10) &&
> +                (((op >> 16) & 31) == 0)) /* Rx: scratch register r0 */
> +         {
> +           continue;
> +         }

Is this case really needed?  I would have thought that the catchall
case at the end would handle this situation.  I'm concerned that
adding this case may cause us to overshoot the prologue in some
circumstances.  (Of course, there's a danger of doing that anyway...)

>         else if ((op & 0xfc1f0003) == 0xf8010000 ||	/* std rx,NUM(r1) */
>   	       (op & 0xfc1f0000) == 0xd8010000 ||	/* stfd Rx,NUM(r1) */
>   	       (op & 0xfc1f0000) == 0xfc010000)		/* frsp, fp?,NUM(r1) */
> ***************
> *** 781,790 ****
>   	  /* store parameters in stack via frame pointer */
>   	}
>         else if (framep &&
> ! 	       ((op & 0xfc1f0000) == 0x901f0000 ||	/* st rx,NUM(r1) */
> ! 		(op & 0xfc1f0000) == 0xd81f0000 ||	/* stfd Rx,NUM(r1) */
> ! 		(op & 0xfc1f0000) == 0xfc1f0000))
> ! 	{			/* frsp, fp?,NUM(r1) */
>   	  continue;
>   
>   	  /* Set up frame pointer */
> --- 789,799 ----
>   	  /* store parameters in stack via frame pointer */
>   	}
>         else if (framep &&
> ! 	       ((op & 0xfc1f0000) == 0x901f0000 ||     /* st rx,NUM(r31) */
> !                 (op & 0xfc1f0000) == 0x981f0000 ||     /* stb Rx,NUM(r31) */
> ! 		(op & 0xfc1f0000) == 0xd81f0000 ||     /* stfd Rx,NUM(r31) */
> ! 		(op & 0xfc1f0000) == 0xfc1f0000))      /* frsp, fp?,NUM(r31) */
> !         {
>   	  continue;
>   
>   	  /* Set up frame pointer */

This part is okay.

Kevin

Re: RFA: recognize new instructions in rs6000 prologues

[Jim Blandy]

Kevin Buettner <kevinb@redhat.com> writes:
> On 24 Mar 2004 10:10:04 -0500
> Jim Blandy <jimb@redhat.com> wrote:
> 
> > 
> > These prologues are generated by a not-yet-released compiler, but the
> > test suite does catch the problem.
> > 
> > 2004-02-25  Jim Blandy  <jimb@redhat.com>
> > 
> > 	* rs6000-tdep.c (skip_prologue): Recognize moves from argument
> > 	registers to temp register r0 and byte stores as prologue
> > 	instructions.
> > 
> > *** gdb/rs6000-tdep.c.~2~	2004-02-25 15:14:13.000000000 -0500
> > --- gdb/rs6000-tdep.c	2004-02-25 15:15:43.000000000 -0500
> > ***************
> > *** 772,777 ****
> > --- 772,785 ----
> >   
> >   	  /* store parameters in stack */
> >   	}
> > +       /* Move parameters from argument registers to temporary register.  */
> > +       else if ((op & 0xfc0007fe) == 0x7c000378 &&	/* mr(.)  Rx,Ry */
> > +                (((op >> 21) & 31) >= 3) &&              /* R3 >= Ry >= R10 */
> > +                (((op >> 21) & 31) <= 10) &&
> > +                (((op >> 16) & 31) == 0)) /* Rx: scratch register r0 */
> > +         {
> > +           continue;
> > +         }
> 
> Is this case really needed?  I would have thought that the catchall
> case at the end would handle this situation.  I'm concerned that
> adding this case may cause us to overshoot the prologue in some
> circumstances.  (Of course, there's a danger of doing that anyway...)

Here's the code for the function in question:

	.align 2
	.globl arg_passing_test2
	.type	arg_passing_test2, @function
arg_passing_test2:
.LFB107:
	.loc 1 62 0
	stwu 1,-64(1)
.LCFI11:
	stw 31,60(1)
.LCFI12:
	mr 31,1
.LCFI13:
	mr 0,3
	evstdd 4,16(31)
	stw 5,24(31)
	stw 7,32(31)
	stw 8,36(31)
	stw 9,40(31)
	stb 0,8(31)
	lwz 11,0(1)
	lwz 31,-4(11)
	mr 1,11
	blr
.LFE107:
	.size	arg_passing_test2, .-arg_passing_test2

The first 'lwz' is the first non-prologue instruction; the stores
above it are argument spills.

You're referring to the catchall case that says that we scan past
unfamiliar, non-branch instructions unless the frame is already set
up, right?  This function leaves its return address in LR, so there's
no need to wait for a return address save instruction.  And the frame
pointer is set up by the time we see the 'mr 0,3' instruction.  So
that catch-all case doesn't apply here.

According to the ABI, upon function entry, r0 is caller-saves, and not
used for passing arguments, so its dead; a move into r0 can't destroy
information.  Moving an argument register into it could at worst be an
initialization of a variable that would be done sooner than expected.

But missing those spills is a serious problem.  Most users don't
consider their frame 'set up' if GDB displays the arguments as
garbage.  :)

(Of course, the best solution would be for GCC to emit location lists.
Then GDB wouldn't need to care whether arguments were spilled or not:
wherever the program was stopped, it would be able to find them.)


> >         else if ((op & 0xfc1f0003) == 0xf8010000 ||	/* std rx,NUM(r1) */
> >   	       (op & 0xfc1f0000) == 0xd8010000 ||	/* stfd Rx,NUM(r1) */
> >   	       (op & 0xfc1f0000) == 0xfc010000)		/* frsp, fp?,NUM(r1) */
> > ***************
> > *** 781,790 ****
> >   	  /* store parameters in stack via frame pointer */
> >   	}
> >         else if (framep &&
> > ! 	       ((op & 0xfc1f0000) == 0x901f0000 ||	/* st rx,NUM(r1) */
> > ! 		(op & 0xfc1f0000) == 0xd81f0000 ||	/* stfd Rx,NUM(r1) */
> > ! 		(op & 0xfc1f0000) == 0xfc1f0000))
> > ! 	{			/* frsp, fp?,NUM(r1) */
> >   	  continue;
> >   
> >   	  /* Set up frame pointer */
> > --- 789,799 ----
> >   	  /* store parameters in stack via frame pointer */
> >   	}
> >         else if (framep &&
> > ! 	       ((op & 0xfc1f0000) == 0x901f0000 ||     /* st rx,NUM(r31) */
> > !                 (op & 0xfc1f0000) == 0x981f0000 ||     /* stb Rx,NUM(r31) */
> > ! 		(op & 0xfc1f0000) == 0xd81f0000 ||     /* stfd Rx,NUM(r31) */
> > ! 		(op & 0xfc1f0000) == 0xfc1f0000))      /* frsp, fp?,NUM(r31) */
> > !         {
> >   	  continue;
> >   
> >   	  /* Set up frame pointer */
> 
> This part is okay.

Great, thanks.

Re: RFA: recognize new instructions in rs6000 prologues

[Kevin Buettner]

On 26 Mar 2004 11:56:14 -0500
Jim Blandy <jimb@redhat.com> wrote:

> Kevin Buettner <kevinb@redhat.com> writes:
> > On 24 Mar 2004 10:10:04 -0500
> > Jim Blandy <jimb@redhat.com> wrote:
> > 
> > > 
> > > These prologues are generated by a not-yet-released compiler, but the
> > > test suite does catch the problem.
> > > 
> > > 2004-02-25  Jim Blandy  <jimb@redhat.com>
> > > 
> > > 	* rs6000-tdep.c (skip_prologue): Recognize moves from argument
> > > 	registers to temp register r0 and byte stores as prologue
> > > 	instructions.
> > > 
> > > *** gdb/rs6000-tdep.c.~2~	2004-02-25 15:14:13.000000000 -0500
> > > --- gdb/rs6000-tdep.c	2004-02-25 15:15:43.000000000 -0500
> > > ***************
> > > *** 772,777 ****
> > > --- 772,785 ----
> > >   
> > >   	  /* store parameters in stack */
> > >   	}
> > > +       /* Move parameters from argument registers to temporary register.  */
> > > +       else if ((op & 0xfc0007fe) == 0x7c000378 &&	/* mr(.)  Rx,Ry */
> > > +                (((op >> 21) & 31) >= 3) &&              /* R3 >= Ry >= R10 */
> > > +                (((op >> 21) & 31) <= 10) &&
> > > +                (((op >> 16) & 31) == 0)) /* Rx: scratch register r0 */
> > > +         {
> > > +           continue;
> > > +         }
> > 
> > Is this case really needed?  I would have thought that the catchall
> > case at the end would handle this situation.  I'm concerned that
> > adding this case may cause us to overshoot the prologue in some
> > circumstances.  (Of course, there's a danger of doing that anyway...)
> 
> Here's the code for the function in question:
> 
> 	.align 2
> 	.globl arg_passing_test2
> 	.type	arg_passing_test2, @function
> arg_passing_test2:
> .LFB107:
> 	.loc 1 62 0
> 	stwu 1,-64(1)
> .LCFI11:
> 	stw 31,60(1)
> .LCFI12:
> 	mr 31,1
> .LCFI13:
> 	mr 0,3
> 	evstdd 4,16(31)
> 	stw 5,24(31)
> 	stw 7,32(31)
> 	stw 8,36(31)
> 	stw 9,40(31)
> 	stb 0,8(31)
> 	lwz 11,0(1)
> 	lwz 31,-4(11)
> 	mr 1,11
> 	blr
> .LFE107:
> 	.size	arg_passing_test2, .-arg_passing_test2
> 
> The first 'lwz' is the first non-prologue instruction; the stores
> above it are argument spills.
> 
> You're referring to the catchall case that says that we scan past
> unfamiliar, non-branch instructions unless the frame is already set
> up, right?  This function leaves its return address in LR, so there's
> no need to wait for a return address save instruction.  And the frame
> pointer is set up by the time we see the 'mr 0,3' instruction.  So
> that catch-all case doesn't apply here.
> 
> According to the ABI, upon function entry, r0 is caller-saves, and not
> used for passing arguments, so its dead; a move into r0 can't destroy
> information.  Moving an argument register into it could at worst be an
> initialization of a variable that would be done sooner than expected.
> 
> But missing those spills is a serious problem.  Most users don't
> consider their frame 'set up' if GDB displays the arguments as
> garbage.  :)

Okay, I'm convinced.  This part is approved too.

Kevin

Re: RFA: recognize new instructions in rs6000 prologues

[Jim Blandy]

Kevin Buettner <kevinb@redhat.com> writes:
> Okay, I'm convinced.  This part is approved too.

Thanks very much.  Committed.