[PATCH] adjust dwarf ra column to libexc expectations on mips-irix

[PATCH] adjust dwarf ra column to libexc expectations on mips-irix

[Olivier Hainque]

Hello,

This is a followup to http://gcc.gnu.org/ml/gcc-patches/2005-03/msg01487.html

 GCC for mips currently uses the hard return reg column (31) as the dwarf
 return address column. This badly confuses the system unwinder on IRIX.

 The patch below addresses that by overriding DWARF_FRAME_RETURN_COLUMN on
 IRIX and by adjusting 'mips_frame_set' to note an additional dwarf ra column
 change when r31 is saved and the dwarf ra column number differs.

 Bootstrapped and regression tested on mips-sgi-irix6.4 for languages=all,ada
 and mainline sources from 20050315.

2005-03-27  Olivier Hainque  <hainque@adacore.com>

	* config/mips/iris6.h (DWARF_FRAME_RETURN_COLUMN): Redefine to
	match what the system unwinder expects.
	* config/mips/mips.c (mips_frame_set): If we're saving the return
	address register and the dwarf return address column number differs
	from the hard register number, note the dwarf return address save
	too.

*** config/mips/iris6.h.orig	Mon Feb 28 10:23:38 2005
--- config/mips/iris6.h	Thu Mar 24 17:37:17 2005
*************** Boston, MA 02111-1307, USA.  */
*** 38,43 ****
--- 38,48 ----
     compiling -g.  This guarantees that we can unwind the stack.  */
  #define DWARF2_FRAME_INFO 1
  
+ /* The system unwinder in libexc requires a specific dwarf return address
+    column to work.  */
+ #undef  DWARF_FRAME_RETURN_COLUMN
+ #define DWARF_FRAME_RETURN_COLUMN (FP_REG_LAST + 1)
+ 
  #undef MACHINE_TYPE
  #define MACHINE_TYPE "SGI running IRIX 6.x"
  
*** config/mips/mips.c.orig	Wed Mar 16 03:56:42 2005
--- config/mips/mips.c	Thu Mar 24 17:35:41 2005
*************** mips_frame_set (rtx mem, rtx reg)
*** 6277,6282 ****
--- 6277,6298 ----
  {
    rtx set = gen_rtx_SET (VOIDmode, mem, reg);
    RTX_FRAME_RELATED_P (set) = 1;
+ 
+   /* If we're saving the return address register and the dwarf return
+      address column number differs from the hard register number, note
+      the dwarf return address save too.  */
+   if (REGNO (reg) == GP_REG_FIRST + 31
+       && DWARF_FRAME_RETURN_COLUMN != GP_REG_FIRST + 31)
+     {
+       rtx ra_column_set
+ 	= gen_rtx_SET (VOIDmode,
+ 		       mem,
+ 		       gen_rtx_REG (word_mode, DWARF_FRAME_RETURN_COLUMN));
+       RTX_FRAME_RELATED_P (ra_column_set) = 1;
+ 
+       set = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, set, ra_column_set));
+     }
+ 
    return set;
  }
  

Re: [PATCH] adjust dwarf ra column to libexc expectations on mips-irix

[Richard Sandiford]

Olivier Hainque <hainque@adacore.com> writes:
> + /* The system unwinder in libexc requires a specific dwarf return address
> +    column to work.  */
> + #undef  DWARF_FRAME_RETURN_COLUMN
> + #define DWARF_FRAME_RETURN_COLUMN (FP_REG_LAST + 1)
> + 

My main concern is that:

  DWARF_FRAME_REG (FP_REG_LAST + 1) == FP_REG_LAST + 1

which is a lie given the definition above.  Wouldn't it be safer/
more correct to override DWARF_FRAME_REG with something like:

  #define DWARF_FRAME_REG(REGNO) \
    ((REGNO) <= FP_REG_LAST ? (REGNO) : INVALID_REGNUM)

at the same time?  I don't know of anything in particular that
would break without this though.

Richard

Re: [PATCH] adjust dwarf ra column to libexc expectations on mips-irix

[Olivier Hainque]

Richard Sandiford wrote:
> > + #undef  DWARF_FRAME_RETURN_COLUMN
> > + #define DWARF_FRAME_RETURN_COLUMN (FP_REG_LAST + 1)
 
