Message ID | 532C872D.8030107@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On 21-03-2014 15:38, Adhemerval Zanella wrote: > This patch cleanups the compiler warnings on powerpc* builds. > > -- > > * nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h > (__PTHREAD_MUTEX_HAVE_ELISION): Define it to 0. > > --- > > diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h > index 71bd3ae..ac7351f 100644 > --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h > +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h > @@ -21,6 +21,8 @@ > > #include <bits/wordsize.h> > > +#define __PTHREAD_MUTEX_HAVE_ELISION 0 > + > #if __WORDSIZE == 64 > # define __SIZEOF_PTHREAD_ATTR_T 56 > # define __SIZEOF_PTHREAD_MUTEX_T 40 > Pushed upstream as 449282f2e0e850c29f6a9666058503d4734964f0
Adhemerval Zanella <azanella@linux.vnet.ibm.com> writes: > On 21-03-2014 15:38, Adhemerval Zanella wrote: >> This patch cleanups the compiler warnings on powerpc* builds. >> >> -- >> >> * nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >> (__PTHREAD_MUTEX_HAVE_ELISION): Define it to 0. >> >> --- >> >> diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >> index 71bd3ae..ac7351f 100644 >> --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >> +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >> @@ -21,6 +21,8 @@ >> >> #include <bits/wordsize.h> >> >> +#define __PTHREAD_MUTEX_HAVE_ELISION 0 >> + >> #if __WORDSIZE == 64 >> # define __SIZEOF_PTHREAD_ATTR_T 56 >> # define __SIZEOF_PTHREAD_MUTEX_T 40 >> > Pushed upstream as 449282f2e0e850c29f6a9666058503d4734964f0 That should be made archtecture independent, since it is an x86/s390-only macro. It doesn't make sence to force everyone to define it. Andreas.
On 25-03-2014 12:37, Andreas Schwab wrote: > Adhemerval Zanella <azanella@linux.vnet.ibm.com> writes: > >> On 21-03-2014 15:38, Adhemerval Zanella wrote: >>> This patch cleanups the compiler warnings on powerpc* builds. >>> >>> -- >>> >>> * nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >>> (__PTHREAD_MUTEX_HAVE_ELISION): Define it to 0. >>> >>> --- >>> >>> diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >>> index 71bd3ae..ac7351f 100644 >>> --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >>> +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >>> @@ -21,6 +21,8 @@ >>> >>> #include <bits/wordsize.h> >>> >>> +#define __PTHREAD_MUTEX_HAVE_ELISION 0 >>> + >>> #if __WORDSIZE == 64 >>> # define __SIZEOF_PTHREAD_ATTR_T 56 >>> # define __SIZEOF_PTHREAD_MUTEX_T 40 >>> >> Pushed upstream as 449282f2e0e850c29f6a9666058503d4734964f0 > That should be made archtecture independent, since it is an > x86/s390-only macro. It doesn't make sence to force everyone to define > it. > > Andreas. > I agree with you, but the patch to check if it is defined in nptl/sysdeps/pthread/pthread.h was rejected: https://sourceware.org/ml/libc-alpha/2014-03/msg00494.html. So I just folowed the way x86_64 and s390 does. If this was just an warning I'd rework it to make it more general, as Roland as suggested in https://sourceware.org/ml/libc-alpha/2014-03/msg00501.html; but it is breaking 'make check' build. So my preference now is let it as and focus on a proper fix, if any, after.
On Tue, 25 Mar 2014, Adhemerval Zanella wrote: > I agree with you, but the patch to check if it is defined in > nptl/sysdeps/pthread/pthread.h was rejected: > https://sourceware.org/ml/libc-alpha/2014-03/msg00494.html. So I just > folowed the way x86_64 and s390 does. If this was just an warning I'd > rework it to make it more general, as Roland as suggested in > https://sourceware.org/ml/libc-alpha/2014-03/msg00501.html; but it is > breaking 'make check' build. So my preference now is let it as and focus > on a proper fix, if any, after. I suggest an installed header bits/pthread-elision.h that defines __PTHREAD_MUTEX_HAVE_ELISION to an appropriate value. That avoids every architecture without elision needing to define this value in separate bits/pthreadtypes.h files. I also strongly advise *not* applying any architecture-specific fixes for -Wundef warnings that appear not to be architecture-specific without: (a) working out the correct general solution; (b) if not fixing all architectures, listing the issue and unfixed architectures on <https://sourceware.org/glibc/wiki/PortStatus>. We've made a lot of progress on avoiding architectures diverging from each other. Instead of ad hoc fixes for particular architectures, follow Siddhesh's example for fixing bits/mathdef.h (where he fixed all affected versions of the header). By all means just fix issues where the issue and fix appear genuinely architecture-specific, rather than relating to a header for which many architectures have their own versions all of which would need fixing similarly. But for architecture-independent issues like this, it's necessary to take the extra steps to avoid divergence and ensure agreement over the right fix for all architectures.
On 25-03-2014 13:19, Joseph S. Myers wrote: > On Tue, 25 Mar 2014, Adhemerval Zanella wrote: > >> I agree with you, but the patch to check if it is defined in >> nptl/sysdeps/pthread/pthread.h was rejected: >> https://sourceware.org/ml/libc-alpha/2014-03/msg00494.html. So I just >> folowed the way x86_64 and s390 does. If this was just an warning I'd >> rework it to make it more general, as Roland as suggested in >> https://sourceware.org/ml/libc-alpha/2014-03/msg00501.html; but it is >> breaking 'make check' build. So my preference now is let it as and focus >> on a proper fix, if any, after. > I suggest an installed header bits/pthread-elision.h that defines > __PTHREAD_MUTEX_HAVE_ELISION to an appropriate value. That avoids every > architecture without elision needing to define this value in separate > bits/pthreadtypes.h files. > > I also strongly advise *not* applying any architecture-specific fixes for > -Wundef warnings that appear not to be architecture-specific without: > > (a) working out the correct general solution; > > (b) if not fixing all architectures, listing the issue and unfixed > architectures on <https://sourceware.org/glibc/wiki/PortStatus>. > > We've made a lot of progress on avoiding architectures diverging from each > other. Instead of ad hoc fixes for particular architectures, follow > Siddhesh's example for fixing bits/mathdef.h (where he fixed all affected > versions of the header). > > By all means just fix issues where the issue and fix appear genuinely > architecture-specific, rather than relating to a header for which many > architectures have their own versions all of which would need fixing > similarly. But for architecture-independent issues like this, it's > necessary to take the extra steps to avoid divergence and ensure agreement > over the right fix for all architectures. > Fair enough, patch reverted.
diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h index 71bd3ae..ac7351f 100644 --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h @@ -21,6 +21,8 @@ #include <bits/wordsize.h> +#define __PTHREAD_MUTEX_HAVE_ELISION 0 + #if __WORDSIZE == 64 # define __SIZEOF_PTHREAD_ATTR_T 56 # define __SIZEOF_PTHREAD_MUTEX_T 40