diff mbox

[1/5] PowerPC: Consolidate NPTL/non versions of vfork

Message ID 537F4C7E.4020005@linux.vnet.ibm.com
State New
Headers show

Commit Message

Adhemerval Zanella May 23, 2014, 1:26 p.m. UTC 
Following Roland's NPTL consolidation, here is the powerpc one.
Checked on powerpc32, powerpc64, powerpc64le and no regressions found.

I also created the branch azanella/nptl-ppc with the 5 patches.

--

	* sysdeps/unix/sysv/linux/powerpc/powerpc32/vfork.S: Include
	<tcb-offsets.h>.
	(__vfork): Incorporate save/restore of PID from nptl/vfork.S here.
	(__libc_vfork): New strong alias.
	* nptl/sysdeps/unix/sysv/linux/powerpc/powerpc32/pt-vfork.S: File
	removed.
	* nptl/sysdeps/unix/sysv/linux/powerpc/powerpc32/vfork.S: File
	Removed.

	* sysdeps/unix/sysv/linux/powerpc/powerpc64/vfork.S: Include
	<tcb-offsets.h>.
	(__vfork): Incorporate save/retore of PID from nptl/vfork.S here.
	(__libc_vfork): New strong alias.
	* nptl/sysdeps/unix/sysv/linux/powerpc/powerpc64/pt-vfork.S: File
	removed.
	* nptl/sysdeps/unix/sysv/linux/powerpc/powerpc64/vfork.S: File
	removed.

---

Comments

Richard Henderson May 23, 2014, 6:29 p.m. UTC | #1 
On 05/23/2014 06:26 AM, Adhemerval Zanella wrote:
> +	/* Load the TCB-cached PID value and negates it. If It it is zero
> +	   sets it to 0x800000.  And then sets its value again on TCB field.
> +	   See raise.c for the logic that relies on this value.  */
> +	lwz	r0,PID(r13)
> +	cmpwi	cr0,r0,0
> +	neg	r0,r0
> +	bne-	cr0,1f
> +	lis	r0,0x8000
> +1:	stw	r0,PID(r13)

I was thinking about this when I did the alpha port:

Why don't we use ~pid instead of -pid?  That way we still get high-bit set
semantics, but don't have to special-case 0, and so don't need a branch here
(or a conditional move on other hosts)?

There is a tiny matter of updating all of the architectures all at once though...


r~
Adhemerval Zanella May 23, 2014, 6:42 p.m. UTC | #2 
On 23-05-2014 15:29, Richard Henderson wrote:
> On 05/23/2014 06:26 AM, Adhemerval Zanella wrote:
>> +	/* Load the TCB-cached PID value and negates it. If It it is zero
>> +	   sets it to 0x800000.  And then sets its value again on TCB field.
>> +	   See raise.c for the logic that relies on this value.  */
>> +	lwz	r0,PID(r13)
>> +	cmpwi	cr0,r0,0
>> +	neg	r0,r0
>> +	bne-	cr0,1f
>> +	lis	r0,0x8000
>> +1:	stw	r0,PID(r13)
> I was thinking about this when I did the alpha port:
>
> Why don't we use ~pid instead of -pid?  That way we still get high-bit set
> semantics, but don't have to special-case 0, and so don't need a branch here
> (or a conditional move on other hosts)?
>
> There is a tiny matter of updating all of the architectures all at once though...
>
>
> r~
>
I don't see why not, however I think such change would be better suited for a following
patch.  Also, do you see this code sequence as a hotspot for such optimization?
Richard Henderson May 23, 2014, 6:46 p.m. UTC | #3 
On 05/23/2014 11:42 AM, Adhemerval Zanella wrote:
> On 23-05-2014 15:29, Richard Henderson wrote:
>> On 05/23/2014 06:26 AM, Adhemerval Zanella wrote:
>>> +	/* Load the TCB-cached PID value and negates it. If It it is zero
>>> +	   sets it to 0x800000.  And then sets its value again on TCB field.
>>> +	   See raise.c for the logic that relies on this value.  */
>>> +	lwz	r0,PID(r13)
>>> +	cmpwi	cr0,r0,0
>>> +	neg	r0,r0
>>> +	bne-	cr0,1f
>>> +	lis	r0,0x8000
>>> +1:	stw	r0,PID(r13)
>> I was thinking about this when I did the alpha port:
>>
>> Why don't we use ~pid instead of -pid?  That way we still get high-bit set
>> semantics, but don't have to special-case 0, and so don't need a branch here
>> (or a conditional move on other hosts)?
>>
>> There is a tiny matter of updating all of the architectures all at once though...
>>
>>
>> r~
>>
> I don't see why not, however I think such change would be better suited for a following
> patch.  Also, do you see this code sequence as a hotspot for such optimization? 
> 

Of course a following patch, and of course it's not a hotspot.
It just seems like an oddity that could be done better.


