target/riscv: Centralize MO_TE uses in a pair of helpers

[PATCH 10/13] target/riscv: Conceal MO_TE|MO_ALIGN within gen_lr() / gen_sc()

Posted by Philippe Mathieu-Daudé 1 month ago

All callers of gen_lr() / gen_sc() set the MO_TE and MO_ALIGN flags.
Set them once in the callees.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/riscv/insn_trans/trans_rva.c.inc | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
index e0fbfafdde4..8737e8d60d1 100644
--- a/target/riscv/insn_trans/trans_rva.c.inc
+++ b/target/riscv/insn_trans/trans_rva.c.inc
@@ -34,6 +34,9 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
 {
     TCGv src1;
 
+    mop |= MO_ALIGN;
+    mop |= MO_TE;
+
     decode_save_opc(ctx, 0);
     src1 = get_address(ctx, a->rs1, 0);
     if (a->rl) {
@@ -61,6 +64,9 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
     TCGLabel *l1 = gen_new_label();
     TCGLabel *l2 = gen_new_label();
 
+    mop |= MO_ALIGN;
+    mop |= MO_TE;
+
     decode_save_opc(ctx, 0);
     src1 = get_address(ctx, a->rs1, 0);
     tcg_gen_brcond_tl(TCG_COND_NE, load_res, src1, l1);
@@ -99,13 +105,13 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
 static bool trans_lr_w(DisasContext *ctx, arg_lr_w *a)
 {
     REQUIRE_A_OR_ZALRSC(ctx);
-    return gen_lr(ctx, a, (MO_ALIGN | MO_TE | MO_SL));
+    return gen_lr(ctx, a, MO_SL);
 }
 
 static bool trans_sc_w(DisasContext *ctx, arg_sc_w *a)
 {
     REQUIRE_A_OR_ZALRSC(ctx);
-    return gen_sc(ctx, a, (MO_ALIGN | MO_TE | MO_SL));
+    return gen_sc(ctx, a, MO_SL);
 }
 
 static bool trans_amoswap_w(DisasContext *ctx, arg_amoswap_w *a)
@@ -166,14 +172,14 @@ static bool trans_lr_d(DisasContext *ctx, arg_lr_d *a)
 {
     REQUIRE_64BIT(ctx);
     REQUIRE_A_OR_ZALRSC(ctx);
-    return gen_lr(ctx, a, MO_ALIGN | MO_TE | MO_UQ);
+    return gen_lr(ctx, a, MO_UQ);
 }
 
 static bool trans_sc_d(DisasContext *ctx, arg_sc_d *a)
 {
     REQUIRE_64BIT(ctx);
     REQUIRE_A_OR_ZALRSC(ctx);
-    return gen_sc(ctx, a, (MO_ALIGN | MO_TE | MO_UQ));
+    return gen_sc(ctx, a, MO_UQ);
 }
 
 static bool trans_amoswap_d(DisasContext *ctx, arg_amoswap_d *a)
-- 
2.51.0

Re: [PATCH 10/13] target/riscv: Conceal MO_TE|MO_ALIGN within gen_lr() / gen_sc()

Posted by Alistair Francis 1 month ago

On Sat, Oct 11, 2025 at 1:58 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> All callers of gen_lr() / gen_sc() set the MO_TE and MO_ALIGN flags.
> Set them once in the callees.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/insn_trans/trans_rva.c.inc | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
> index e0fbfafdde4..8737e8d60d1 100644
> --- a/target/riscv/insn_trans/trans_rva.c.inc
> +++ b/target/riscv/insn_trans/trans_rva.c.inc
> @@ -34,6 +34,9 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
>  {
>      TCGv src1;
>
> +    mop |= MO_ALIGN;
> +    mop |= MO_TE;
> +
>      decode_save_opc(ctx, 0);
>      src1 = get_address(ctx, a->rs1, 0);
>      if (a->rl) {
> @@ -61,6 +64,9 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
>      TCGLabel *l1 = gen_new_label();
>      TCGLabel *l2 = gen_new_label();
>
> +    mop |= MO_ALIGN;
> +    mop |= MO_TE;
> +
>      decode_save_opc(ctx, 0);
>      src1 = get_address(ctx, a->rs1, 0);
>      tcg_gen_brcond_tl(TCG_COND_NE, load_res, src1, l1);
> @@ -99,13 +105,13 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
>  static bool trans_lr_w(DisasContext *ctx, arg_lr_w *a)
>  {
>      REQUIRE_A_OR_ZALRSC(ctx);
> -    return gen_lr(ctx, a, (MO_ALIGN | MO_TE | MO_SL));
> +    return gen_lr(ctx, a, MO_SL);
>  }
>
>  static bool trans_sc_w(DisasContext *ctx, arg_sc_w *a)
>  {
>      REQUIRE_A_OR_ZALRSC(ctx);
> -    return gen_sc(ctx, a, (MO_ALIGN | MO_TE | MO_SL));
> +    return gen_sc(ctx, a, MO_SL);
>  }
>
>  static bool trans_amoswap_w(DisasContext *ctx, arg_amoswap_w *a)
> @@ -166,14 +172,14 @@ static bool trans_lr_d(DisasContext *ctx, arg_lr_d *a)
>  {
>      REQUIRE_64BIT(ctx);
>      REQUIRE_A_OR_ZALRSC(ctx);
> -    return gen_lr(ctx, a, MO_ALIGN | MO_TE | MO_UQ);
> +    return gen_lr(ctx, a, MO_UQ);
>  }
>
>  static bool trans_sc_d(DisasContext *ctx, arg_sc_d *a)
>  {
>      REQUIRE_64BIT(ctx);
>      REQUIRE_A_OR_ZALRSC(ctx);
> -    return gen_sc(ctx, a, (MO_ALIGN | MO_TE | MO_UQ));
> +    return gen_sc(ctx, a, MO_UQ);
>  }
>
>  static bool trans_amoswap_d(DisasContext *ctx, arg_amoswap_d *a)
> --
> 2.51.0
>
>

Re: [PATCH 10/13] target/riscv: Conceal MO_TE|MO_ALIGN within gen_lr() / gen_sc()

Posted by Richard Henderson 1 month ago

On 10/10/25 08:50, Philippe Mathieu-Daudé wrote:
> All callers of gen_lr() / gen_sc() set the MO_TE and MO_ALIGN flags.
> Set them once in the callees.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/riscv/insn_trans/trans_rva.c.inc | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~