Re: PPC -fpic bug introduced recently

[Geoff Keating <geoffk at ozemail dot com dot au>]

> From: "Melissa O'Neill" <oneill@cs.sfu.ca>
> Date: Tue,  5 Oct 1999 10:24:46 -0700

> If you take a look at rs6000.h, you'll see (in the #definition of
> CONDITIONAL_REGISTER_USAGE):
>   if ((DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_SOLARIS)		\
>       && flag_pic == 1)							\
>     fixed_regs[PIC_OFFSET_TABLE_REGNUM]					\
>       = call_used_regs[PIC_OFFSET_TABLE_REGNUM] = 1;			\
> 
> This says pretty clearly that PIC_OFFSET_TABLE_REGNUM is *not* to be
> saved across calls.
> 
> Since the PPC defines PIC_OFFSET_TABLE_REGNUM to be a high numbered
> register (r30), it's clear that the intent is for this register to be
> a call-saved register.  (Someone might want to look at the tradeoffs
> involved in using r13, leaving the above code alone, and defining
> PIC_OFFSET_TABLE_REG_CALL_CLOBBERED, but my feeling is that that would
> be a worse solution).
> 
> So anyway, it appears the logical patch would be:
> 
> (CONDITIONAL_REGISTER_USAGE)    Don't make the pic offset table register
> 				a call-used register.
> --- config/rs6000/rs6000.h	Thu Aug 19 01:21:45 1999
> +++ config/rs6000/rs6000.h.new	Tue Oct  5 09:28:15 1999
> @@ -894,10 +894,9 @@
>      for (i = 32; i < 64; i++)						\
>        fixed_regs[i] = call_used_regs[i] = 1; 				\
>    if ((DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_SOLARIS)		\
>        && flag_pic == 1)							\
> -    fixed_regs[PIC_OFFSET_TABLE_REGNUM]					\
> -      = call_used_regs[PIC_OFFSET_TABLE_REGNUM] = 1;			\
> +    fixed_regs[PIC_OFFSET_TABLE_REGNUM] = 1;				\
>  }
> 
>  /* Specify the registers used for certain standard purposes.
>     The values of these macros are register numbers.  */
> 
> Assuming this is right, the code register saving code should be modified
> to be a little less zealous. (I think Ian Piumarta's test on
> fixed_regs[first_reg] was redundant, and appears to disallowed fixed
> registers that wanted to be call saved, which this case shows not to be
> desirable):
> 
> (first_reg_to_save)		Removed redundant (and wrong) test. Fixed
> 				registers *can* be call-saved registers.
> --- config/rs6000/rs6000.c	Fri Sep  3 00:54:28 1999
> +++ config/rs6000/rs6000.c.new	Tue Oct  5 09:51:33 1999
> @@ -3242,10 +3242,9 @@
> 
>    /* Find lowest numbered live register.  */
>    for (first_reg = 13 + FIXED_R13; first_reg <= 31; first_reg++)
>      if (regs_ever_live[first_reg]
> -	&& !call_used_regs[first_reg]
> -	&& !fixed_regs[first_reg])
> +	&& !call_used_regs[first_reg])
>        break;
> 
>    if (profile_flag)
>      {

This is what the code currently looks like:

  /* Find lowest numbered live register.  */
  for (first_reg = 13; first_reg <= 31; first_reg++)
    if (regs_ever_live[first_reg] && ! call_used_regs[first_reg])
      break;

I'm not sure if you're supposed to have registers which are fixed and
yet call-saved.  On the other hand, it clearly makes sense to do so.

Otherwise, we'd change the test to read

  /* Find lowest numbered live register.  */
  for (first_reg = 13; first_reg <= 31; first_reg++)
    if (regs_ever_live[first_reg] 
        && (! call_used_regs[first_reg]
	    || ((DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_SOLARIS)
	        && flag_pic == 1
		&& first_reg == PIC_OFFSET_TABLE_REGNUM)))
      break;

perhaps with the test encapsulated in some macro so we can share it
between this and CONDITIONAL_REGISTER_USE.

-- 
Geoffrey Keating <geoffk@cygnus.com>