diff mbox

Remove __PTHREAD_MUTEX_HAVE_ELISION undefined warning

Message ID 5332D913.5050903@linux.vnet.ibm.com
State New
Headers show

Commit Message

Adhemerval Zanella March 26, 2014, 1:41 p.m. UTC 
This is a respin of the original attempt of remove __PTHREAD_MUTEX_HAVE_ELISION
compile warnings on arch that does not define it. Following Joseph suggestion
I changed to use a bits header.

This patch adds new installed header bits/pthread-elision.h that defines
__PTHREAD_MUTEX_HAVE_ELISION to an appropriated value depending of the
architecture support.

Tested on PPC64 (no warnings) and x86_64 (with --lock-elision=yes/no). On X86_64
I don't have a TM capable chip, however the defines by checking the -E output
is as expected. I didn't check the s390 build, however since it also does not
define __PTHREAD_MUTEX_HAVE_ELISION I expect to follow PPC64 way.

--

	* nptl/sysdeps/pthread/bits/pthread-elision.h: New header: define
	default lock elision support and defines.
	* nptl/sysdeps/pthread/pthread.h: Include pthread-elision.h.
	* nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
	(__PTHREAD_MUTEX_HAVE_ELISION): Undefine.
	* nptl/sysdeps/unix/sysv/linux/x86/bits/pthread-elision.h: New
	header: x86 specific lock elision support.
	* nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h:
	(__PTHREAD_MUTEX_HAVE_ELISION): Definition moved to specific lock
	elision header.
	* posix/Makefile: Add bits/pthread-elision.h to install.

---

Comments

Joseph Myers March 26, 2014, 2:08 p.m. UTC | #1 
On Wed, 26 Mar 2014, Adhemerval Zanella wrote:

> +#define __PTHREAD_MUTEX_HAVE_ELISION 0

I think the default file should have a comment explaining what the 
semantics of the possible values for this macro (0, 1 and 2) are.
Roland McGrath March 26, 2014, 4:30 p.m. UTC | #2 
> On Wed, 26 Mar 2014, Adhemerval Zanella wrote:
> 
> > +#define __PTHREAD_MUTEX_HAVE_ELISION 0
> 
> I think the default file should have a comment explaining what the 
> semantics of the possible values for this macro (0, 1 and 2) are.

Absolutely.  This is a general rule for any sysdeps/generic or similar file
that sets the terms of what identifiers sysdeps variants are required to
define and what their definitions mean.
Roland McGrath March 26, 2014, 4:36 p.m. UTC | #3 
> +#if !defined _PTHREAD_H

This is OK but there is no reason not to use #ifndef.

> +# error "Never include this file directly. Use <pthread.h> instead"

Two spaces between the sentences, and a period at the end of each.

(Those two also apply to the sysdeps variant files you're adding, of course.)

> +#define __PTHREAD_MUTEX_HAVE_ELISION 0

As Joseph also requested, add an explanatory comment describing the
protocol for this macro and how other sysdeps variant files might define
it.

> +#endif
> diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h
> index 1e0c5dc..39d6b91 100644
> --- a/nptl/sysdeps/pthread/pthread.h
> +++ b/nptl/sysdeps/pthread/pthread.h
> @@ -26,6 +26,7 @@
>  #include <bits/pthreadtypes.h>
>  #include <bits/setjmp.h>
>  #include <bits/wordsize.h>
> +#include <bits/pthread-elision.h>

I'd put it right after bits/pthreadtypes.h.

> --- a/posix/Makefile
> +++ b/posix/Makefile
> @@ -29,7 +29,7 @@ headers	:= sys/utsname.h sys/times.h sys/wait.h sys/types.h unistd.h	      \
>  	   bits/local_lim.h tar.h bits/utsname.h bits/confname.h	      \
>  	   bits/waitflags.h bits/waitstatus.h sys/unistd.h sched.h	      \
>  	   bits/sched.h re_comp.h wait.h bits/environments.h cpio.h	      \
> -	   sys/sysmacros.h spawn.h bits/unistd.h
> +	   sys/sysmacros.h spawn.h bits/unistd.h bits/pthread-elision.h

That belongs in nptl/Makefile.


Thanks,
Roland
diff mbox

Patch

diff --git a/nptl/sysdeps/pthread/bits/pthread-elision.h b/nptl/sysdeps/pthread/bits/pthread-elision.h
new file mode 100644
index 0000000..cc997ae
--- /dev/null
+++ b/nptl/sysdeps/pthread/bits/pthread-elision.h
@@ -0,0 +1,28 @@ 
+/* Lock elision default definitions.
+   Copyright (C) 2014 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _PTHREAD_ELISION_H
+#define _PTHREAD_ELISION_H	1
+
+#if !defined _PTHREAD_H
+# error "Never include this file directly. Use <pthread.h> instead"
+#endif
+
+#define __PTHREAD_MUTEX_HAVE_ELISION 0
+
+#endif
diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h
index 1e0c5dc..39d6b91 100644
--- a/nptl/sysdeps/pthread/pthread.h
+++ b/nptl/sysdeps/pthread/pthread.h
@@ -26,6 +26,7 @@ 
 #include <bits/pthreadtypes.h>
 #include <bits/setjmp.h>
 #include <bits/wordsize.h>
+#include <bits/pthread-elision.h>
 
 
 /* Detach state.  */
diff --git a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
index 23a1698..a361db6 100644
--- a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
+++ b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
@@ -20,8 +20,6 @@ 
 
 #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
diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthread-elision.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthread-elision.h
new file mode 100644
index 0000000..1d3fef1
--- /dev/null
+++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthread-elision.h
@@ -0,0 +1,32 @@ 
+/* x86 lock elision definitions.
+   Copyright (C) 2014 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _PTHREAD_ELISION_H
+#define _PTHREAD_ELISION_H	1
+
+#if !defined _PTHREAD_H
+# error "Never include this file directly. Use <pthread.h> instead"
+#endif
+
+#ifdef __x86_64__
+# define __PTHREAD_MUTEX_HAVE_ELISION   1
+#else
+# define __PTHREAD_MUTEX_HAVE_ELISION   2
+#endif
+
+#endif
diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
index 28e5144..a5363ec 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
+++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
@@ -105,7 +105,6 @@  typedef union
     short __elision;
     __pthread_list_t __list;
 # define __PTHREAD_MUTEX_HAVE_PREV	1
-# define __PTHREAD_MUTEX_HAVE_ELISION   1
 #else
     unsigned int __nusers;
     __extension__ union
@@ -116,7 +115,6 @@  typedef union
 	short __elision;
 # define __spins d.__espins
 # define __elision d.__elision
-# define __PTHREAD_MUTEX_HAVE_ELISION   2
       } d;
       __pthread_slist_t __list;
     };
diff --git a/posix/Makefile b/posix/Makefile
index 328c2c5..daf22cb 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -29,7 +29,7 @@  headers	:= sys/utsname.h sys/times.h sys/wait.h sys/types.h unistd.h	      \
 	   bits/local_lim.h tar.h bits/utsname.h bits/confname.h	      \
 	   bits/waitflags.h bits/waitstatus.h sys/unistd.h sched.h	      \
 	   bits/sched.h re_comp.h wait.h bits/environments.h cpio.h	      \
-	   sys/sysmacros.h spawn.h bits/unistd.h
+	   sys/sysmacros.h spawn.h bits/unistd.h bits/pthread-elision.h
 
 routines :=								      \
 	uname								      \