| Message ID | 20220301214305.2778-1-dmiller423@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] tests/tcg/s390x: Cleanup of mie3 tests. | expand |
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.
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
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~
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) ); } +
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(-)