r~
Roland McGrath May 23, 2014, 9:59 p.m. UTC | #4 
Looks right to me.
diff mbox

Patch

diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/powerpc32/pt-vfork.S b/nptl/sysdeps/unix/sysv/linux/powerpc/powerpc32/pt-vfork.S
deleted file mode 100644
index 81dbdee..0000000
--- a/nptl/sysdeps/unix/sysv/linux/powerpc/powerpc32/pt-vfork.S
+++ /dev/null
@@ -1,48 +0,0 @@ 
-/* Copyright (C) 2004-2014 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Jakub Jelinek <jakub@redhat.com>, 2004.
-
-   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 <sysdep.h>
-#define _ERRNO_H	1
-#include <bits/errno.h>
-#include <kernel-features.h>
-#include <tcb-offsets.h>
-
-/* Clone the calling process, but without copying the whole address space.
-   The calling process is suspended until the new process exits or is
-   replaced by a call to `execve'.  Return -1 for errors, 0 to the new process,
-   and the process ID of the new process to the old process.  */
-
-ENTRY (__vfork)
-	lwz	0,PID(2)
-	neg	0,0
-	stw	0,PID(2)
-
-	DO_CALL (SYS_ify (vfork))
-
-	cmpwi	1,3,0
-	beqlr-	1
-
-	lwz	0,PID(2)
-	neg	0,0
-	stw	0,PID(2)
-
-	PSEUDO_RET
-
-PSEUDO_END (__vfork)
-
-weak_alias (__vfork, vfork)
diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/powerpc32/vfork.S b/nptl/sysdeps/unix/sysv/linux/powerpc/powerpc32/vfork.S
deleted file mode 100644
index e016105..0000000
--- a/nptl/sysdeps/unix/sysv/linux/powerpc/powerpc32/vfork.S
+++ /dev/null
@@ -1,56 +0,0 @@ 
-/* Copyright (C) 2004-2014 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Jakub Jelinek <jakub@redhat.com>, 2004.
-
-   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 <sysdep.h>
-#define _ERRNO_H	1
-#include <bits/errno.h>
-#include <kernel-features.h>
-#include <tcb-offsets.h>
-
-/* Clone the calling process, but without copying the whole address space.
-   The calling process is suspended until the new process exits or is
-   replaced by a call to `execve'.  Return -1 for errors, 0 to the new process,
-   and the process ID of the new process to the old process.  */
-
-ENTRY (__vfork)
-	lwz	0,PID(2)
-	cmpwi	0,0,0
-	neg	0,0
-	bne-	0,1f
-	lis	0,0x8000
-1:	stw	0,PID(2)
-
-	DO_CALL (SYS_ify (vfork))
-
-	cmpwi	1,3,0
-	beqlr-	1
-
-	lwz	0,PID(2)
-	/* Cannot use clrlwi. here, because cr0 needs to be preserved
-	   until PSEUDO_RET.  */
-	clrlwi	4,0,1
-	cmpwi	1,4,0
-	beq-	1,1f
-	neg	4,0
-1:	stw	4,PID(2)
-
-	PSEUDO_RET
-
-PSEUDO_END (__vfork)
-libc_hidden_def (__vfork)
-weak_alias (__vfork, vfork)
diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/powerpc64/pt-vfork.S b/nptl/sysdeps/unix/sysv/linux/powerpc/powerpc64/pt-vfork.S
deleted file mode 100644
index bbf570f..0000000
--- a/nptl/sysdeps/unix/sysv/linux/powerpc/powerpc64/pt-vfork.S
+++ /dev/null
@@ -1,48 +0,0 @@ 
-/* Copyright (C) 2004-2014 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Jakub Jelinek <jakub@redhat.com>, 2004.
-
-   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 <sysdep.h>
-#define _ERRNO_H	1
-#include <bits/errno.h>
-#include <kernel-features.h>
-#include <tcb-offsets.h>
-
-/* Clone the calling process, but without copying the whole address space.
-   The calling process is suspended until the new process exits or is
-   replaced by a call to `execve'.  Return -1 for errors, 0 to the new process,
-   and the process ID of the new process to the old process.  */
-
-ENTRY (__vfork)
-	lwz	0,PID(13)
-	neg	0,0
-	stw	0,PID(13)
-
-	DO_CALL (SYS_ify (vfork))
-
-	cmpwi	1,3,0
-	beqlr-	1
-
-	lwz	0,PID(13)
-	neg	0,0
-	stw	0,PID(13)
-
-	PSEUDO_RET
-
-PSEUDO_END (__vfork)
-
-weak_alias (__vfork, vfork)
diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/powerpc64/vfork.S b/nptl/sysdeps/unix/sysv/linux/powerpc/powerpc64/vfork.S
deleted file mode 100644
index f8bf016..0000000
--- a/nptl/sysdeps/unix/sysv/linux/powerpc/powerpc64/vfork.S
+++ /dev/null
@@ -1,54 +0,0 @@ 
-/* Copyright (C) 2004-2014 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Jakub Jelinek <jakub@redhat.com>, 2004.
-
-   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 <sysdep.h>
-#define _ERRNO_H	1
-#include <bits/errno.h>
-#include <kernel-features.h>
-#include <tcb-offsets.h>
-
-/* Clone the calling process, but without copying the whole address space.
-   The calling process is suspended until the new process exits or is
-   replaced by a call to `execve'.  Return -1 for errors, 0 to the new process,
-   and the process ID of the new process to the old process.  */
-
-ENTRY (__vfork)
-	lwz	0,PID(13)
-	cmpwi	0,0,0
-	neg	0,0
-	bne-	0,1f
-	lis	0,0x8000
-1:	stw	0,PID(13)
-
-	DO_CALL (SYS_ify (vfork))
-
-	cmpwi	1,3,0
-	beqlr-	1
-
-	lwz	0,PID(13)
-	clrlwi	4,0,1
-	cmpwi	1,4,0
-	beq-	1,1f
-	neg	4,0
-1:	stw	4,PID(13)
-
-	PSEUDO_RET
-
-PSEUDO_END (__vfork)
-libc_hidden_def (__vfork)
-weak_alias (__vfork, vfork)
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/vfork.S b/sysdeps/unix/sysv/linux/powerpc/powerpc32/vfork.S
index 7a1e842..8a84336 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/vfork.S
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/vfork.S
@@ -19,6 +19,7 @@ 
 #define _ERRNO_H	1
 #include <bits/errno.h>
 #include <kernel-features.h>
