[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