PowerPC: Define __PTHREAD_MUTEX_HAVE_ELISION to 0
diff mbox

Message ID 532C872D.8030107@linux.vnet.ibm.com
State New
Headers show

Commit Message

Adhemerval Zanella March 21, 2014, 6:38 p.m. UTC
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.

---

Comments

Adhemerval Zanella March 25, 2014, 1:59 p.m. UTC | #1
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
Andreas Schwab March 25, 2014, 3:37 p.m. UTC | #2
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.
Adhemerval Zanella March 25, 2014, 4:03 p.m. UTC | #3
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.
Joseph Myers March 25, 2014, 4:19 p.m. UTC | #4
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.
Adhemerval Zanella March 25, 2014, 4:37 p.m. UTC | #5
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.

Patch
diff mbox

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