From: David Hildenbrand <david@redhat.com>
To: Richard Henderson <richard.henderson@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [PATCH 2/4] target/s390x: Improve ADD LOGICAL WITH CARRY
Date: Tue, 20 Oct 2020 16:12:46 +0200 [thread overview]
Message-ID: <91224b3b-0e0d-9071-401d-3773fa42687f@redhat.com> (raw)
In-Reply-To: <20201017022901.78425-3-richard.henderson@linaro.org>
On 17.10.20 04:28, Richard Henderson wrote:
> Now that ADD LOGICAL outputs carry, we can use that as input directly.
> It also means we can re-use CC_OP_ZC and produce an output carry
> directly from ADD LOGICAL WITH CARRY.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/s390x/internal.h | 2 --
> target/s390x/cc_helper.c | 26 ---------------
> target/s390x/helper.c | 2 --
> target/s390x/translate.c | 66 ++++++++++++++++++--------------------
> target/s390x/insn-data.def | 8 ++---
> 5 files changed, 35 insertions(+), 69 deletions(-)
>
> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> index 55c5442102..f5f3ae063e 100644
> --- a/target/s390x/internal.h
> +++ b/target/s390x/internal.h
> @@ -170,7 +170,6 @@ enum cc_op {
> CC_OP_LTGT0_64, /* signed less/greater than 0 (64bit) */
>
> CC_OP_ADD_64, /* overflow on add (64bit) */
> - CC_OP_ADDC_64, /* overflow on unsigned add-carry (64bit) */
> CC_OP_SUB_64, /* overflow on subtraction (64bit) */
> CC_OP_SUBU_64, /* overflow on unsigned subtraction (64bit) */
> CC_OP_SUBB_64, /* overflow on unsigned sub-borrow (64bit) */
> @@ -179,7 +178,6 @@ enum cc_op {
> CC_OP_MULS_64, /* overflow on signed multiply (64bit) */
>
> CC_OP_ADD_32, /* overflow on add (32bit) */
> - CC_OP_ADDC_32, /* overflow on unsigned add-carry (32bit) */
> CC_OP_SUB_32, /* overflow on subtraction (32bit) */
> CC_OP_SUBU_32, /* overflow on unsigned subtraction (32bit) */
> CC_OP_SUBB_32, /* overflow on unsigned sub-borrow (32bit) */
> diff --git a/target/s390x/cc_helper.c b/target/s390x/cc_helper.c
> index 59da4d1cc2..cd2c5c4b39 100644
> --- a/target/s390x/cc_helper.c
> +++ b/target/s390x/cc_helper.c
> @@ -144,16 +144,6 @@ static uint32_t cc_calc_add_64(int64_t a1, int64_t a2, int64_t ar)
> }
> }
>
> -static uint32_t cc_calc_addc_64(uint64_t a1, uint64_t a2, uint64_t ar)
> -{
> - /* Recover a2 + carry_in. */
> - uint64_t a2c = ar - a1;
> - /* Check for a2+carry_in overflow, then a1+a2c overflow. */
> - int carry_out = (a2c < a2) || (ar < a1);
> -
> - return (ar != 0) + 2 * carry_out;
> -}
> -
> static uint32_t cc_calc_sub_64(int64_t a1, int64_t a2, int64_t ar)
> {
> if ((a1 > 0 && a2 < 0 && ar < 0) || (a1 < 0 && a2 > 0 && ar > 0)) {
> @@ -240,16 +230,6 @@ static uint32_t cc_calc_add_32(int32_t a1, int32_t a2, int32_t ar)
> }
> }
>
> -static uint32_t cc_calc_addc_32(uint32_t a1, uint32_t a2, uint32_t ar)
> -{
> - /* Recover a2 + carry_in. */
> - uint32_t a2c = ar - a1;
> - /* Check for a2+carry_in overflow, then a1+a2c overflow. */
> - int carry_out = (a2c < a2) || (ar < a1);
> -
> - return (ar != 0) + 2 * carry_out;
> -}
> -
> static uint32_t cc_calc_sub_32(int32_t a1, int32_t a2, int32_t ar)
> {
> if ((a1 > 0 && a2 < 0 && ar < 0) || (a1 < 0 && a2 > 0 && ar > 0)) {
> @@ -485,9 +465,6 @@ static uint32_t do_calc_cc(CPUS390XState *env, uint32_t cc_op,
> case CC_OP_ADD_64:
> r = cc_calc_add_64(src, dst, vr);
> break;
> - case CC_OP_ADDC_64:
> - r = cc_calc_addc_64(src, dst, vr);
> - break;
> case CC_OP_SUB_64:
> r = cc_calc_sub_64(src, dst, vr);
> break;
> @@ -513,9 +490,6 @@ static uint32_t do_calc_cc(CPUS390XState *env, uint32_t cc_op,
> case CC_OP_ADD_32:
> r = cc_calc_add_32(src, dst, vr);
> break;
> - case CC_OP_ADDC_32:
> - r = cc_calc_addc_32(src, dst, vr);
> - break;
> case CC_OP_SUB_32:
> r = cc_calc_sub_32(src, dst, vr);
> break;
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index db87a62a57..4f4561bc64 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -403,14 +403,12 @@ const char *cc_name(enum cc_op cc_op)
> [CC_OP_LTGT0_32] = "CC_OP_LTGT0_32",
> [CC_OP_LTGT0_64] = "CC_OP_LTGT0_64",
> [CC_OP_ADD_64] = "CC_OP_ADD_64",
> - [CC_OP_ADDC_64] = "CC_OP_ADDC_64",
> [CC_OP_SUB_64] = "CC_OP_SUB_64",
> [CC_OP_SUBU_64] = "CC_OP_SUBU_64",
> [CC_OP_SUBB_64] = "CC_OP_SUBB_64",
> [CC_OP_ABS_64] = "CC_OP_ABS_64",
> [CC_OP_NABS_64] = "CC_OP_NABS_64",
> [CC_OP_ADD_32] = "CC_OP_ADD_32",
> - [CC_OP_ADDC_32] = "CC_OP_ADDC_32",
> [CC_OP_SUB_32] = "CC_OP_SUB_32",
> [CC_OP_SUBU_32] = "CC_OP_SUBU_32",
> [CC_OP_SUBB_32] = "CC_OP_SUBB_32",
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 9bf4c14f66..570b3c88c8 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -600,12 +600,10 @@ static void gen_op_calc_cc(DisasContext *s)
> dummy = tcg_const_i64(0);
> /* FALLTHRU */
> case CC_OP_ADD_64:
> - case CC_OP_ADDC_64:
> case CC_OP_SUB_64:
> case CC_OP_SUBU_64:
> case CC_OP_SUBB_64:
> case CC_OP_ADD_32:
> - case CC_OP_ADDC_32:
> case CC_OP_SUB_32:
> case CC_OP_SUBU_32:
> case CC_OP_SUBB_32:
> @@ -665,12 +663,10 @@ static void gen_op_calc_cc(DisasContext *s)
> gen_helper_calc_cc(cc_op, cpu_env, local_cc_op, cc_src, cc_dst, dummy);
> break;
> case CC_OP_ADD_64:
> - case CC_OP_ADDC_64:
> case CC_OP_SUB_64:
> case CC_OP_SUBU_64:
> case CC_OP_SUBB_64:
> case CC_OP_ADD_32:
> - case CC_OP_ADDC_32:
> case CC_OP_SUB_32:
> case CC_OP_SUBU_32:
> case CC_OP_SUBB_32:
> @@ -1442,30 +1438,40 @@ static DisasJumpType op_addu64(DisasContext *s, DisasOps *o)
> return DISAS_NEXT;
> }
>
> -static DisasJumpType op_addc(DisasContext *s, DisasOps *o)
> +/* Compute carry into cc_src. */
> +static void compute_carry(DisasContext *s)
> {
> - DisasCompare cmp;
> - TCGv_i64 carry;
> -
> - tcg_gen_add_i64(o->out, o->in1, o->in2);
> -
> - /* The carry flag is the msb of CC, therefore the branch mask that would
> - create that comparison is 3. Feeding the generated comparison to
> - setcond produces the carry flag that we desire. */
> - disas_jcc(s, &cmp, 3);
> - carry = tcg_temp_new_i64();
> - if (cmp.is_64) {
> - tcg_gen_setcond_i64(cmp.cond, carry, cmp.u.s64.a, cmp.u.s64.b);
> - } else {
> - TCGv_i32 t = tcg_temp_new_i32();
> - tcg_gen_setcond_i32(cmp.cond, t, cmp.u.s32.a, cmp.u.s32.b);
> - tcg_gen_extu_i32_i64(carry, t);
> - tcg_temp_free_i32(t);
> + switch (s->cc_op) {
> + case CC_OP_ADDU:
Can you add a comment that we have the carry right in out hands already?
Took me while to figure that out.
Apart from that
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
next prev parent reply other threads:[~2020-10-20 14:14 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-17 2:28 [PATCH 0/4] target/s390x: Improve carry computation Richard Henderson
2020-10-17 2:28 ` [PATCH 1/4] target/s390x: Improve cc computation for ADD LOGICAL Richard Henderson
2020-10-20 14:08 ` David Hildenbrand
2020-10-17 2:28 ` [PATCH 2/4] target/s390x: Improve ADD LOGICAL WITH CARRY Richard Henderson
2020-10-20 14:12 ` David Hildenbrand [this message]
2020-10-17 2:29 ` [PATCH 3/4] target/s390x: Improve cc computation for SUBTRACT LOGICAL Richard Henderson
2020-10-20 14:14 ` David Hildenbrand
2020-10-17 2:29 ` [PATCH 4/4] target/s390x: Improve SUB LOGICAL WITH BORROW Richard Henderson
2020-10-20 14:17 ` David Hildenbrand
2020-10-20 15:11 ` Richard Henderson
2020-10-20 15:12 ` David Hildenbrand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=91224b3b-0e0d-9071-401d-3773fa42687f@redhat.com \
--to=david@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.