Re: [RFA] Fix small problems in rs6000-tdep.c:skip_prologue()

[Kevin Buettner <kevinb at redhat dot com>]

On 02 Apr 2004 16:14:16 -0500
Jim Blandy <jimb@redhat.com> wrote:

> Joel Brobecker <brobecker@gnat.com> writes:
>
> > The second problem is with the following instruction: "stw r0,28(r31)".
> > The prologue analyzer tags this instruction as being part of the
> > prologue, but this is incorrect, I believe. According to the PPC
> > ABI document I have, only r3 to r10 are used for parameter passing:
> > 
> >         For PowerPC, up to eight words are passed in general purpose
> >         registers, loaded sequentially into general purpose registers r3
> >         through r10.
> > 
> > Unfortunately, skip_prologue() detects the "stw Rx, NUM(R31)" sequence,
> > but forgot to check the register number. Here, it's the scratch register
> > zero, so the instruction should not be considered part of the prologue
> > either.
> 
> Here's a prologue I saw recently where an 'stX r0, NUM(r31)' really is
> part of the prologue:
> 
>             .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
> 
> In this case, the stX 0,N(31) is spilling an argument, even though r0
> is not an argument register.  ('evstdd' is an E500 instruction that
> is definitely an argument spill.)

Do you have any idea why the compiler chose to arrange the code this way?
Why couldn't it have just done "stb 3, 8(31)" and ommitted the "mr 0, 3"
altogether?

> Clearly, both your function and mine need to go into the test suite...
> 
> What if we did something like this?  It'd need to be combined with the 
> rest of your change, I'm just sketching:
> 
> *** rs6000-tdep.c.~1.191.~	2004-03-29 16:45:15.000000000 -0500
> --- rs6000-tdep.c	2004-04-02 16:11:26.000000000 -0500
> ***************
> *** 441,446 ****
> --- 441,447 ----
>     int minimal_toc_loaded = 0;
>     int prev_insn_was_prologue_insn = 1;
>     int num_skip_non_prologue_insns = 0;
> +   int r0_contains_argument = 0;
>     const struct bfd_arch_info *arch_info = gdbarch_bfd_arch_info (current_gdbarch);
>     struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
>     
> ***************
> *** 509,519 ****
> --- 510,524 ----
>   	     ones.  */
>   	  if (lr_reg < 0)
>   	    lr_reg = (op & 0x03e00000);
> +           if (lr_reg == 0)
> +             r0_contains_argument = 0;

This special casing of r0 bothers me.  In your example above, what's to
prevent the compiler from using some other scratch register, e.g. r11 or
r12?  If it starts doing so, do we add more state variables to record this
fact?

Kevin