This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi! On Fri, 4 May 2012 15:03:04 +0800, Chung-Lin Tang <cltang@codesourcery.com> wrote: > Hi Thomas, this patch adds CFI directives to epilogues in the SH port > assembly files. Thanks, generally looks ready to be applied! > Some of the cancellation point calls were also modified to now use CFI > assembler directives, instead of hard-coding the information. I already pushed that as e1b4354e663fe7f68c96b6c6e72e55492bf38b91 -- please send such clean-up patches independently, whenever easily possible. Also, please indicate if/how you have tested the changes, helping your friendly reviewer. :-) Some further comments: 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? > --- 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? > diff --git a/sysdeps/unix/sh/sysdep.S b/sysdeps/unix/sh/sysdep.S > index e816575..939eb9a 100644 > --- a/sysdeps/unix/sh/sysdep.S > +++ b/sysdeps/unix/sh/sysdep.S > @@ -41,13 +41,15 @@ skip: > sts.l pr, @-r15 > cfi_adjust_cfa_offset (4) > cfi_rel_offset (pr, 0) > + cfi_adjust_cfa_offset (4) > jsr @r1 > mov.l r0, @-r15 > - cfi_adjust_cfa_offset (4) Hmm? > diff --git a/sysdeps/unix/sysv/linux/sh/sysdep.h b/sysdeps/unix/sysv/linux/sh/sysdep.h > index 5215a84..f5e7524 100644 > --- a/sysdeps/unix/sysv/linux/sh/sysdep.h > +++ b/sysdeps/unix/sysv/linux/sh/sysdep.h > @@ -120,14 +120,13 @@ > # endif > # define SYSCALL_ERROR_HANDLER \ > neg r0,r1; \ > - mov r12,r2; \ > - mov.l 0f,r12; \ > + mov.l 0f,r2; \ > mova 0f,r0; \ > - add r0,r12; \ > + add r0,r2; \ > mov.l 1f,r0; \ > stc gbr, r4; \ > - mov.l @(r0,r12),r0; \ > - mov r2,r12; \ > + add r0,r2; \ > + mov.l @r2,r0; \ > add r4,r0; \ > mov.l r1,@r0; \ > bra .Lpseudo_end; \ > @@ -139,13 +138,12 @@ > /* Store (-r0) into errno through the GOT. */ > # define SYSCALL_ERROR_HANDLER \ > neg r0,r1; \ > - mov r12,r2; \ > - mov.l 0f,r12; \ > + mov.l 0f,r2; \ > mova 0f,r0; \ > - add r0,r12; \ > + add r0,r2; \ > mov.l 1f,r0; \ > - mov.l @(r0,r12),r0; \ > - mov r2,r12; \ > + add r0,r2; \ > + mov.l @r2,r0; \ > mov.l r1,@r0; \ > bra .Lpseudo_end; \ > mov _IMM1,r0; \ Hmm. In my understanding, r12 is the standard register to use for accessing the GOT. I think the linker is only able to optimize this exact sequences. Also, isn't there another case for RTLD_PRIVATE_ERRNO? What about using Âcfi_register (r12, r2)Â, and later Âcfi_restore (r12)Â. GrÃÃe, Thomas
Attachment:
pgp00000.pgp
Description: PGP signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |