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

[Thomas Huth]

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