From: Richard Henderson <richard.henderson@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org, qemu-arm@nongnu.org
Subject: Re: [PATCH v1 16/19] target/arm: Relax ordered/atomic alignment checks for LSE2
Date: Thu, 23 Feb 2023 07:16:38 -1000 [thread overview]
Message-ID: <835a509d-caa4-9cf7-88f5-d7f3705dd646@linaro.org> (raw)
In-Reply-To: <CAFEAcA_6_Om4hGBB5=pFVRzOiE-PndYU9TAFGRj1s7=s9-C0qQ@mail.gmail.com>
On 2/23/23 06:49, Peter Maydell wrote:
> On Thu, 16 Feb 2023 at 03:09, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> FEAT_LSE2 only requires that atomic operations not cross a
>> 16-byte boundary. Ordered operations may be completely
>> unaligned if SCTLR.nAA is set.
>>
>> Because this alignment check is so special, do it by hand.
>> Make sure not to keep TCG temps live across the branch.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
>
>> +static void check_lse2_align(DisasContext *s, int rn, int imm,
>> + bool is_write, MemOp mop)
>> +{
>> + TCGv_i32 tmp;
>> + TCGv_i64 addr;
>> + TCGLabel *over_label;
>> + MMUAccessType type;
>> + int mmu_idx;
>> +
>> + tmp = tcg_temp_new_i32();
>> + tcg_gen_extrl_i64_i32(tmp, cpu_reg_sp(s, rn));
>> + tcg_gen_addi_i32(tmp, tmp, imm & 15);
>> + tcg_gen_andi_i32(tmp, tmp, 15);
>> + tcg_gen_addi_i32(tmp, tmp, memop_size(mop));
>> +
>> + over_label = gen_new_label();
>> + tcg_gen_brcond_i32(TCG_COND_LEU, tmp, tcg_constant_i32(16), over_label);
>
> This brcond ends the basic block and destroys the content
> of TCG temporaries, which is bad because some of the
> callsites have set some of those up before calling this
> function (eg gen_compare_and_swap() has called cpu_reg()
> which might have created and initialized a temporary
> for xZR).
xzr uses tcg_constant_i64(), which has no lifetime issues.
>
> Using a brcond anywhere other than directly in a top level
> function where you can see it and work around this awkward
> property seems rather fragile.
>
> (Ideally there would be a variant of brcond that didn't
> trash temporaries, because almost all the time that is
> an annoying hazard rather than a useful property.)
I've cc'd you on a patch set that fixes all the temporary lifetime stuff.
v1: https://patchew.org/QEMU/20230130205935.1157347-1-richard.henderson@linaro.org/
v2: https://patchew.org/QEMU/20230222232715.15034-1-richard.henderson@linaro.org/
r~
next prev parent reply other threads:[~2023-02-23 17:17 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-16 3:08 [PATCH v1 00/19] target/arm: Implement FEAT_LSE2 Richard Henderson
2023-02-16 3:08 ` [PATCH v1 01/19] target/arm: Make cpu_exclusive_high hold the high bits Richard Henderson
2023-02-23 15:14 ` Peter Maydell
2023-02-23 16:12 ` Richard Henderson
2023-02-23 16:51 ` Peter Maydell
2023-02-23 17:08 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 02/19] target/arm: Use tcg_gen_qemu_ld_i128 for LDXP Richard Henderson
2023-02-16 3:08 ` [PATCH v1 03/19] target/arm: Use tcg_gen_qemu_{st, ld}_i128 for do_fp_{st, ld} Richard Henderson
2023-02-23 15:23 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 04/19] target/arm: Use tcg_gen_qemu_st_i128 for STZG, STZ2G Richard Henderson
2023-02-23 15:24 ` Peter Maydell
2023-02-23 16:20 ` Richard Henderson
2023-02-23 16:53 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 05/19] target/arm: Use tcg_gen_qemu_{ld, st}_i128 in gen_sve_{ld, st}r Richard Henderson
2023-02-23 15:36 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 06/19] target/arm: Sink gen_mte_check1 into load/store_exclusive Richard Henderson
2023-02-23 15:40 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 07/19] target/arm: Add feature test for FEAT_LSE2 Richard Henderson
2023-02-23 15:43 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 08/19] target/arm: Add atom_data to DisasContext Richard Henderson
2023-02-23 15:47 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 09/19] target/arm: Load/store integer pair with one tcg operation Richard Henderson
2023-02-23 15:57 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 10/19] target/arm: Hoist finalize_memop out of do_gpr_{ld, st} Richard Henderson
2023-02-23 16:03 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 11/19] target/arm: Hoist finalize_memop out of do_fp_{ld, st} Richard Henderson
2023-02-23 16:04 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 12/19] target/arm: Pass memop to gen_mte_check1* Richard Henderson
2023-02-23 16:08 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 13/19] target/arm: Pass single_memop to gen_mte_checkN Richard Henderson
2023-02-23 16:10 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 14/19] target/arm: Check alignment in helper_mte_check Richard Henderson
2023-02-23 16:28 ` Peter Maydell
2023-02-23 16:38 ` Richard Henderson
2023-02-23 16:54 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 15/19] target/arm: Add SCTLR.nAA to TBFLAG_A64 Richard Henderson
2023-02-23 16:32 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 16/19] target/arm: Relax ordered/atomic alignment checks for LSE2 Richard Henderson
2023-02-23 16:49 ` Peter Maydell
2023-02-23 17:16 ` Richard Henderson [this message]
2023-02-23 17:22 ` Peter Maydell
2023-02-23 17:27 ` Richard Henderson
2023-02-16 3:08 ` [PATCH v1 17/19] target/arm: Move mte check for store-exclusive Richard Henderson
2023-02-23 16:36 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 18/19] test/tcg/multiarch: Adjust sigbus.c Richard Henderson
2023-02-23 16:36 ` Peter Maydell
2023-02-16 3:08 ` [PATCH v1 19/19] target/arm: Enable FEAT_LSE2 for -cpu max Richard Henderson
2023-02-23 16:37 ` Peter Maydell
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=835a509d-caa4-9cf7-88f5-d7f3705dd646@linaro.org \
--to=richard.henderson@linaro.org \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).