LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3] selftests/powerpc: Fixup clobbers for TM tests
@ 2019-10-29  9:53 Michael Ellerman
  2019-11-07  3:45 ` Michael Ellerman
  0 siblings, 1 reply; 2+ messages in thread
From: Michael Ellerman @ 2019-10-29  9:53 UTC (permalink / raw)
  To: linuxppc-dev

Some of our TM (Transactional Memory) tests, list "r1" (the stack
pointer) as a clobbered register.

GCC >= 9 doesn't accept this, and the build breaks:

  ptrace-tm-spd-tar.c: In function 'tm_spd_tar':
  ptrace-tm-spd-tar.c:31:2: error: listing the stack pointer register 'r1' in a clobber list is deprecated [-Werror=deprecated]
     31 |  asm __volatile__(
        |  ^~~
  ptrace-tm-spd-tar.c:31:2: note: the value of the stack pointer after an 'asm' statement must be the same as it was before the statement

We do have some fairly large inline asm blocks in these tests, and
some of them do change the value of r1. However they should all return
to C with the value in r1 restored, so I think it's legitimate to say
r1 is not clobbered.

As Segher points out, the r1 clobbers may have been added because of
the use of `or 1,1,1`, however that doesn't actually clobber r1.

Segher also points out that some of these tests do clobber LR, because
they call functions, and that is not listed in the clobbers, so add
that where appropriate.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20191014023043.2969-1-mpe@ellerman.id.au
---
 tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c | 2 +-
 tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-vsx.c | 4 ++--
 tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c     | 2 +-
 tools/testing/selftests/powerpc/ptrace/ptrace-tm-vsx.c     | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

v3: Realise I'd already sent this patch twice.
    Add LR clobber as noticed by Segher:
    https://patchwork.ozlabs.org/patch/1140456/

diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c
index 25e23e73c72e..2ecfa1158e2b 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c
@@ -73,7 +73,7 @@ void tm_spd_tar(void)
 		[sprn_texasr]"i"(SPRN_TEXASR), [tar_1]"i"(TAR_1),
 		[dscr_1]"i"(DSCR_1), [tar_2]"i"(TAR_2), [dscr_2]"i"(DSCR_2),
 		[tar_3]"i"(TAR_3), [dscr_3]"i"(DSCR_3)
-		: "memory", "r0", "r1", "r3", "r4", "r5", "r6"
+		: "memory", "r0", "r3", "r4", "r5", "r6", "lr"
 		);
 
 	/* TM failed, analyse */
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-vsx.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-vsx.c
index f603fe5a445b..6f7fb51f0809 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-vsx.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-vsx.c
@@ -74,8 +74,8 @@ void tm_spd_vsx(void)
 		"3: ;"
 		: [res] "=r" (result), [texasr] "=r" (texasr)
 		: [sprn_texasr] "i"  (SPRN_TEXASR)
-		: "memory", "r0", "r1", "r3", "r4",
-		"r7", "r8", "r9", "r10", "r11"
+		: "memory", "r0", "r3", "r4",
+		  "r7", "r8", "r9", "r10", "r11", "lr"
 		);
 
 	if (result) {
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c
index e0d37f07bdeb..46ef378a15ec 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c
@@ -62,7 +62,7 @@ void tm_tar(void)
 		[sprn_ppr]"i"(SPRN_PPR), [sprn_texasr]"i"(SPRN_TEXASR),
 		[tar_1]"i"(TAR_1), [dscr_1]"i"(DSCR_1), [tar_2]"i"(TAR_2),
 		[dscr_2]"i"(DSCR_2), [cptr1] "b" (&cptr[1])
-		: "memory", "r0", "r1", "r3", "r4", "r5", "r6"
+		: "memory", "r0", "r3", "r4", "r5", "r6"
 		);
 
 	/* TM failed, analyse */
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-vsx.c b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-vsx.c
index 8027457b97b7..70ca01234f79 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-vsx.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-vsx.c
@@ -62,8 +62,8 @@ void tm_vsx(void)
 		"3: ;"
 		: [res] "=r" (result), [texasr] "=r" (texasr)
 		: [sprn_texasr] "i"  (SPRN_TEXASR), [cptr1] "b" (&cptr[1])
-		: "memory", "r0", "r1", "r3", "r4",
-		"r7", "r8", "r9", "r10", "r11"
+		: "memory", "r0", "r3", "r4",
+		  "r7", "r8", "r9", "r10", "r11", "lr"
 		);
 
 	if (result) {
-- 
2.21.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v3] selftests/powerpc: Fixup clobbers for TM tests
  2019-10-29  9:53 [PATCH v3] selftests/powerpc: Fixup clobbers for TM tests Michael Ellerman
@ 2019-11-07  3:45 ` Michael Ellerman
  0 siblings, 0 replies; 2+ messages in thread
From: Michael Ellerman @ 2019-11-07  3:45 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev

On Tue, 2019-10-29 at 09:53:24 UTC, Michael Ellerman wrote:
> Some of our TM (Transactional Memory) tests, list "r1" (the stack
> pointer) as a clobbered register.
>
> GCC >= 9 doesn't accept this, and the build breaks:
>
>   ptrace-tm-spd-tar.c: In function 'tm_spd_tar':
>   ptrace-tm-spd-tar.c:31:2: error: listing the stack pointer register 'r1' in a clobber list is deprecated [-Werror=deprecated]
>      31 |  asm __volatile__(
>         |  ^~~
>   ptrace-tm-spd-tar.c:31:2: note: the value of the stack pointer after an 'asm' statement must be the same as it was before the statement
>
> We do have some fairly large inline asm blocks in these tests, and
> some of them do change the value of r1. However they should all return
> to C with the value in r1 restored, so I think it's legitimate to say
> r1 is not clobbered.
>
> As Segher points out, the r1 clobbers may have been added because of
> the use of `or 1,1,1`, however that doesn't actually clobber r1.
>
> Segher also points out that some of these tests do clobber LR, because
> they call functions, and that is not listed in the clobbers, so add
> that where appropriate.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Link: https://lore.kernel.org/r/20191014023043.2969-1-mpe@ellerman.id.au

Applied to powerpc next.

https://git.kernel.org/powerpc/c/a02cbc7ffe529ed58b6bbe54652104fc2c88bd77

cheers

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-29  9:53 [PATCH v3] selftests/powerpc: Fixup clobbers for TM tests Michael Ellerman
2019-11-07  3:45 ` Michael Ellerman

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git