[patch] Avoid "anonymous" code in pthread_spin_lock

Paul Pluzhnikov ppluzhnikov@google.com
Sat Apr 28 00:22:00 GMT 2012


On Wed, Apr 25, 2012 at 4:10 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:

> Attached patch introduces a local label, changing above picture to:
>
> (gdb) x/11i 0xcb00
>   0xcb00 <pthread_spin_lock>:  lock decl (%rdi)
>   0xcb03 <pthread_spin_lock+3>:        jne    0xcb10 <_L_pthread_spin_lock>
>   0xcb05 <pthread_spin_lock+5>:        xor    %eax,%eax
>   0xcb07 <pthread_spin_lock+7>:        retq
>   0xcb08:      nopl   0x0(%rax,%rax,1)
>   0xcb10 <_L_pthread_spin_lock>:       pause
>   0xcb12 <_L_pthread_spin_lock+2>:     cmpl   $0x0,(%rdi)
>   0xcb15 <_L_pthread_spin_lock+5>:     jg     0xcb00 <pthread_spin_lock>
>   0xcb17 <_L_pthread_spin_lock+7>:     jmp    0xcb10 <_L_pthread_spin_lock>
>   0xcb19:      nopl   0x0(%rax)
>   0xcb20 <pthread_spin_trylock>:       mov    $0x1,%eax

As Andrew pointed out, this is still quite undesirable from profiling
perspective: in a profile, we would see something like:

   8069  30.0%  30.0%     8069  30.0% SomeClass::SomeFunc
   3992  14.8%  44.9%     3992  14.8% _L_pthread_spin_lock
   3212  11.9%  56.8%     3212  11.9% syscall
   1867   6.9%  63.8%     1867   6.9% mlx4_post_send
   1446   5.4%  69.1%     1446   5.4% pthread_spin_lock

but we'd like to see pthread_spin_lock taking the 20.2% that it actually
does.

There appears to be very little value in writing pthread_spin_lock in C:
it only contains a single "return".

I therefore would like to propose attached patch, which re-implements
exact same code in straight ASM.

This is also slightly faster on i386, since we don't need frame pointer.

Tested on Linux/x86_64 and Linux/i686.

Thanks,
-- 
Paul Pluzhnikov


nptl/ChangeLog:

2012-04-27  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* sysdeps/i386/pthread_spin_lock.S: New.
	* sysdeps/i386/pthread_spin_lock.c: Delete.
	* sysdeps/x86_64/pthread_spin_lock.S: New.
	* sysdeps/x86_64/pthread_spin_lock.c: Delete.
-------------- next part --------------
diff --git a/nptl/sysdeps/i386/pthread_spin_lock.S b/nptl/sysdeps/i386/pthread_spin_lock.S
new file mode 100644
index 0000000..0b9fa4f
--- /dev/null
+++ b/nptl/sysdeps/i386/pthread_spin_lock.S
@@ -0,0 +1,38 @@
+/* Copyright (C) 2012 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Paul Pluzhnikov <ppluzhnikov@google.com>
+
+   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/>.  */
+
+#include <lowlevellock.h>
+
+	.globl	pthread_spin_lock
+	.type	pthread_spin_lock,@function
+	.align	16
+pthread_spin_lock:
+	mov	4(%esp), %eax
+1:	LOCK
+	decl	0(%eax)
+	jne	2f
+	xor	%eax, %eax
+	ret
+
+	.align	16
+2:	rep
+	nop
+	cmpl	$0, 0(%eax)
+	jg	1b
+	jmp	2b
+	.size	pthread_spin_lock,.-pthread_spin_lock
diff --git a/nptl/sysdeps/i386/pthread_spin_lock.c b/nptl/sysdeps/i386/pthread_spin_lock.c
deleted file mode 100644
index c059e01..0000000
--- a/nptl/sysdeps/i386/pthread_spin_lock.c
+++ /dev/null
@@ -1,48 +0,0 @@
-/* Copyright (C) 2002,2003,2004 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
-
-   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/>.  */
-
-#include "pthreadP.h"
-
-#ifndef LOCK_PREFIX
-# ifdef UP
-#  define LOCK_PREFIX	/* nothing */
-# else
-#  define LOCK_PREFIX	"lock;"
-# endif
-#endif
-
-
-int
-pthread_spin_lock (lock)
-     pthread_spinlock_t *lock;
-{
-  asm ("\n"
-       "1:\t" LOCK_PREFIX "decl %0\n\t"
-       "jne 2f\n\t"
-       ".subsection 1\n\t"
-       ".align 16\n"
-       "2:\trep; nop\n\t"
-       "cmpl $0, %0\n\t"
-       "jg 1b\n\t"
-       "jmp 2b\n\t"
-       ".previous"
-       : "=m" (*lock)
-       : "m" (*lock));
-
-  return 0;
-}
diff --git a/nptl/sysdeps/x86_64/pthread_spin_lock.S b/nptl/sysdeps/x86_64/pthread_spin_lock.S
new file mode 100644
index 0000000..d70f0ac
--- /dev/null
+++ b/nptl/sysdeps/x86_64/pthread_spin_lock.S
@@ -0,0 +1,37 @@
+/* Copyright (C) 2012 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Paul Pluzhnikov <ppluzhnikov@google.com>
+
+   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/>.  */
+
+#include <lowlevellock.h>
+	
+	.globl	pthread_spin_lock
+	.type	pthread_spin_lock,@function
+	.align	16
+pthread_spin_lock:
+1:	LOCK
+	decl	0(%rdi)
+	jne	2f
+	xor	%eax, %eax
+	ret
+
+	.align	16
+2:	rep
+	nop
+	cmpl	$0, 0(%rdi)
+	jg	1b
+	jmp	2b
+	.size	pthread_spin_lock,.-pthread_spin_lock
diff --git a/nptl/sysdeps/x86_64/pthread_spin_lock.c b/nptl/sysdeps/x86_64/pthread_spin_lock.c
deleted file mode 100644
index 7cf0e0e..0000000
--- a/nptl/sysdeps/x86_64/pthread_spin_lock.c
+++ /dev/null
@@ -1 +0,0 @@
-#include "../i386/pthread_spin_lock.c"


More information about the Libc-alpha mailing list