QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Stefan Weil <sw@weilnetz.de>
To: Richard Henderson <richard.henderson@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [PATCH 22/23] tcg/tci: Implement 64-bit division
Date: Thu, 28 Jan 2021 11:04:44 +0100
Message-ID: <8d395910-8656-9c4f-5dfa-749c50e8de22@weilnetz.de> (raw)
In-Reply-To: <20210128082331.196801-23-richard.henderson@linaro.org>

Am 28.01.21 um 09:23 schrieb Richard Henderson:

> Trivially implemented like other arithmetic.
> Tested via check-tcg and the ppc64 target.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/tci/tcg-target.h     |  4 ++--
>   tcg/tci.c                | 28 ++++++++++++++++++++++------
>   tcg/tci/tcg-target.c.inc | 12 ++++--------
>   3 files changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/tcg/tci/tcg-target.h b/tcg/tci/tcg-target.h
> index bb784e018e..7fc349a3de 100644
> --- a/tcg/tci/tcg-target.h
> +++ b/tcg/tci/tcg-target.h
> @@ -100,8 +100,8 @@
>   #define TCG_TARGET_HAS_extract_i64      0
>   #define TCG_TARGET_HAS_sextract_i64     0
>   #define TCG_TARGET_HAS_extract2_i64     0
> -#define TCG_TARGET_HAS_div_i64          0
> -#define TCG_TARGET_HAS_rem_i64          0
> +#define TCG_TARGET_HAS_div_i64          1
> +#define TCG_TARGET_HAS_rem_i64          1
>   #define TCG_TARGET_HAS_ext8s_i64        1
>   #define TCG_TARGET_HAS_ext16s_i64       1
>   #define TCG_TARGET_HAS_ext32s_i64       1
> diff --git a/tcg/tci.c b/tcg/tci.c
> index 32931ea611..0065c854a4 100644
> --- a/tcg/tci.c
> +++ b/tcg/tci.c
> @@ -889,14 +889,30 @@ uintptr_t QEMU_DISABLE_CFI tcg_qemu_tb_exec(CPUArchState *env,
>               t2 = tci_read_ri64(regs, &tb_ptr);
>               tci_write_reg(regs, t0, t1 * t2);
>               break;
> -#if TCG_TARGET_HAS_div_i64


I suggest to keep this and other identical #if TCG_TARGET_HAS_div_i64 
and the matching #endif in the code.

They are not needed for the default case, but useful as long as it is 
possible to write a TCG backend without those opcodes. Then it helps 
running TCI with the same subset of opcodes.

As far as I see currently s390 does not set that macro. So to compare 
the s390 TCG with TCI, I'd want to have the same subset of TCG opcodes, 
which means none of those implemented here. That can be done when the 
preprocessor conditionals remain in the code.


>           case INDEX_op_div_i64:
> -        case INDEX_op_divu_i64:
> -        case INDEX_op_rem_i64:
> -        case INDEX_op_remu_i64:
> -            TODO();
> +            t0 = *tb_ptr++;
> +            t1 = tci_read_ri64(regs, &tb_ptr);
> +            t2 = tci_read_ri64(regs, &tb_ptr);
> +            tci_write_reg(regs, t0, (int64_t)t1 / (int64_t)t2);
> +            break;
> +        case INDEX_op_divu_i64:
> +            t0 = *tb_ptr++;
> +            t1 = tci_read_ri64(regs, &tb_ptr);
> +            t2 = tci_read_ri64(regs, &tb_ptr);
> +            tci_write_reg(regs, t0, (uint64_t)t1 / (uint64_t)t2);
> +            break;
> +        case INDEX_op_rem_i64:
> +            t0 = *tb_ptr++;
> +            t1 = tci_read_ri64(regs, &tb_ptr);
> +            t2 = tci_read_ri64(regs, &tb_ptr);
> +            tci_write_reg(regs, t0, (int64_t)t1 % (int64_t)t2);
> +            break;
> +        case INDEX_op_remu_i64:
> +            t0 = *tb_ptr++;
> +            t1 = tci_read_ri64(regs, &tb_ptr);
> +            t2 = tci_read_ri64(regs, &tb_ptr);
> +            tci_write_reg(regs, t0, (uint64_t)t1 % (uint64_t)t2);
>               break;
> -#endif
>           case INDEX_op_and_i64:
>               t0 = *tb_ptr++;
>               t1 = tci_read_ri64(regs, &tb_ptr);
> diff --git a/tcg/tci/tcg-target.c.inc b/tcg/tci/tcg-target.c.inc
> index 842807ff2e..8c0e77a0be 100644
> --- a/tcg/tci/tcg-target.c.inc
> +++ b/tcg/tci/tcg-target.c.inc
> @@ -146,12 +146,10 @@ static const TCGTargetOpDef tcg_target_op_defs[] = {
>       { INDEX_op_add_i64, { R, RI, RI } },
>       { INDEX_op_sub_i64, { R, RI, RI } },
>       { INDEX_op_mul_i64, { R, RI, RI } },
> -#if TCG_TARGET_HAS_div_i64
>       { INDEX_op_div_i64, { R, R, R } },
>       { INDEX_op_divu_i64, { R, R, R } },
>       { INDEX_op_rem_i64, { R, R, R } },
>       { INDEX_op_remu_i64, { R, R, R } },
> -#endif
>       { INDEX_op_and_i64, { R, RI, RI } },
>   #if TCG_TARGET_HAS_andc_i64
>       { INDEX_op_andc_i64, { R, RI, RI } },
> @@ -678,6 +676,10 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
>       case INDEX_op_sar_i64:
>       case INDEX_op_rotl_i64:     /* Optional (TCG_TARGET_HAS_rot_i64). */
>       case INDEX_op_rotr_i64:     /* Optional (TCG_TARGET_HAS_rot_i64). */
> +    case INDEX_op_div_i64:      /* Optional (TCG_TARGET_HAS_div_i64). */
> +    case INDEX_op_divu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */
> +    case INDEX_op_rem_i64:      /* Optional (TCG_TARGET_HAS_div_i64). */
> +    case INDEX_op_remu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */
>           tcg_out_r(s, args[0]);
>           tcg_out_ri64(s, const_args[1], args[1]);
>           tcg_out_ri64(s, const_args[2], args[2]);
> @@ -691,12 +693,6 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
>           tcg_debug_assert(args[4] <= UINT8_MAX);
>           tcg_out8(s, args[4]);
>           break;
> -    case INDEX_op_div_i64:      /* Optional (TCG_TARGET_HAS_div_i64). */
> -    case INDEX_op_divu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */
> -    case INDEX_op_rem_i64:      /* Optional (TCG_TARGET_HAS_div_i64). */
> -    case INDEX_op_remu_i64:     /* Optional (TCG_TARGET_HAS_div_i64). */
> -        TODO();
> -        break;
>       case INDEX_op_brcond_i64:
>           tcg_out_r(s, args[0]);
>           tcg_out_ri64(s, const_args[1], args[1]);


Thanks. See my comment above where I suggest a slight modification.

Reviewed-by: Stefan Weil <sw@weilnetz.de>





  reply index

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-28  8:23 [PATCH 00/23] TCI fixes and cleanups Richard Henderson
2021-01-28  8:23 ` [PATCH 01/23] configure: Fix --enable-tcg-interpreter Richard Henderson
2021-01-28 11:47   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 02/23] tcg: Manage splitwx in tc_ptr_to_region_tree by hand Richard Henderson
2021-01-28 13:09   ` Alex Bennée
2021-01-28 13:54     ` Alex Bennée
2021-01-28  8:23 ` [PATCH 03/23] exec: Make tci_tb_ptr thread-local Richard Henderson
2021-01-28  8:23 ` [PATCH 04/23] tcg/tci: Implement INDEX_op_ld16s_i32 Richard Henderson
2021-01-28 13:59   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 05/23] tcg/tci: Implement INDEX_op_ld8s_i64 Richard Henderson
2021-01-28 13:59   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 06/23] tcg/tci: Inline tci_write_reg32s into the only caller Richard Henderson
2021-01-28 15:28   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 07/23] tcg/tci: Inline tci_write_reg8 into its callers Richard Henderson
2021-01-28 15:30   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 08/23] tcg/tci: Inline tci_write_reg16 into the only caller Richard Henderson
2021-01-28 15:30   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 09/23] tcg/tci: Inline tci_write_reg32 into all callers Richard Henderson
2021-01-28 15:31   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 10/23] tcg/tci: Inline tci_write_reg64 into 64-bit callers Richard Henderson
2021-01-28 15:32   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 11/23] tcg/tci: Merge INDEX_op_ld8u_{i32,i64} Richard Henderson
2021-01-28 16:18   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 12/23] tcg/tci: Merge INDEX_op_ld8s_{i32,i64} Richard Henderson
2021-01-28 16:18   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 13/23] tcg/tci: Merge INDEX_op_ld16u_{i32,i64} Richard Henderson
2021-01-28 16:19   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 14/23] tcg/tci: Merge INDEX_op_ld16s_{i32,i64} Richard Henderson
2021-01-28 16:20   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 15/23] tcg/tci: Merge INDEX_op_{ld_i32,ld32u_i64} Richard Henderson
2021-01-28 16:20   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 16/23] tcg/tci: Merge INDEX_op_st8_{i32,i64} Richard Henderson
2021-01-28 16:20   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 17/23] tcg/tci: Merge INDEX_op_st16_{i32,i64} Richard Henderson
2021-01-28 16:20   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 18/23] tcg/tci: Move stack bounds check to compile-time Richard Henderson
2021-01-28 16:37   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 19/23] tcg/tci: Merge INDEX_op_{st_i32,st32_i64} Richard Henderson
2021-01-28 16:38   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 20/23] tcg/tci: Use g_assert_not_reached Richard Henderson
2021-01-28 10:07   ` Stefan Weil
2021-01-28 15:34   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 21/23] tcg/tci: Remove dead code for TCG_TARGET_HAS_div2_* Richard Henderson
2021-01-28 15:36   ` Alex Bennée
2021-01-28 15:39   ` Stefan Weil
2021-01-28 17:56     ` Richard Henderson
2021-01-28  8:23 ` [PATCH 22/23] tcg/tci: Implement 64-bit division Richard Henderson
2021-01-28 10:04   ` Stefan Weil [this message]
2021-01-28 17:56     ` Richard Henderson
2021-01-28 15:38   ` Alex Bennée
2021-01-28  8:23 ` [PATCH 23/23] tcg/tci: Remove TODO as unused Richard Henderson
2021-01-28 15:38   ` Alex Bennée
2021-01-28 15:38 ` [PATCH 00/23] TCI fixes and cleanups Alex Bennée
2021-01-28 16:39 ` Alex Bennée

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=8d395910-8656-9c4f-5dfa-749c50e8de22@weilnetz.de \
    --to=sw@weilnetz.de \
    --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

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git
	git clone --mirror https://lore.kernel.org/qemu-devel/2 qemu-devel/git/2.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git