Re: RFA: recognize new instructions in rs6000 prologues

[Jim Blandy <jimb at redhat dot com>]

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.