Re: [PATCH/RFA] SH: DWARF2 exception handling

[Joern Rennecke <joern dot rennecke at superh dot com>]

kaz Kojima wrote:
> 
> Thanks a lot for reviewing it.

It was actually not a full review - I was waiting for the description
before delving into the details.
	
> Joern Rennecke <joern.rennecke@superh.com> wrote:
> > Please describe the purpose of all the changes.
> I'd like to add "Emit code to change the current function's return
> address to RA." as a comment for sh_set_return_address (ra, temp).

TEMP should also be documented.  But that's just superficial -
the design is broken (see below), and it is made to a questionable
interface to start with - see my separate email titled
"EH_RETURN_STACKADJ_RTX / EH_RETURN_DATA_REGNO vs. EH_RETURN_HANDLER_RTX /
eh_return" that I've sent to the gcc mailing list.

> > I just happened to find this independently last week - it is also needed
> > to get valid call frame debugging information.
> 
> Hmm... I'd like to wait your patch about it.

It is just a two-liner - version 1.174 of sh.h in CVS.  I didn't think that
the patch would be of immediate interest to others when I checked it in.

I've attached it to this email now.

> >>         Don't abort even if the number is mapped to -1.
> >
> > Why?  When is that expected to be valid?
> 
> dwarf2out.c:expand_builtin_init_dwarf_reg_sizes tests the result
> of DWARF_FRAME_REGNUM, which is SH_DBX_REGISTER_NUMBER in our case,
> for all numbers from 0 to (FIRST_PSEUDO_REGISTER - 1).

Then there should be a comment to that effect.

>         * config/sh/sh-protos.h (sh_set_return_address): Declare.
>         * config/sh/sh.c (sh_expand_prologue): Push PR_REG or
>         PR_MEDIA_REG last for TARGET_SH5.
>         (calc_live_regs): Count PR and EH_RETURN_DATA_REGNO registers
>         if the current function calls EH return.
>         (sh_expand_epilogue): Pop PR_REG or PR_MEDIA_REG first for
>         TARGET_SH5. Handle EH stack adjustments.
>         (sh_set_return_address): New function.
>         * config/sh/sh.h (SH_DBX_REGISTER_NUMBER): Handle PR_MEDIA_REG.
>         Don't abort even if the number is mapped to -1.
>         (DWARF_FRAME_RETURN_COLUMN): Map with DWARF_FRAME_REGNUM.
>         (EH_RETURN_DATA_REGNO): Define.
>         (EH_RETURN_STACKADJ_RTX): Define.
>         * config/sh/sh.md (UNSPEC_EH_RETURN): New.
>         (eh_return): New pattern.
>         (eh_set_ra_di, eh_set_ra_si): Likewise.
>         Add splitter to perform EH return after reload.

This breaks SH64, since PR_MEDIA_REG is saved as 64 bit there, and
hence handled in the aligned case.
I don't see why you would need to reorder the register saves; as
you calculate the frame size anyway, you could also determine
where exactly the PR register is.

Or if we can really rely on call-clobbered registers to live from
__builtin_eh_return to the epilogue - they have to to get the values
in EH_RETURN_STACKADJ_RTX and EH_RETURN_DATA_REGNO to / beyond the
epilogue - we can just put the return address into a register -
GPR for SHcompact, target register for SHmedia - and use that
register in the epilogue.
	
-- 
--------------------------
SuperH (UK) Ltd.
2410 Aztec West / Almondsbury / BRISTOL / BS32 4QX
T:+44 1454 465658
Thu Nov  7 15:50:18 2002  J"orn Rennecke <joern.rennecke@superh.com>

	* sh.h (DWARF_FRAME_RETURN_COLUMN): Use DWARF_FRAME_REGNUM.

Index: config/sh/sh.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/sh/sh.h,v
retrieving revision 1.173
retrieving revision 1.174
diff -p -r1.173 -r1.174
*** config/sh/sh.h	4 Nov 2002 16:57:11 -0000	1.173
--- config/sh/sh.h	7 Nov 2002 15:51:27 -0000	1.174
*************** extern int rtx_equal_function_value_matt
*** 3316,3322 ****
  #define MD_CAN_REDIRECT_BRANCH(INSN, SEQ) \
    sh_can_redirect_branch ((INSN), (SEQ))
  
! #define DWARF_FRAME_RETURN_COLUMN (TARGET_SH5 ? PR_MEDIA_REG : PR_REG)
  
  #if (defined CRT_BEGIN || defined CRT_END) && ! __SHMEDIA__
  /* SH constant pool breaks the devices in crtstuff.c to control section
--- 3316,3323 ----
  #define MD_CAN_REDIRECT_BRANCH(INSN, SEQ) \
    sh_can_redirect_branch ((INSN), (SEQ))
  
! #define DWARF_FRAME_RETURN_COLUMN \
!   (TARGET_SH5 ? DWARF_FRAME_REGNUM (PR_MEDIA_REG) : DWARF_FRAME_REGNUM (PR_REG))
  
  #if (defined CRT_BEGIN || defined CRT_END) && ! __SHMEDIA__
  /* SH constant pool breaks the devices in crtstuff.c to control section