[PATCH] PPC enable Altivec for setjmp/longjmp part 1 of 5
Steve Munroe
sjmunroe@us.ibm.com
Fri Feb 13 23:17:00 GMT 2004
Ulrich drepper wrote:
> Steve Munroe wrote:
>
>> -typedef long int __jmp_buf[58];
>> +/* The alignment is not essential, i.e.the buffer can be copied to a 4
byte aligned buffer as per the ABI it is just added for performance
reasons. */
>> +typedef long int __jmp_buf[64+(12*4)] __attribute__((aligned(16)));
>> # endif
>> #endif
>
>Add some newline to the comment, it's far too wide.
Done. Will update the patch
>> +extern void __vmx__longjmp (__jmp_buf __env, int __val);
>> +extern void __vmx__libc_longjmp (sigjmp_buf env, int val)
>> + __attribute__ ((noreturn));
>> +libc_hidden_proto (__vmx__libc_longjmp)
>
>Why does __vmx_longjmp not have the noreturn attribute?
An oversite. Will update the patch.
>> diff -urN libc23-cvstip-20040210/sysdeps/powerpc/novmxsetjmp.h
>>libc23/sysdeps/powerpc/novmxsetjmp.h
>
>This is an internal header (I thought this was clear after Roland's
>comments) but still you use
>
>> +#ifndef _SETJMP_H
>> +#define _SETJMP_H 1
>> +
>> +#include <features.h>
>> +
>> +__BEGIN_DECLS
>
>...all this. None should be needed, maybe the include protection, but
>then the sources aren't cleanly structured.
Thought I needed features.h to get the __THROW definitions but I can use
sys/cdefs.h. I missed the __BEGIN_DECLS in my last pass. Gone now. The
include protection is just habit and forgot to change the name. How about
I change this to:
+#ifndef __NOVMX_SETJMP_H
+#define __NOVMX_SETJMP_H 1
+
+#include <sys/cdefs.h>
+#include <bits/wordsize.h>
+
>> +#if defined __USE_MISC || defined _ASM
>
> What does __USE_* have to do in an internal header? It's always
> defined. You can keep the _ASM if it is really used. But is it?
An oversite. Will update the patch.
> > +/* Calling environment, plus possibly a saved signal mask. */
> > +typedef struct __jmp_buf_tag /* C++ doesn't like
tagless structs. */
> > + {
> > + /* NOTE: The machine-dependent definitions of `__sigsetjmp'
> > + assume that a `jmp_buf' begins with a `__jmp_buf' and that
> > + `__mask_was_saved' follows it. Do not move these members
> > + or add others before it. */
> > + __jmp_buf __jmpbuf; /* Calling
environment. */
> > + int __mask_was_saved; /* Saved the signal mask? */
> > + __sigset_t __saved_mask; /* Saved signal mask. */
> > + } jmp_buf[1];
>
> How does this work together with the external definition in <setjmp.h>?
NOVMX needes its own definition as the offset to __mask_was_saved and
__saved_mask will differ because the size of __jmpbuf changes. Is it
clearer if I change the name?
+typedef struct __novmx__jmp_buf_tag
+ {
+ ...
+ } __novmx__jmp_buf[1];
> Plus: the C++ comment doesn't apply, glibc is written in C.
An oversite. Will update the patch.
> I am insistent on this because none of these setjmp changes must be
> visible to the outside. Nobody must even think about using the internal
> headers outside libc.
I trying to meet you requirements. Thats why I included the comment:
+/* copied from setjmp/setjmp.h, powerpc/bits/setjmp.h and modified
+ appropriately to keep backward compatible with setjmp without
+ AltiVec/VMX support. This file is not exported and the interfaces
+ are private to libc. */
What can I do to make this clearer?
Steven J. Munroe
Power Linux Toolchain Architect
IBM Corporation, Linux Technology Center
More information about the Libc-alpha
mailing list