qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] tests/tcg/s390x: Cleanup of mie3 tests.
@ 2022-03-01 21:43 David Miller
  2022-03-01 22:18 ` Richard Henderson
  2022-03-07  8:42 ` Thomas Huth
  0 siblings, 2 replies; 4+ messages in thread
From: David Miller @ 2022-03-01 21:43 UTC (permalink / raw)
  To: qemu-s390x, qemu-devel
  Cc: thuth, david, cohuck, richard.henderson, farman, David Miller,
	pasic, borntraeger

Adds clobbers and merges remaining separate asm statements.

v2 -> v3:
* Removed all direct memory references in mie3-sel.c

v1 -> v2:
* Corrected side in rebase conflict, removing older code.

Signed-off-by: David Miller <dmiller423@gmail.com>
---
 tests/tcg/s390x/mie3-compl.c | 18 ++++++++++++-----
 tests/tcg/s390x/mie3-mvcrl.c | 12 ++++++++----
 tests/tcg/s390x/mie3-sel.c   | 38 ++++++++++++++++--------------------
 3 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/tests/tcg/s390x/mie3-compl.c b/tests/tcg/s390x/mie3-compl.c
index 35649f3b02..938938df9e 100644
--- a/tests/tcg/s390x/mie3-compl.c
+++ b/tests/tcg/s390x/mie3-compl.c
@@ -1,13 +1,20 @@
 #include <stdint.h>
 
+
 #define FbinOp(S, ASM) uint64_t S(uint64_t a, uint64_t b) \