> My main concern is that:
> 
>   DWARF_FRAME_REG (FP_REG_LAST + 1) == FP_REG_LAST + 1
> 
> which is a lie given the definition above.  Wouldn't it be safer/
> more correct to override DWARF_FRAME_REG with something like:
> 
>   #define DWARF_FRAME_REG(REGNO) \
>     ((REGNO) <= FP_REG_LAST ? (REGNO) : INVALID_REGNUM)
> 
> at the same time?

 Possibly.  I agree with you on 

> I don't know of anything in particular that would break without this though.

 still :) 

 One point is that we already have 
 
     #define SIGNAL_UNWIND_RETURN_COLUMN (FP_REG_LAST + 1)

 I can give it a whirl.

 Would the patch be OK if it passes with that additional change ?

 Thanks for your feedback,

 Olivier

Re: [PATCH] adjust dwarf ra column to libexc expectations on mips-irix

[Richard Sandiford]

Olivier Hainque <hainque@adacore.com> writes:

> Richard Sandiford wrote:
>> > + #undef  DWARF_FRAME_RETURN_COLUMN
>> > + #define DWARF_FRAME_RETURN_COLUMN (FP_REG_LAST + 1)
>  
>> My main concern is that:
>> 
>>   DWARF_FRAME_REG (FP_REG_LAST + 1) == FP_REG_LAST + 1
>> 
>> which is a lie given the definition above.  Wouldn't it be safer/
>> more correct to override DWARF_FRAME_REG with something like:
>> 
>>   #define DWARF_FRAME_REG(REGNO) \
>>     ((REGNO) <= FP_REG_LAST ? (REGNO) : INVALID_REGNUM)
>> 
>> at the same time?
>
>  Possibly.  I agree with you on 
>
>> I don't know of anything in particular that would break without this though.
>
>  still :)

Yeah.  I was hoping rth or someone would pipe up ;)

>  Would the patch be OK if it passes with that additional change ?

Yes, both for 4.0 and mainline.

Richard

Re: [PATCH] adjust dwarf ra column to libexc expectations on mips-irix

[Richard Henderson]

On Sun, Mar 27, 2005 at 12:54:36PM +0200, Olivier Hainque wrote:
> *************** mips_frame_set (rtx mem, rtx reg)
> *** 6277,6282 ****
> --- 6277,6298 ----
>   {
>     rtx set = gen_rtx_SET (VOIDmode, mem, reg);
>     RTX_FRAME_RELATED_P (set) = 1;
> + 
> +   /* If we're saving the return address register and the dwarf return
> +      address column number differs from the hard register number, note
> +      the dwarf return address save too.  */
> +   if (REGNO (reg) == GP_REG_FIRST + 31
> +       && DWARF_FRAME_RETURN_COLUMN != GP_REG_FIRST + 31)
> +     {
> +       rtx ra_column_set
> + 	= gen_rtx_SET (VOIDmode,
> + 		       mem,
> + 		       gen_rtx_REG (word_mode, DWARF_FRAME_RETURN_COLUMN));
> +       RTX_FRAME_RELATED_P (ra_column_set) = 1;
> + 
> +       set = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, set, ra_column_set));

There's absolutely no reason to use a parallel here.  Just change
the original reg to be DWARF_FRAME_RETURN_COLUMN.


r~

Re: [PATCH] adjust dwarf ra column to libexc expectations on mips-irix

[Olivier Hainque]

Richard Henderson wrote:
> > +   /* If we're saving the return address register and the dwarf return
> > +      address column number differs from the hard register number, note
> > +      the dwarf return address save too.  */

> There's absolutely no reason to use a parallel here.  Just change
> the original reg to be DWARF_FRAME_RETURN_COLUMN.

 Fine with me. I started that way and then thought it would be good to
 preserve the r31 tracking.

 What is your take on the DWARF_FRAME_REGNUM related adjustment ?

 The initial suggestion was to redefine as

 #define DWARF_FRAME_REGNUM(REGNO) \
    ((REGNO) <= FP_REG_LAST ? (REGNO) : INVALID_REGNUM)

 This gets into a couple of

     warning: signed and unsigned type in conditional expression

 in dwarf2out.c, that I haven't looked at in detail yet.

 Both Richard Sandiford and I were not quite sure the change was absolutely
 required, so ... is it worth pursuing ?

 Thanks in advance,

 Olivier



 
 

Re: [PATCH] adjust dwarf ra column to libexc expectations on mips-irix

[Richard Henderson]

On Thu, Mar 31, 2005 at 05:36:10PM +0200, Olivier Hainque wrote:
>  What is your take on the DWARF_FRAME_REGNUM related adjustment ?
> 
>  The initial suggestion was to redefine as
> 
>  #define DWARF_FRAME_REGNUM(REGNO) \
>     ((REGNO) <= FP_REG_LAST ? (REGNO) : INVALID_REGNUM)

