1. Patchew
  2. QEMU
  3. View series

[PATCH v3] tests/tcg/s390x: Cleanup of mie3 tests.

David Miller posted 1 patch 2 years, 2 months ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220301214305.2778-1-dmiller423@gmail.com
Maintainers: Richard Henderson <richard.henderson@linaro.org>, David Hildenbrand <david@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Thomas Huth <thuth@redhat.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(-)
[PATCH v3] tests/tcg/s390x: Cleanup of mie3 tests.
Posted by David Miller 2 years, 2 months ago 
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
Re: [PATCH v3] tests/tcg/s390x: Cleanup of mie3 tests.
Posted by Richard Henderson 2 years, 2 months ago 
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.
Re: [PATCH v3] tests/tcg/s390x: Cleanup of mie3 tests.
Posted by Thomas Huth 2 years, 1 month ago 
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
Re: [PATCH v3] tests/tcg/s390x: Cleanup of mie3 tests.
Posted by Richard Henderson 2 years, 1 month ago 
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~

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.