-{ \
-    uint64_t res = 0; \
-    asm ("llihf %[res],801\n" ASM \
-         : [res]"=&r"(res) : [a]"r"(a), [b]"r"(b) : "cc"); \
-    return res; \
+{                       \
+    uint64_t res = 0;   \
+asm volatile (          \
+    "llihf %[res],801\n"\
+    ASM                 \
+    : [res] "=&r" (res)  \
+    : [a] "r" (a)       \
+    , [b] "r" (b)       \
+);                      \
+    return res;         \
 }
 
+
 /* AND WITH COMPLEMENT */
 FbinOp(_ncrk,  ".insn rrf, 0xB9F50000, %[res], %[b], %[a], 0\n")
 FbinOp(_ncgrk, ".insn rrf, 0xB9E50000, %[res], %[b], %[a], 0\n")
@@ -28,6 +35,7 @@ FbinOp(_nogrk, ".insn rrf, 0xB9660000, %[res], %[b], %[a], 0\n")
 FbinOp(_ocrk,  ".insn rrf, 0xB9750000, %[res], %[b], %[a], 0\n")
 FbinOp(_ocgrk, ".insn rrf, 0xB9650000, %[res], %[b], %[a], 0\n")
 
+
 int main(int argc, char *argv[])
 {
     if (_ncrk(0xFF88, 0xAA11)  != 0x0000032100000011ull ||
diff --git a/tests/tcg/s390x/mie3-mvcrl.c b/tests/tcg/s390x/mie3-mvcrl.c
index 57b08e48d0..f749dad9c2 100644
--- a/tests/tcg/s390x/mie3-mvcrl.c
+++ b/tests/tcg/s390x/mie3-mvcrl.c
@@ -1,15 +1,17 @@
 #include <stdint.h>
 #include <string.h>
 
+
 static inline void mvcrl_8(const char *dst, const char *src)
 {
     asm volatile (
-    "llill %%r0, 8\n"
-    ".insn sse, 0xE50A00000000, 0(%[dst]), 0(%[src])"
-    : : [dst] "d" (dst), [src] "d" (src)
-    : "memory");
+        "llill %%r0, 8\n"
+        ".insn sse, 0xE50A00000000, 0(%[dst]), 0(%[src])"
+        : : [dst] "d" (dst), [src] "d" (src)
+        : "r0", "memory");
 }
 
+
 int main(int argc, char *argv[])
 {
     const char *alpha = "abcdefghijklmnop";
@@ -25,3 +27,5 @@ int main(int argc, char *argv[])
 
     return strncmp(alpha, tstr, 16ul);
 }
+
+
diff --git a/tests/tcg/s390x/mie3-sel.c b/tests/tcg/s390x/mie3-sel.c
index b0c5c9857d..4f54d37eeb 100644
--- a/tests/tcg/s390x/mie3-sel.c
+++ b/tests/tcg/s390x/mie3-sel.c
@@ -1,32 +1,27 @@
 #include <stdint.h>
 
+
 #define Fi3(S, ASM) uint64_t S(uint64_t a, uint64_t b, uint64_t c) \
-{                            \
-    uint64_t res = 0;        \
-    asm (                    \
-         "lg %%r2, %[a]\n"   \
-         "lg %%r3, %[b]\n"   \
-         "lg %%r0, %[c]\n"   \
-         "ltgr %%r0, %%r0\n" \
-         ASM                 \
-         "stg %%r0, %[res] " \
-         : [res] "=m" (res)  \
-         : [a] "m" (a),      \
-           [b] "m" (b),      \
-           [c] "m" (c)       \
-         : "r0", "r2",       \
-           "r3", "r4"        \
-    );                       \
-    return res;              \
+{                       \
+asm volatile (          \
+    "ltgr %[c], %[c]\n" \
+    ASM                 \
+    : [c] "+r" (c)      \
+    : [a]  "r" (a)      \
+    , [b]  "r" (b)      \
+);                      \
+    return c;           \
 }
 
-Fi3 (_selre,     ".insn rrf, 0xB9F00000, %%r0, %%r3, %%r2, 8\n")
-Fi3 (_selgrz,    ".insn rrf, 0xB9E30000, %%r0, %%r3, %%r2, 8\n")
-Fi3 (_selfhrnz,  ".insn rrf, 0xB9C00000, %%r0, %%r3, %%r2, 7\n")
+Fi3 (_selre,     ".insn rrf, 0xB9F00000, %[c], %[b], %[a], 8\n")
+Fi3 (_selgrz,    ".insn rrf, 0xB9E30000, %[c], %[b], %[a], 8\n")
+Fi3 (_selfhrnz,  ".insn rrf, 0xB9C00000, %[c], %[b], %[a], 7\n")
+
 
 int main(int argc, char *argv[])
 {
     uint64_t a = ~0, b = ~0, c = ~0;
+
     a =    _selre(0x066600000066ull, 0x066600000006ull, a);
     b =   _selgrz(0xF00D00000005ull, 0xF00D00000055ull, b);
     c = _selfhrnz(0x043200000044ull, 0x065400000004ull, c);
@@ -34,5 +29,6 @@ int main(int argc, char *argv[])
     return (int) (
         (0xFFFFFFFF00000066ull != a) ||
         (0x0000F00D00000005ull != b) ||
-        (0x00000654FFFFFFFFull != c));
+        (0x00000654FFFFFFFFull != c) );
 }
+
-- 
2.34.1



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

* Re: [PATCH v3] tests/tcg/s390x: Cleanup of mie3 tests.
  2022-03-01 21:43 [PATCH v3] tests/tcg/s390x: Cleanup of mie3 tests David Miller
@ 2022-03-01 22:18 ` Richard Henderson
  2022-03-07  8:42 ` Thomas Huth
  1 sibling, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2022-03-01 22:18 UTC (permalink / raw)
  To: David Miller, qemu-s390x, qemu-devel
  Cc: thuth, david, cohuck, farman, pasic, borntraeger

On 3/1/22 11:43, David Miller wrote:
> Adds clobbers and merges remaining separate asm statements.
> 
> v2 -> v3:
> * Removed all direct memory references in mie3-sel.c
> 
> v1 -> v2:
> * Corrected side in rebase conflict, removing older code.
> 
> Signed-off-by: David Miller<dmiller423@gmail.com>
> ---
>   tests/tcg/s390x/mie3-compl.c | 18 ++++++++++++-----
>   tests/tcg/s390x/mie3-mvcrl.c | 12 ++++++++----
>   tests/tcg/s390x/mie3-sel.c   | 38 ++++++++++++++++--------------------
>   3 files changed, 38 insertions(+), 30 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

PS: Those versioning comments should be after a --- line, so that standard tooling removes 
them.


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

* Re: [PATCH v3] tests/tcg/s390x: Cleanup of mie3 tests.
  2022-03-01 21:43 [PATCH v3] tests/tcg/s390x: Cleanup of mie3 tests David Miller
  2022-03-01 22:18 ` Richard Henderson
@ 2022-03-07  8:42 ` Thomas Huth
  2022-03-07 17:50   ` Richard Henderson
  1 sibling, 1 reply; 4+ messages in thread
From: Thomas Huth @ 2022-03-07  8:42 UTC (permalink / raw)
  To: David Miller, qemu-s390x, qemu-devel
  Cc: farman, david, cohuck, richard.henderson, pasic, borntraeger

On 01/03/2022 22.43, David Miller wrote:
> Adds clobbers and merges remaining separate asm statements.
> 
> v2 -> v3:
> * Removed all direct memory references in mie3-sel.c
> 
> v1 -> v2:
> * Corrected side in rebase conflict, removing older code.
> 
> Signed-off-by: David Miller <dmiller423@gmail.com>
> ---
>   tests/tcg/s390x/mie3-compl.c | 18 ++++++++++++-----
>   tests/tcg/s390x/mie3-mvcrl.c | 12 ++++++++----
>   tests/tcg/s390x/mie3-sel.c   | 38 ++++++++++++++++--------------------
>   3 files changed, 38 insertions(+), 30 deletions(-)
> 
> diff --git a/tests/tcg/s390x/mie3-compl.c b/tests/tcg/s390x/mie3-compl.c
> index 35649f3b02..938938df9e 100644
> --- a/tests/tcg/s390x/mie3-compl.c
> +++ b/tests/tcg/s390x/mie3-compl.c
> @@ -1,13 +1,20 @@
>   #include <stdint.h>
>   
> +
>   #define FbinOp(S, ASM) uint64_t S(uint64_t a, uint64_t b) \
> -{ \
> -    uint64_t res = 0; \
> -    asm ("llihf %[res],801\n" ASM \
> -         : [res]"=&r"(res) : [a]"r"(a), [b]"r"(b) : "cc"); \
> -    return res; \
> +{                       \
> +    uint64_t res = 0;   \
> +asm volatile (          \
> +    "llihf %[res],801\n"\
> +    ASM                 \
> +    : [res] "=&r" (res)  \
> +    : [a] "r" (a)       \
> +    , [b] "r" (b)       \
> +);                      \

Hmm, don't we still need "cc" in the clobber list? AFAICS some of these 
instructions still alter the CC register... so I'd suggest to rather drop 
the changes to this file?

Since QEMU has it's soft-freeze deadline tomorrow, I'll pick this up without 
the changes to mie3-compl.c for now... in case it's necessary, we can still 
sort this out later.

> +    return res;         \
>   }
>   
> +
>   /* AND WITH COMPLEMENT */
>   FbinOp(_ncrk,  ".insn rrf, 0xB9F50000, %[res], %[b], %[a], 0\n")
>   FbinOp(_ncgrk, ".insn rrf, 0xB9E50000, %[res], %[b], %[a], 0\n")
> @@ -28,6 +35,7 @@ FbinOp(_nogrk, ".insn rrf, 0xB9660000, %[res], %[b], %[a], 0\n")
>   FbinOp(_ocrk,  ".insn rrf, 0xB9750000, %[res], %[b], %[a], 0\n")
>   FbinOp(_ocgrk, ".insn rrf, 0xB9650000, %[res], %[b], %[a], 0\n")
>   
> +
>   int main(int argc, char *argv[])
>   {
>       if (_ncrk(0xFF88, 0xAA11)  != 0x0000032100000011ull ||
> diff --git a/tests/tcg/s390x/mie3-mvcrl.c b/tests/tcg/s390x/mie3-mvcrl.c
> index 57b08e48d0..f749dad9c2 100644
> --- a/tests/tcg/s390x/mie3-mvcrl.c
> +++ b/tests/tcg/s390x/mie3-mvcrl.c
> @@ -1,15 +1,17 @@
>   #include <stdint.h>
>   #include <string.h>
>   
> +
>   static inline void mvcrl_8(const char *dst, const char *src)
>   {
>       asm volatile (
> -    "llill %%r0, 8\n"
> -    ".insn sse, 0xE50A00000000, 0(%[dst]), 0(%[src])"
> -    : : [dst] "d" (dst), [src] "d" (src)
> -    : "memory");
> +        "llill %%r0, 8\n"
> +        ".insn sse, 0xE50A00000000, 0(%[dst]), 0(%[src])"
> +        : : [dst] "d" (dst), [src] "d" (src)
> +        : "r0", "memory");
>   }
>   
> +
>   int main(int argc, char *argv[])
>   {
>       const char *alpha = "abcdefghijklmnop";
> @@ -25,3 +27,5 @@ int main(int argc, char *argv[])
>   
>       return strncmp(alpha, tstr, 16ul);
>   }
> +
> +

Some tooling like "git am" (or rather "git apply") complain about trailing 
whitespace, so please try to avoid empty lines at the end of files in future 
patches if possible. (I've dropped the empty lines while picking up the 
patch, so no need to resend just because of this)

  Thomas



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

* Re: [PATCH v3] tests/tcg/s390x: Cleanup of mie3 tests.
  2022-03-07  8:42 ` Thomas Huth
@ 2022-03-07 17:50   ` Richard Henderson
  0 siblings, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2022-03-07 17:50 UTC (permalink / raw)
  To: Thomas Huth, David Miller, qemu-s390x, qemu-devel
  Cc: pasic, borntraeger, farman, cohuck, david

On 3/6/22 22:42, Thomas Huth wrote:
>> +asm volatile (          \
>> +    "llihf %[res],801\n"\
>> +    ASM                 \
>> +    : [res] "=&r" (res)  \
>> +    : [a] "r" (a)       \
>> +    , [b] "r" (b)       \
>> +);                      \
> 
> Hmm, don't we still need "cc" in the clobber list? AFAICS some of these instructions still 
> alter the CC register... so I'd suggest to rather drop the changes to this file?

Yep, my bad.  I wrongly assumed cc would be auto-clobbered, like i386.


r~


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

end of thread, other threads:[~2022-03-07 17:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-01 21:43 [PATCH v3] tests/tcg/s390x: Cleanup of mie3 tests David Miller
2022-03-01 22:18 ` Richard Henderson
2022-03-07  8:42 ` Thomas Huth
2022-03-07 17:50   ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).