Re: [PATCH][SH] CFI directives patch

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Fri, 11 May 2012 17:15:52 +0800, Chung-Lin Tang <chunglin.tang@gmail.com> wrote:
> > I don't really feel strongly about this, but would it make sense to move
> > the cfi_remember_state instructions to the place just before the control
> > flow (potentially) branches?  It does not make any functional difference,
> > but might make things more clear conceptually.  Or, instead push it even
> > further forward, right to the place where the last CFI update is done,
> > like I had done in
> > <http://thread.gmane.org/gmane.comp.lib.glibc.ports/514/focus=518>, for
> > example?
> 
> IMHO, I just thought it more clear to mark the epilogue-in-the-middle
> like a push/pop. In that sense, the cfi_remember_state should be kept
> right before where the later thrown away CFI state starts...

Hmm, I'm not yet totally convinced about that concept (but committed your
code as-is, apart from the one case where that didn't work out).


> >> --- a/nptl/sysdeps/unix/sysv/linux/sh/sysdep-cancel.h
> >> +++ b/nptl/sysdeps/unix/sysv/linux/sh/sysdep-cancel.h
> >> @@ -71,6 +71,7 @@
> >>      CDISABLE; \
> >>      mov.l @r15+,r0; \
> >>      cfi_adjust_cfa_offset (-4); \
> >> +    cfi_restore (r0); \
> >>      lds.l @r15+,pr; \
> >>      cfi_adjust_cfa_offset (-4); \
> >>      cfi_restore (pr); \
> >> @@ -97,10 +98,10 @@
> >>  # define SAVE_ARGS_6	SAVE_ARGS_5
> >>  
> >>  # define LOAD_ARGS_0	/* Nothing.  */
> >> -# define LOAD_ARGS_1	LOAD_ARGS_0; mov.l @(0,r15),r4
> >> -# define LOAD_ARGS_2	LOAD_ARGS_1; mov.l @(4,r15),r5
> >> -# define LOAD_ARGS_3	LOAD_ARGS_2; mov.l @(8,r15),r6
> >> -# define LOAD_ARGS_4	LOAD_ARGS_3; mov.l @(12,r15),r7
> >> +# define LOAD_ARGS_1	LOAD_ARGS_0; mov.l @(0,r15),r4; cfi_restore (r4)
> >> +# define LOAD_ARGS_2	LOAD_ARGS_1; mov.l @(4,r15),r5; cfi_restore (r5)
> >> +# define LOAD_ARGS_3	LOAD_ARGS_2; mov.l @(8,r15),r6; cfi_restore (r6)
> >> +# define LOAD_ARGS_4	LOAD_ARGS_3; mov.l @(12,r15),r7; cfi_restore (r7)
> >>  # define LOAD_ARGS_5	LOAD_ARGS_4
> >>  # define LOAD_ARGS_6	LOAD_ARGS_5
> > 
> > Why do we need to care about these caller-save registers here?  You are
> > right that the current code is assymetric in that, for example,
> > Âcfi_rel_offset (r0, 0)Â is used but not Âcfi_restore (r0)Â -- but what
> > is the reason for doing the former in the first place (and likewise for
> > the others occurences/registers)?  And if it's not needed to preserve
> > these registers' content, should rather the Âcfi_rel_offsetÂs be removed?
> 
> I think it has something to do with unwinding across cancellable system
> calls, similar to what other architectures have (I see MIPS with similar
> code).

(But the MIPS code seems to be missing the cfi_restore instructions?  I
do see them in the i386 and m68k code, for example.)

> I cannot seem to google up a really definitive explanation on why this
> point of caller-saves has to be restored though.

It certainly does make sense to use cfi_restore when a respective "save"
operation has been used before, so I committed that change, but I'm still
interested in figuring out the general concept here.


GrÃÃe,
 Thomas

Attachment: pgp00000.pgp
Description: PGP signature