[v4,1/2] target/riscv: refactor Zicond support

Commit Message

Philipp Tomsich March 6, 2023, 3:23 p.m. UTC

After the original Zicond support was stuck/fell through the cracks on
the mailing list at v3 (and a different implementation was merged in
the meanwhile), we need to refactor Zicond to prepare it to be reused
by XVentanaCondOps.

This commit lifts the common logic out into gen_czero and uses this
via gen_logic and 2 helper functions (effectively partial closures).

Signed-off-by: Philipp Tomsich <[e-mail]>
---

Changes in v4:
- rebase onto master

Changes in v3:
- don't add this to MAINTAINERS, as it is an official extension

Changes in v2:
- gates availability of the instructions through a REQUIRE_ZICOND
  macro (these were previously always enabled)

 target/riscv/cpu.c                           |  3 --
 target/riscv/insn_trans/trans_rvzicond.c.inc | 36 ++++++++++++--------
 2 files changed, 21 insertions(+), 18 deletions(-)

Comments

Richard Henderson March 6, 2023, 6:24 p.m. UTC | #1

On 3/6/23 07:23, Philipp Tomsich wrote:
> After the original Zicond support was stuck/fell through the cracks on
> the mailing list at v3 (and a different implementation was merged in
> the meanwhile), we need to refactor Zicond to prepare it to be reused
> by XVentanaCondOps.
> 
> This commit lifts the common logic out into gen_czero and uses this
> via gen_logic and 2 helper functions (effectively partial closures).
> 
> Signed-off-by: Philipp Tomsich <[e-mail]>

For any multi-patch series, you should use a cover letter.
Lacking this causes the set to be missed by tooling.

> -static bool trans_czero_eqz(DisasContext *ctx, arg_czero_eqz *a)
> +/* Emits "$rd = ($rs2 <cond> $zero) ? $zero : $rs1" */
> +static inline void gen_czero(TCGv dest, TCGv src1, TCGv src2, TCGCond cond)

Drop the inline markers and let the compiler choose.

> +static inline void gen_czero_eqz(TCGv dest, TCGv src1, TCGv src2)
> +static inline void gen_czero_nez(TCGv dest, TCGv src1, TCGv src2)

These especially, where we use their function pointer...

> +    return gen_logic(ctx, a, gen_czero_eqz);
> +    return gen_logic(ctx, a, gen_czero_nez);

here, so they will most definitely exist out-of-line.

Otherwise,
Reviewed-by: Richard Henderson <[e-mail]>


r~

Patch

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 5bc0005cc7..88c2484eee 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1191,9 +1191,6 @@  static Property riscv_cpu_extensions[] = {
     DEFINE_PROP_BOOL("x-smaia", RISCVCPU, cfg.ext_smaia, false),
     DEFINE_PROP_BOOL("x-ssaia", RISCVCPU, cfg.ext_ssaia, false),
 
-    DEFINE_PROP_BOOL("x-zvfh", RISCVCPU, cfg.ext_zvfh, false),
-    DEFINE_PROP_BOOL("x-zvfhmin", RISCVCPU, cfg.ext_zvfhmin, false),
-
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/target/riscv/insn_trans/trans_rvzicond.c.inc b/target/riscv/insn_trans/trans_rvzicond.c.inc
index 645260164e..b35cb4cbc7 100644
--- a/target/riscv/insn_trans/trans_rvzicond.c.inc
+++ b/target/riscv/insn_trans/trans_rvzicond.c.inc
@@ -2,6 +2,7 @@ 
  * RISC-V translation routines for the Zicond Standard Extension.
  *
  * Copyright (c) 2020-2023 PLCT Lab
+ * Copyright (c) 2022 VRULL GmbH.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -22,28 +23,33 @@ 
     }                                     \
 } while (0)
 
-static bool trans_czero_eqz(DisasContext *ctx, arg_czero_eqz *a)
+/* Emits "$rd = ($rs2 <cond> $zero) ? $zero : $rs1" */
+static inline void gen_czero(TCGv dest, TCGv src1, TCGv src2, TCGCond cond)
 {
-    REQUIRE_ZICOND(ctx);
+    TCGv zero = tcg_constant_tl(0);
+    tcg_gen_movcond_tl(cond, dest, src2, zero, zero, src1);
+}
 
-    TCGv dest = dest_gpr(ctx, a->rd);
-    TCGv src1 = get_gpr(ctx, a->rs1, EXT_NONE);
-    TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
+static inline void gen_czero_eqz(TCGv dest, TCGv src1, TCGv src2)
+{
+    gen_czero(dest, src1, src2, TCG_COND_EQ);
+}
 
-    tcg_gen_movcond_tl(TCG_COND_EQ, dest, src2, ctx->zero, ctx->zero, src1);
-    gen_set_gpr(ctx, a->rd, dest);
-    return true;
+static inline void gen_czero_nez(TCGv dest, TCGv src1, TCGv src2)
+{
+    gen_czero(dest, src1, src2, TCG_COND_NE);
 }
 
-static bool trans_czero_nez(DisasContext *ctx, arg_czero_nez *a)
+static bool trans_czero_eqz(DisasContext *ctx, arg_r *a)
 {
     REQUIRE_ZICOND(ctx);
 
-    TCGv dest = dest_gpr(ctx, a->rd);
-    TCGv src1 = get_gpr(ctx, a->rs1, EXT_NONE);
-    TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
+    return gen_logic(ctx, a, gen_czero_eqz);
+}
+
+static bool trans_czero_nez(DisasContext *ctx, arg_r *a)
+{
+    REQUIRE_ZICOND(ctx);
 
-    tcg_gen_movcond_tl(TCG_COND_NE, dest, src2, ctx->zero, ctx->zero, src1);
-    gen_set_gpr(ctx, a->rd, dest);
-    return true;
+    return gen_logic(ctx, a, gen_czero_nez);
 }