Re: [PATCH v1 16/19] target/arm: Relax ordered/atomic alignment checks for LSE2

[Peter Maydell]

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).

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.)

> +    tcg_temp_free_i32(tmp);
> +
> +    addr = tcg_temp_new_i64();
> +    tcg_gen_addi_i64(addr, cpu_reg_sp(s, rn), imm);
> +
> +    type = is_write ? MMU_DATA_STORE : MMU_DATA_LOAD,
> +    mmu_idx = get_mem_index(s);
> +    gen_helper_unaligned_access(cpu_env, addr, tcg_constant_i32(type),
> +                                tcg_constant_i32(mmu_idx));
> +    tcg_temp_free_i64(addr);
> +
> +    gen_set_label(over_label);
> +
> +}

thanks
-- PMM