+#include <tcb-offsets.h>
 
 /* Clone the calling process, but without copying the whole address space.
    The calling process is suspended until the new process exits or is
@@ -26,9 +27,39 @@ 
    and the process ID of the new process to the old process.  */
 
 ENTRY (__vfork)
+
+	/* Load the TCB-cached PID value and negates it. If It it is zero
+	   sets it to 0x800000.  And then sets its value again on TCB field.
+	   See raise.c for the logic that relies on this value.  */
+
+	lwz	r0,PID(r2)
+	cmpwi	cr0,r0,0
+	neg	r0,r0
+	bne-	cr0,1f
+	lis	r0,0x8000
+1:	stw	r0,PID(r2)
+
 	DO_CALL (SYS_ify (vfork))
+
+	cmpwi	cr1,r3,0
+	beqlr-	1
+
+	/* Restore the original value of the TCB cache of the PID, if we're
+	   the parent.  But in the child (syscall return value equals zero),
+	   leave things as they are.  */
+	lwz	r0,PID(r2)
+	/* Cannot use clrlwi. here, because cr0 needs to be preserved
+	   until PSEUDO_RET.  */
+	clrlwi	r4,r0,1
+	cmpwi	cr1,r4,0
+	beq-	cr1,1f
+	neg	r4,r0
+1:	stw	r4,PID(r2)
+
 	PSEUDO_RET
+
 PSEUDO_END (__vfork)
 libc_hidden_def (__vfork)
 
 weak_alias (__vfork, vfork)
+strong_alias (__vfork, __libc_vfork)
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/vfork.S b/sysdeps/unix/sysv/linux/powerpc/powerpc64/vfork.S
index ebffc4c..72e6da8 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/vfork.S
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/vfork.S
@@ -19,6 +19,7 @@ 
 #define _ERRNO_H	1
 #include <bits/errno.h>
 #include <kernel-features.h>
+#include <tcb-offsets.h>
 
 /* Clone the calling process, but without copying the whole address space.
    The calling process is suspended until the new process exits or is
@@ -27,9 +28,36 @@ 
 
 ENTRY (__vfork)
 	CALL_MCOUNT 0
+
+	/* Load the TCB-cached PID value and negates it. If It it is zero
+	   sets it to 0x800000.  And then sets its value again on TCB field.
+	   See raise.c for the logic that relies on this value.  */
+	lwz	r0,PID(r13)
+	cmpwi	cr0,r0,0
+	neg	r0,r0
+	bne-	cr0,1f
+	lis	r0,0x8000
+1:	stw	r0,PID(r13)
+
 	DO_CALL (SYS_ify (vfork))
+
+	cmpwi	cr1,r3,0
+	beqlr-	1
+
+	/* Restore the original value of the TCB cache of the PID, if we're
+	   the parent.  But in the child (syscall return value equals zero),
+	   leave things as they are.  */
+	lwz	r0,PID(r13)
+	clrlwi	r4,r0,1
+	cmpwi	cr1,r4,0
+	beq-	cr1,1f
+	neg	r4,r0
+1:	stw	r4,PID(r13)
+
 	PSEUDO_RET
+
 PSEUDO_END (__vfork)
 libc_hidden_def (__vfork)
 
 weak_alias (__vfork, vfork)
+strong_alias (__vfork, __libc_vfork)