I don't really see a need for it.


r~

Re: [PATCH] adjust dwarf ra column to libexc expectations on mips-irix

[Olivier Hainque]

Hello,

Back on this issue, after a new testing roundtrip (ssslooow).

Richard Henderson wrote:
> On Thu, Mar 31, 2005 at 05:36:10PM +0200, Olivier Hainque wrote:
> >  What is your take on the DWARF_FRAME_REGNUM related adjustment ?
> > 
> >  The initial suggestion was to redefine as
> > 
> >  #define DWARF_FRAME_REGNUM(REGNO) \
> >     ((REGNO) <= FP_REG_LAST ? (REGNO) : INVALID_REGNUM)
> 
> I don't really see a need for it.

 The updated version below accounts for that note and the previous one
 mentioning no need for a parallel in mips_frame_set.

 Bootstrapped and regression tested fine on mips-sgi-irix6.5 against 20050315
 sources configured with

   --enable-languages=all,ada --with-gnu-as --enable-threads=posix
 
 OK ?

 Thanks in advance,
 
 Olivier

2005-03-15  Olivier Hainque  <hainque@adacore.com>

        * config/mips/iris6.h (DWARF_FRAME_RETURN_COLUMN): Redefine to
        match what the system unwinder expects.
        * config/mips/mips.c (mips_frame_set): If we're saving the return
        address register and the dwarf return address column number differs
        from the hard register number, adjust the note reg to refer to the
	former.

*** config/mips/iris6.h.ori	Tue Apr  5 03:54:58 2005
--- config/mips/iris6.h	Thu Mar 31 13:17:24 2005
*************** Boston, MA 02111-1307, USA.  */
*** 38,43 ****
--- 38,48 ----
     compiling -g.  This guarantees that we can unwind the stack.  */
  #define DWARF2_FRAME_INFO 1
  
+ /* The system unwinder in libexc requires a specific dwarf return address
+    column to work.  */
+ #undef  DWARF_FRAME_RETURN_COLUMN
+ #define DWARF_FRAME_RETURN_COLUMN (FP_REG_LAST + 1)
+ 
  #undef MACHINE_TYPE
  #define MACHINE_TYPE "SGI running IRIX 6.x"
  
*** config/mips/mips.c.ori	Tue Apr  5 03:54:40 2005
--- config/mips/mips.c	Sat Apr  2 00:54:13 2005
*************** mips_set_frame_expr (rtx frame_pattern)
*** 6275,6282 ****
  static rtx
  mips_frame_set (rtx mem, rtx reg)
  {
!   rtx set = gen_rtx_SET (VOIDmode, mem, reg);
    RTX_FRAME_RELATED_P (set) = 1;
    return set;
  }
  
--- 6275,6292 ----
  static rtx
  mips_frame_set (rtx mem, rtx reg)
  {
!   rtx set;
! 
!   /* If we're saving the return address register and the dwarf return
!      address column differs from the hard register number, adjust the
!      note reg to refer to the former.  */
!   if (REGNO (reg) == GP_REG_FIRST + 31
!       && DWARF_FRAME_RETURN_COLUMN != GP_REG_FIRST + 31)
!     reg = gen_rtx_REG (GET_MODE (reg), DWARF_FRAME_RETURN_COLUMN);
! 
!   set = gen_rtx_SET (VOIDmode, mem, reg);
    RTX_FRAME_RELATED_P (set) = 1;
+ 
    return set;
  }
  

Re: [PATCH] adjust dwarf ra column to libexc expectations on mips-irix

[Richard Sandiford]

Olivier Hainque <hainque@adacore.com> writes:
> 2005-03-15  Olivier Hainque  <hainque@adacore.com>
>
>         * config/mips/iris6.h (DWARF_FRAME_RETURN_COLUMN): Redefine to
>         match what the system unwinder expects.
>         * config/mips/mips.c (mips_frame_set): If we're saving the return
>         address register and the dwarf return address column number differs
>         from the hard register number, adjust the note reg to refer to the
> 	former.

OK for mainline and 4.0, thanks.

Richard

Re: [PATCH] adjust dwarf ra column to libexc expectations on mips-irix

[Olivier Hainque]

Richard Sandiford wrote:
> OK for mainline and 4.0, thanks.

 Most welcome. Thanks for your prompt and constructive feedback.

 Olivier