From: "Alex Bennée" <alex.bennee@linaro.org>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: git@xen0n.name, Alistair.Francis@wdc.com, f4bug@amsat.org,
qemu-devel@nongnu.org
Subject: Re: [PATCH 6/8] tcg/aarch64: Support TCG_TARGET_SIGNED_ADDR32
Date: Mon, 11 Oct 2021 11:28:19 +0100 [thread overview]
Message-ID: <877dejyhrb.fsf@linaro.org> (raw)
In-Reply-To: <20211010174401.141339-7-richard.henderson@linaro.org>
Richard Henderson <richard.henderson@linaro.org> writes:
> AArch64 has both sign and zero-extending addressing modes, which
> means that either treatment of guest addresses is equally efficient.
> Enabling this for AArch64 gives us testing of the feature in CI.
So which guests front ends will exercise this backend? I realise you
never mentioned it in the cover letter. Is this something we can
exercise in 32 bit user mode tests?
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> tcg/aarch64/tcg-target-sa32.h | 8 ++++-
> tcg/aarch64/tcg-target.c.inc | 68 ++++++++++++++++++++++-------------
> 2 files changed, 51 insertions(+), 25 deletions(-)
>
> diff --git a/tcg/aarch64/tcg-target-sa32.h b/tcg/aarch64/tcg-target-sa32.h
> index cb185b1526..c99e502e4c 100644
> --- a/tcg/aarch64/tcg-target-sa32.h
> +++ b/tcg/aarch64/tcg-target-sa32.h
> @@ -1 +1,7 @@
> -#define TCG_TARGET_SIGNED_ADDR32 0
> +/*
> + * AArch64 has both SXTW and UXTW addressing modes, which means that
> + * it is agnostic to how guest addresses should be represented.
> + * Because aarch64 is more common than the other hosts that will
> + * want to use this feature, enable it for continuous testing.
> + */
> +#define TCG_TARGET_SIGNED_ADDR32 1
> diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
> index 5edca8d44d..88b2963f9d 100644
> --- a/tcg/aarch64/tcg-target.c.inc
> +++ b/tcg/aarch64/tcg-target.c.inc
> @@ -12,6 +12,7 @@
>
> #include "../tcg-pool.c.inc"
> #include "qemu/bitops.h"
> +#include "tcg-target-sa32.h"
>
> /* We're going to re-use TCGType in setting of the SF bit, which controls
> the size of the operation performed. If we know the values match, it
> @@ -804,12 +805,12 @@ static void tcg_out_insn_3617(TCGContext *s, AArch64Insn insn, bool q,
> }
>
> static void tcg_out_insn_3310(TCGContext *s, AArch64Insn insn,
> - TCGReg rd, TCGReg base, TCGType ext,
> + TCGReg rd, TCGReg base, int option,
> TCGReg regoff)
> {
> /* Note the AArch64Insn constants above are for C3.3.12. Adjust. */
> tcg_out32(s, insn | I3312_TO_I3310 | regoff << 16 |
> - 0x4000 | ext << 13 | base << 5 | (rd & 0x1f));
> + option << 13 | base << 5 | (rd & 0x1f));
> }
>
> static void tcg_out_insn_3312(TCGContext *s, AArch64Insn insn,
> @@ -1124,7 +1125,7 @@ static void tcg_out_ldst(TCGContext *s, AArch64Insn insn, TCGReg rd,
>
> /* Worst-case scenario, move offset to temp register, use reg offset. */
> tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_TMP, offset);
> - tcg_out_ldst_r(s, insn, rd, rn, TCG_TYPE_I64, TCG_REG_TMP);
> + tcg_out_ldst_r(s, insn, rd, rn, 3 /* LSL #0 */, TCG_REG_TMP);
> }
>
> static bool tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
> @@ -1718,34 +1719,34 @@ static void tcg_out_tlb_read(TCGContext *s, TCGReg addr_reg, MemOp opc,
>
> static void tcg_out_qemu_ld_direct(TCGContext *s, MemOp memop, TCGType ext,
> TCGReg data_r, TCGReg addr_r,
> - TCGType otype, TCGReg off_r)
> + int option, TCGReg off_r)
> {
> /* Byte swapping is left to middle-end expansion. */
> tcg_debug_assert((memop & MO_BSWAP) == 0);
>
> switch (memop & MO_SSIZE) {
> case MO_UB:
> - tcg_out_ldst_r(s, I3312_LDRB, data_r, addr_r, otype, off_r);
> + tcg_out_ldst_r(s, I3312_LDRB, data_r, addr_r, option, off_r);
> break;
> case MO_SB:
> tcg_out_ldst_r(s, ext ? I3312_LDRSBX : I3312_LDRSBW,
> - data_r, addr_r, otype, off_r);
> + data_r, addr_r, option, off_r);
> break;
> case MO_UW:
> - tcg_out_ldst_r(s, I3312_LDRH, data_r, addr_r, otype, off_r);
> + tcg_out_ldst_r(s, I3312_LDRH, data_r, addr_r, option, off_r);
> break;
> case MO_SW:
> tcg_out_ldst_r(s, (ext ? I3312_LDRSHX : I3312_LDRSHW),
> - data_r, addr_r, otype, off_r);
> + data_r, addr_r, option, off_r);
> break;
> case MO_UL:
> - tcg_out_ldst_r(s, I3312_LDRW, data_r, addr_r, otype, off_r);
> + tcg_out_ldst_r(s, I3312_LDRW, data_r, addr_r, option, off_r);
> break;
> case MO_SL:
> - tcg_out_ldst_r(s, I3312_LDRSWX, data_r, addr_r, otype, off_r);
> + tcg_out_ldst_r(s, I3312_LDRSWX, data_r, addr_r, option, off_r);
> break;
> case MO_Q:
> - tcg_out_ldst_r(s, I3312_LDRX, data_r, addr_r, otype, off_r);
> + tcg_out_ldst_r(s, I3312_LDRX, data_r, addr_r, option, off_r);
> break;
> default:
> tcg_abort();
> @@ -1754,50 +1755,68 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, MemOp memop, TCGType ext,
>
> static void tcg_out_qemu_st_direct(TCGContext *s, MemOp memop,
> TCGReg data_r, TCGReg addr_r,
> - TCGType otype, TCGReg off_r)
> + int option, TCGReg off_r)
> {
> /* Byte swapping is left to middle-end expansion. */
> tcg_debug_assert((memop & MO_BSWAP) == 0);
>
> switch (memop & MO_SIZE) {
> case MO_8:
> - tcg_out_ldst_r(s, I3312_STRB, data_r, addr_r, otype, off_r);
> + tcg_out_ldst_r(s, I3312_STRB, data_r, addr_r, option, off_r);
> break;
> case MO_16:
> - tcg_out_ldst_r(s, I3312_STRH, data_r, addr_r, otype, off_r);
> + tcg_out_ldst_r(s, I3312_STRH, data_r, addr_r, option, off_r);
> break;
> case MO_32:
> - tcg_out_ldst_r(s, I3312_STRW, data_r, addr_r, otype, off_r);
> + tcg_out_ldst_r(s, I3312_STRW, data_r, addr_r, option, off_r);
> break;
> case MO_64:
> - tcg_out_ldst_r(s, I3312_STRX, data_r, addr_r, otype, off_r);
> + tcg_out_ldst_r(s, I3312_STRX, data_r, addr_r, option, off_r);
> break;
> default:
> tcg_abort();
> }
> }
>
> +static int guest_ext_option(void)
> +{
> +#ifdef CONFIG_USER_ONLY
> + bool signed_addr32 = guest_base_signed_addr32;
> +#else
> + bool signed_addr32 = TCG_TARGET_SIGNED_ADDR32;
> +#endif
> +
> + if (TARGET_LONG_BITS == 64) {
> + return 3; /* LSL #0 */
> + } else if (signed_addr32) {
> + return 6; /* SXTW */
> + } else {
> + return 2; /* UXTW */
> + }
> +}
If this is is going to be a magic number we pass into our code
generation we could at least wrap it in a confined enum rather than a
bare int we chuck around.
> +
> static void tcg_out_qemu_ld(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
> MemOpIdx oi, TCGType ext)
> {
> MemOp memop = get_memop(oi);
> - const TCGType otype = TARGET_LONG_BITS == 64 ? TCG_TYPE_I64 : TCG_TYPE_I32;
> + int option = guest_ext_option();
> +
> #ifdef CONFIG_SOFTMMU
> unsigned mem_index = get_mmuidx(oi);
> tcg_insn_unit *label_ptr;
>
> tcg_out_tlb_read(s, addr_reg, memop, &label_ptr, mem_index, 1);
> tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
> - TCG_REG_X1, otype, addr_reg);
> + TCG_REG_X1, option, addr_reg);
> add_qemu_ldst_label(s, true, oi, ext, data_reg, addr_reg,
> s->code_ptr, label_ptr);
> #else /* !CONFIG_SOFTMMU */
> if (USE_GUEST_BASE) {
> tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
> - TCG_REG_GUEST_BASE, otype, addr_reg);
> + TCG_REG_GUEST_BASE, option, addr_reg);
> } else {
> tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
> - addr_reg, TCG_TYPE_I64, TCG_REG_XZR);
> + addr_reg, option, TCG_REG_XZR);
> }
> #endif /* CONFIG_SOFTMMU */
> }
> @@ -1806,23 +1825,24 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
> MemOpIdx oi)
> {
> MemOp memop = get_memop(oi);
> - const TCGType otype = TARGET_LONG_BITS == 64 ? TCG_TYPE_I64 : TCG_TYPE_I32;
> + int option = guest_ext_option();
> +
> #ifdef CONFIG_SOFTMMU
> unsigned mem_index = get_mmuidx(oi);
> tcg_insn_unit *label_ptr;
>
> tcg_out_tlb_read(s, addr_reg, memop, &label_ptr, mem_index, 0);
> tcg_out_qemu_st_direct(s, memop, data_reg,
> - TCG_REG_X1, otype, addr_reg);
> + TCG_REG_X1, option, addr_reg);
> add_qemu_ldst_label(s, false, oi, (memop & MO_SIZE)== MO_64,
> data_reg, addr_reg, s->code_ptr, label_ptr);
> #else /* !CONFIG_SOFTMMU */
> if (USE_GUEST_BASE) {
> tcg_out_qemu_st_direct(s, memop, data_reg,
> - TCG_REG_GUEST_BASE, otype, addr_reg);
> + TCG_REG_GUEST_BASE, option, addr_reg);
> } else {
> tcg_out_qemu_st_direct(s, memop, data_reg,
> - addr_reg, TCG_TYPE_I64, TCG_REG_XZR);
> + addr_reg, option, TCG_REG_XZR);
> }
> #endif /* CONFIG_SOFTMMU */
> }
--
Alex Bennée
next prev parent reply other threads:[~2021-10-11 10:32 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-10 17:43 [PATCH 0/8] tcg: support 32-bit guest addresses as signed Richard Henderson
2021-10-10 17:43 ` [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32 Richard Henderson
2021-10-11 4:21 ` WANG Xuerui
2021-10-11 9:55 ` Alex Bennée
2021-10-11 22:07 ` Philippe Mathieu-Daudé
2021-10-11 23:16 ` Alistair Francis
2021-10-10 17:43 ` [PATCH 2/8] accel/tcg: Split out g2h_tlbe Richard Henderson
2021-10-11 4:22 ` WANG Xuerui
2021-10-11 9:55 ` Alex Bennée
2021-10-11 21:48 ` Philippe Mathieu-Daudé
2021-10-11 23:19 ` Alistair Francis
2021-10-10 17:43 ` [PATCH 3/8] accel/tcg: Support TCG_TARGET_SIGNED_ADDR32 for softmmu Richard Henderson
2021-10-11 4:30 ` WANG Xuerui
2021-10-11 15:27 ` Richard Henderson
2021-10-10 17:43 ` [PATCH 4/8] accel/tcg: Add guest_base_signed_addr32 for user-only Richard Henderson
2021-10-11 22:06 ` Philippe Mathieu-Daudé
2021-10-13 7:07 ` Alistair Francis
2021-10-10 17:43 ` [PATCH 5/8] linux-user: Support TCG_TARGET_SIGNED_ADDR32 Richard Henderson
2021-10-11 10:22 ` Alex Bennée
2021-10-11 15:32 ` Richard Henderson
2021-10-10 17:43 ` [PATCH 6/8] tcg/aarch64: " Richard Henderson
2021-10-11 10:28 ` Alex Bennée [this message]
2021-10-11 15:24 ` Richard Henderson
2021-10-13 21:05 ` Richard Henderson
2021-10-10 17:44 ` [PATCH 7/8] target/mips: " Richard Henderson
2021-10-11 4:20 ` WANG Xuerui
2021-10-13 22:24 ` Richard Henderson
2021-10-10 17:44 ` [PATCH 8/8] target/riscv: " Richard Henderson
2021-10-11 22:00 ` Philippe Mathieu-Daudé
2021-10-13 7:08 ` Alistair Francis
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=877dejyhrb.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=Alistair.Francis@wdc.com \
--cc=f4bug@amsat.org \
--cc=git@xen0n.name \
--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.