[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/3] s390x/tcg: Implement Miscellaneous-Instruction-Extens
From: |
David Hildenbrand |
Subject: |
Re: [PATCH v3 1/3] s390x/tcg: Implement Miscellaneous-Instruction-Extensions Facility 3 for the s390x |
Date: |
Wed, 16 Feb 2022 10:08:20 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0 |
On 15.02.22 21:26, David Miller wrote:
> resolves: https://gitlab.com/qemu-project/qemu/-/issues/737
> implements:
> AND WITH COMPLEMENT (NCRK, NCGRK)
> NAND (NNRK, NNGRK)
> NOT EXCLUSIVE OR (NXRK, NXGRK)
> NOR (NORK, NOGRK)
> OR WITH COMPLEMENT (OCRK, OCGRK)
> SELECT (SELR, SELGR)
> SELECT HIGH (SELFHR)
> MOVE RIGHT TO LEFT (MVCRL)
> POPULATION COUNT (POPCNT)
Unfortunately the patch can still not get applied because it's broken.
I strongly assume that either
* your mail sending client
* your MTA (Mail Transfer Agent)
messes with newlines in the patch and wraps long lines, corrupting the patch;
The mails are also not properly threaded.
I can spot:
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
Thunderbird/91.5.0
Try sending mails with git send-email instead. Let me point out a couple of
cases that are broken in the patch as I received it, so you can check if
they are correct on the source.
>
> Signed-off-by: David Miller <dmiller423@gmail.com>
> ---
> target/s390x/gen-features.c | 1 +
> target/s390x/helper.h | 1 +
> target/s390x/tcg/insn-data.def | 30 ++++++++++++++++--
> target/s390x/tcg/mem_helper.c | 20 ++++++++++++
> target/s390x/tcg/translate.c | 56 +++++++++++++++++++++++++++++++++-
> 5 files changed, 104 insertions(+), 4 deletions(-)
>
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 7cb1a6ec10..a3f30f69d9 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -740,6 +740,7 @@ static uint16_t qemu_LATEST[] = {
> /* add all new definitions before this point */
> static uint16_t qemu_MAX[] = {
> + S390_FEAT_MISC_INSTRUCTION_EXT3,
> /* generates a dependency warning, leave it out for now */
> S390_FEAT_MSA_EXT_5,
> };
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index 271b081e8c..69f69cf718 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -4,6 +4,7 @@ DEF_HELPER_FLAGS_4(nc, TCG_CALL_NO_WG, i32, env, i32,
> i64, i64)
^ this line was broken although it shouldn't have been. The hunk should be:
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 271b081e8c..69f69cf718 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -4,6 +4,7 @@ DEF_HELPER_FLAGS_4(nc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
DEF_HELPER_FLAGS_4(oc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
DEF_HELPER_FLAGS_4(xc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
DEF_HELPER_FLAGS_4(mvc, TCG_CALL_NO_WG, void, env, i32, i64, i64)
+DEF_HELPER_FLAGS_4(mvcrl, TCG_CALL_NO_WG, void, env, i64, i64, i64)
DEF_HELPER_FLAGS_4(mvcin, TCG_CALL_NO_WG, void, env, i32, i64, i64)
DEF_HELPER_FLAGS_4(clc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
DEF_HELPER_3(mvcl, i32, env, i32, i32)
diff --git a/target/s390x/tcg/insn-data.def b/target/s390x/tcg/insn-data.def
> DEF_HELPER_FLAGS_4(oc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
> DEF_HELPER_FLAGS_4(xc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
> DEF_HELPER_FLAGS_4(mvc, TCG_CALL_NO_WG, void, env, i32, i64, i64)
> +DEF_HELPER_FLAGS_4(mvcrl, TCG_CALL_NO_WG, void, env, i64, i64, i64)
> DEF_HELPER_FLAGS_4(mvcin, TCG_CALL_NO_WG, void, env, i32, i64, i64)
> DEF_HELPER_FLAGS_4(clc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
> DEF_HELPER_3(mvcl, i32, env, i32, i32)
> diff --git a/target/s390x/tcg/insn-data.def b/target/s390x/tcg/insn-data.def
> index 1c3e115712..a64555f824 100644
> --- a/target/s390x/tcg/insn-data.def
> +++ b/target/s390x/tcg/insn-data.def
> @@ -105,6 +105,9 @@
> D(0xa507, NILL, RI_a, Z, r1_o, i2_16u, r1, 0, andi, 0, 0x1000)
> D(0x9400, NI, SI, Z, la1, i2_8u, new, 0, ni, nz64, MO_UB)
> D(0xeb54, NIY, SIY, LD, la1, i2_8u, new, 0, ni, nz64, MO_UB)
> +/* AND WITH COMPLEMENT */
> + C(0xb9f5, NCRK, RRF_a, MIE3, r2, r3, new, r1_32, andc, nz32)
> + C(0xb9e5, NCGRK, RRF_a, MIE3, r2, r3, r1, 0, andc, nz64)
^ note that there is a newline in the code before /* BRANCH AND LINK */,
but it's gone in your patch. The hunk should have been
@@ -105,6 +105,9 @@
D(0xa507, NILL, RI_a, Z, r1_o, i2_16u, r1, 0, andi, 0, 0x1000)
D(0x9400, NI, SI, Z, la1, i2_8u, new, 0, ni, nz64, MO_UB)
D(0xeb54, NIY, SIY, LD, la1, i2_8u, new, 0, ni, nz64, MO_UB)
+/* AND WITH COMPLEMENT */
+ C(0xb9f5, NCRK, RRF_a, MIE3, r2, r3, new, r1_32, andc, nz32)
+ C(0xb9e5, NCGRK, RRF_a, MIE3, r2, r3, r1, 0, andc, nz64)
/* BRANCH AND LINK */
C(0x0500, BALR, RR_a, Z, 0, r2_nz, r1, 0, bal, 0)
> /* BRANCH AND LINK */
> C(0x0500, BALR, RR_a, Z, 0, r2_nz, r1, 0, bal, 0)
> @@ -640,6 +643,8 @@
> C(0xeb8e, MVCLU, RSY_a, E2, 0, a2, 0, 0, mvclu, 0)
> /* MOVE NUMERICS */
> C(0xd100, MVN, SS_a, Z, la1, a2, 0, 0, mvn, 0)
> +/* MOVE RIGHT TO LEFT */
> + C(0xe50a, MVCRL, SSE, MIE3, la1, a2, 0, 0, mvcrl, 0)
> /* MOVE PAGE */
> C(0xb254, MVPG, RRE, Z, 0, 0, 0, 0, mvpg, 0)
> /* MOVE STRING */
> @@ -707,6 +712,16 @@
> F(0xed0f, MSEB, RXF, Z, e1, m2_32u, new, e1, mseb, 0, IF_BFP)
> F(0xed1f, MSDB, RXF, Z, f1, m2_64, new, f1, msdb, 0, IF_BFP)
> +/* NAND */
^ note the + is preceded by " "
This hunk should have been
/* MOVE STRING */
C(0xb255, MVST, RRE, Z, 0, 0, 0, 0, mvst, 0)
+/* NAND */
+ C(0xb974, NNRK, RRF_a, MIE3, r2, r3, new, r1_32, nand, nz32)
+ C(0xb964, NNGRK, RRF_a, MIE3, r2, r3, r1, 0, nand, nz64)
+/* NOR */
+ C(0xb976, NORK, RRF_a, MIE3, r2, r3, new, r1_32, nor, nz32)
+ C(0xb966, NOGRK, RRF_a, MIE3, r2, r3, r1, 0, nor, nz64)
+/* NOT EXCLUSIVE OR */
+ C(0xb977, NXRK, RRF_a, MIE3, r2, r3, new, r1_32, nxor, nz32)
+ C(0xb967, NXGRK, RRF_a, MIE3, r2, r3, r1, 0, nxor, nz64)
+
/* MOVE WITH OPTIONAL SPECIFICATION */
[..]
> {
> - gen_helper_popcnt(o->out, o->in2);
> + const uint8_t m3 = get_field(s, m3);
> +
> + if ((m3 & 1) && s390_has_feat(S390_FEAT_MISC_INSTRUCTION_EXT3)) {
> + tcg_gen_ctpop_i64(o->out, o->in2);
> + }
> + else {
This should not be your patch workflow fault: "} else {"
--
Thanks,
David / dhildenb