From: Claudio Fontana <claudio.fontana@huawei.com>
To: Richard Henderson <rth@twiddle.net>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2] tcg-aarch64: Use 32-bit loads for qemu_ld_i32
Date: Fri, 12 Sep 2014 16:14:30 +0200 [thread overview]
Message-ID: <5412FFC6.7040903@huawei.com> (raw)
In-Reply-To: <1409762817-24029-3-git-send-email-rth@twiddle.net>
On 03.09.2014 18:46, Richard Henderson wrote:
> The "old" qemu_ld opcode did not specify the size of the result,
> and so we had to assume full register width. With the new opcodes,
> we can narrow the result.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> tcg/aarch64/tcg-target.c | 29 ++++++++++++++++-------------
> tcg/tcg-be-ldst.h | 1 +
> tcg/tcg.h | 3 ++-
> 3 files changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
> index 56dae66..5017cfe 100644
> --- a/tcg/aarch64/tcg-target.c
> +++ b/tcg/aarch64/tcg-target.c
> @@ -1007,7 +1007,7 @@ static void tcg_out_qemu_ld_slow_path(TCGContext *s, TCGLabelQemuLdst *lb)
> tcg_out_adr(s, TCG_REG_X3, lb->raddr);
> tcg_out_call(s, qemu_ld_helpers[opc & ~MO_SIGN]);
> if (opc & MO_SIGN) {
> - tcg_out_sxt(s, TCG_TYPE_I64, size, lb->datalo_reg, TCG_REG_X0);
> + tcg_out_sxt(s, lb->type, size, lb->datalo_reg, TCG_REG_X0);
> } else {
> tcg_out_mov(s, size == MO_64, lb->datalo_reg, TCG_REG_X0);
> }
> @@ -1032,14 +1032,15 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *lb)
> }
>
> static void add_qemu_ldst_label(TCGContext *s, bool is_ld, TCGMemOp opc,
> - TCGReg data_reg, TCGReg addr_reg,
> + TCGType type, TCGReg data_reg, TCGReg addr_reg,
> int mem_index, tcg_insn_unit *raddr,
> tcg_insn_unit *label_ptr)
> {
> TCGLabelQemuLdst *label = new_ldst_label(s);
>
> - label->is_ld = is_ld;
> label->opc = opc;
> + label->is_ld = is_ld;
> + label->type = type;
> label->datalo_reg = data_reg;
> label->addrlo_reg = addr_reg;
> label->mem_index = mem_index;
> @@ -1108,7 +1109,7 @@ static void tcg_out_tlb_read(TCGContext *s, TCGReg addr_reg, TCGMemOp s_bits,
>
> #endif /* CONFIG_SOFTMMU */
>
> -static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop,
> +static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop, TCGType type,
> TCGReg data_r, TCGReg addr_r, TCGReg off_r)
> {
> const TCGMemOp bswap = memop & MO_BSWAP;
> @@ -1118,7 +1119,8 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop,
> tcg_out_ldst_r(s, I3312_LDRB, data_r, addr_r, off_r);
> break;
> case MO_SB:
> - tcg_out_ldst_r(s, I3312_LDRSBX, data_r, addr_r, off_r);
> + tcg_out_ldst_r(s, type ? I3312_LDRSBX : I3312_LDRSBW,
since we are using the enum type TCGType, why do we check type as an int?
I would have expected the conditional to be something like
type == TCG_TYPE_I32 ? I3312_LDRSBW : I3312_LDRSBX
> + data_r, addr_r, off_r);
> break;
> case MO_UW:
> tcg_out_ldst_r(s, I3312_LDRH, data_r, addr_r, off_r);
> @@ -1130,9 +1132,10 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop,
> if (bswap) {
> tcg_out_ldst_r(s, I3312_LDRH, data_r, addr_r, off_r);
> tcg_out_rev16(s, data_r, data_r);
> - tcg_out_sxt(s, TCG_TYPE_I64, MO_16, data_r, data_r);
> + tcg_out_sxt(s, type, MO_16, data_r, data_r);
> } else {
> - tcg_out_ldst_r(s, I3312_LDRSHX, data_r, addr_r, off_r);
> + tcg_out_ldst_r(s, type ? I3312_LDRSHX : I3312_LDRSHW,
> + data_r, addr_r, off_r);
> }
> break;
> case MO_UL:
> @@ -1197,18 +1200,18 @@ static void tcg_out_qemu_st_direct(TCGContext *s, TCGMemOp memop,
> }
>
> static void tcg_out_qemu_ld(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
> - TCGMemOp memop, int mem_index)
> + TCGMemOp memop, TCGType type, int mem_index)
> {
> #ifdef CONFIG_SOFTMMU
> TCGMemOp s_bits = memop & MO_SIZE;
> tcg_insn_unit *label_ptr;
>
> tcg_out_tlb_read(s, addr_reg, s_bits, &label_ptr, mem_index, 1);
> - tcg_out_qemu_ld_direct(s, memop, data_reg, addr_reg, TCG_REG_X1);
> - add_qemu_ldst_label(s, true, memop, data_reg, addr_reg,
> + tcg_out_qemu_ld_direct(s, memop, type, data_reg, addr_reg, TCG_REG_X1);
> + add_qemu_ldst_label(s, true, memop, type, data_reg, addr_reg,
> mem_index, s->code_ptr, label_ptr);
> #else /* !CONFIG_SOFTMMU */
> - tcg_out_qemu_ld_direct(s, memop, data_reg, addr_reg,
> + tcg_out_qemu_ld_direct(s, memop, type, data_reg, addr_reg,
> GUEST_BASE ? TCG_REG_GUEST_BASE : TCG_REG_XZR);
> #endif /* CONFIG_SOFTMMU */
> }
> @@ -1222,7 +1225,7 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
>
> tcg_out_tlb_read(s, addr_reg, s_bits, &label_ptr, mem_index, 0);
> tcg_out_qemu_st_direct(s, memop, data_reg, addr_reg, TCG_REG_X1);
> - add_qemu_ldst_label(s, false, memop, data_reg, addr_reg,
> + add_qemu_ldst_label(s, false, memop, s_bits == MO_64, data_reg, addr_reg,
> mem_index, s->code_ptr, label_ptr);
> #else /* !CONFIG_SOFTMMU */
> tcg_out_qemu_st_direct(s, memop, data_reg, addr_reg,
> @@ -1515,7 +1518,7 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
>
> case INDEX_op_qemu_ld_i32:
> case INDEX_op_qemu_ld_i64:
> - tcg_out_qemu_ld(s, a0, a1, a2, args[3]);
> + tcg_out_qemu_ld(s, a0, a1, a2, ext, args[3]);
> break;
> case INDEX_op_qemu_st_i32:
> case INDEX_op_qemu_st_i64:
> diff --git a/tcg/tcg-be-ldst.h b/tcg/tcg-be-ldst.h
> index 904eeda..825de14 100644
> --- a/tcg/tcg-be-ldst.h
> +++ b/tcg/tcg-be-ldst.h
> @@ -29,6 +29,7 @@ QEMU_BUILD_BUG_ON(NB_MMU_MODES > 8);
> typedef struct TCGLabelQemuLdst {
> TCGMemOp opc : 4;
> bool is_ld : 1; /* qemu_ld: true, qemu_st: false */
> + TCGType type : 1; /* result type of a load */
> TCGReg addrlo_reg : 5; /* reg index for low word of guest virtual addr */
> TCGReg addrhi_reg : 5; /* reg index for high word of guest virtual addr */
> TCGReg datalo_reg : 5; /* reg index for low word to be loaded or stored */
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index 997a704..fcc50bd 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -194,7 +194,6 @@ typedef struct TCGPool {
> typedef enum TCGType {
> TCG_TYPE_I32,
> TCG_TYPE_I64,
> - TCG_TYPE_COUNT, /* number of different types */
>
> /* An alias for the size of the host register. */
> #if TCG_TARGET_REG_BITS == 32
> @@ -218,6 +217,8 @@ typedef enum TCGType {
> #endif
> } TCGType;
>
> +#define TCG_TYPE_COUNT 2
> +
why this move from the enum to this standalone define?
> /* Constants for qemu_ld and qemu_st for the Memory Operation field. */
> typedef enum TCGMemOp {
> MO_8 = 0,
>
Ciao,
Claudio
next prev parent reply other threads:[~2014-09-12 14:15 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-03 16:46 [Qemu-devel] [PATCH 0/2] tcg/aarch64 improvement Richard Henderson
2014-09-03 16:46 ` [Qemu-devel] [PATCH 1/2] tcg: Compress TCGLabelQemuLdst Richard Henderson
2014-09-08 15:16 ` Claudio Fontana
2014-09-03 16:46 ` [Qemu-devel] [PATCH 2/2] tcg-aarch64: Use 32-bit loads for qemu_ld_i32 Richard Henderson
2014-09-12 14:14 ` Claudio Fontana [this message]
2014-09-12 15:49 ` Richard Henderson
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=5412FFC6.7040903@huawei.com \
--to=claudio.fontana@huawei.com \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
/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.