Re: [PATCH] target/riscv: add support for zmmul extension v0.1

[Weiwei Li]

在 2022/5/24 上午5:15, Alistair Francis 写道:
On Mon, May 23, 2022 at 6:10 PM Weiwei Li <liwei...@iscas.ac.cn> wrote:

在 2022/5/23 下午2:34, Alistair Francis 写道:
On Wed, May 18, 2022 at 11:54 AM Weiwei Li <liwei...@iscas.ac.cn> wrote:
   - includes all multiplication operations for M extension

Signed-off-by: Weiwei Li <liwei...@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqi...@iscas.ac.cn>
---
   target/riscv/cpu.c                      |  2 ++
   target/riscv/cpu.h                      |  1 +
   target/riscv/insn_trans/trans_rvm.c.inc | 18 ++++++++++++------
   3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e373c61ba2..01b57d3784 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -903,6 +903,7 @@ static Property riscv_cpu_properties[] = {

       /* These are experimental so mark with 'x-' */
       DEFINE_PROP_BOOL("x-j", RISCVCPU, cfg.ext_j, false),
+    DEFINE_PROP_BOOL("x-zmmul", RISCVCPU, cfg.ext_zmmul, false),
Is this really experimental?

Alistair
I think it's experimental currently. The zmmul version in latest riscv
spec is v0.1, even though described as  v1.0 in spike README.
Hmm... Your right that it is only v0.1, but there is no indication of
draft state in the RISC-V spec chapter on Zmmul

Its specification status
(https://wiki.riscv.org/display/home/specification+status) is Freeze
Complete and TSC Sign-Off Voting.

And It's not in the ratified extension
list(https://wiki.riscv.org/display/home/recently+ratified+extensions).

Any status update I missed?
Confusing. Ok, I guess let's leave it as experimental, we can always
remove the `x-` easily :)

Regards,

Weiwei Li

       /* ePMP 0.9.3 */
       DEFINE_PROP_BOOL("x-epmp", RISCVCPU, cfg.epmp, false),
       DEFINE_PROP_BOOL("x-aia", RISCVCPU, cfg.aia, false),
@@ -1027,6 +1028,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char 
**isa_str, int max_str_len)
        *    extensions by an underscore.
        */
       struct isa_ext_data isa_edata_arr[] = {
+        ISA_EDATA_ENTRY(zmmul, ext_zmmul),
We should have some checks though. We don't want users to enable this
and the multiply (M) extension


Alistair

Maybe we can add a check like this:

/* M + Zmmul = Zmmul */
if (cpu->cfg.ext_m && cpu->cfg.ext_zmmul) {
warn_report("Zmmul will override M");
cpu->cfg.ext_m = false;
}

It seems OK to enable both M and Zmmul in gnu toolchain. However, divide 
operations

will be disabled when Zmmul is enabled.

Regards,

Weiwei Li

           ISA_EDATA_ENTRY(zfh, ext_zfh),
           ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
           ISA_EDATA_ENTRY(zfinx, ext_zfinx),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f5ff7294c6..68177eae12 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -405,6 +405,7 @@ struct RISCVCPUConfig {
       bool ext_zhinxmin;
       bool ext_zve32f;
       bool ext_zve64f;
+    bool ext_zmmul;

       uint32_t mvendorid;
       uint64_t marchid;
diff --git a/target/riscv/insn_trans/trans_rvm.c.inc 
b/target/riscv/insn_trans/trans_rvm.c.inc
index 16b029edf0..ec7f705aab 100644
--- a/target/riscv/insn_trans/trans_rvm.c.inc
+++ b/target/riscv/insn_trans/trans_rvm.c.inc
@@ -18,6 +18,12 @@
    * this program.  If not, see <http://www.gnu.org/licenses/>.
    */

+#define REQUIRE_M_OR_ZMMUL(ctx) do {                      \
+    if (!ctx->cfg_ptr->ext_zmmul && !has_ext(ctx, RVM)) { \
+        return false;                                     \
+    }                                                     \
+} while (0)
+
   static void gen_mulhu_i128(TCGv r2, TCGv r3, TCGv al, TCGv ah, TCGv bl, TCGv 
bh)
   {
       TCGv tmpl = tcg_temp_new();
@@ -65,7 +71,7 @@ static void gen_mul_i128(TCGv rl, TCGv rh,

   static bool trans_mul(DisasContext *ctx, arg_mul *a)
   {
-    REQUIRE_EXT(ctx, RVM);
+    REQUIRE_M_OR_ZMMUL(ctx);
       return gen_arith(ctx, a, EXT_NONE, tcg_gen_mul_tl, gen_mul_i128);
   }

@@ -109,7 +115,7 @@ static void gen_mulh_w(TCGv ret, TCGv s1, TCGv s2)

   static bool trans_mulh(DisasContext *ctx, arg_mulh *a)
   {
-    REQUIRE_EXT(ctx, RVM);
+    REQUIRE_M_OR_ZMMUL(ctx);
       return gen_arith_per_ol(ctx, a, EXT_SIGN, gen_mulh, gen_mulh_w,
                               gen_mulh_i128);
   }
@@ -161,7 +167,7 @@ static void gen_mulhsu_w(TCGv ret, TCGv arg1, TCGv arg2)

   static bool trans_mulhsu(DisasContext *ctx, arg_mulhsu *a)
   {
-    REQUIRE_EXT(ctx, RVM);
+    REQUIRE_M_OR_ZMMUL(ctx);
       return gen_arith_per_ol(ctx, a, EXT_NONE, gen_mulhsu, gen_mulhsu_w,
                               gen_mulhsu_i128);
   }
@@ -176,7 +182,7 @@ static void gen_mulhu(TCGv ret, TCGv s1, TCGv s2)

   static bool trans_mulhu(DisasContext *ctx, arg_mulhu *a)
   {
-    REQUIRE_EXT(ctx, RVM);
+    REQUIRE_M_OR_ZMMUL(ctx);
       /* gen_mulh_w works for either sign as input. */
       return gen_arith_per_ol(ctx, a, EXT_ZERO, gen_mulhu, gen_mulh_w,
                               gen_mulhu_i128);
@@ -349,7 +355,7 @@ static bool trans_remu(DisasContext *ctx, arg_remu *a)
   static bool trans_mulw(DisasContext *ctx, arg_mulw *a)
   {
       REQUIRE_64_OR_128BIT(ctx);
-    REQUIRE_EXT(ctx, RVM);
+    REQUIRE_M_OR_ZMMUL(ctx);
       ctx->ol = MXL_RV32;
       return gen_arith(ctx, a, EXT_NONE, tcg_gen_mul_tl, NULL);
   }
@@ -389,7 +395,7 @@ static bool trans_remuw(DisasContext *ctx, arg_remuw *a)
   static bool trans_muld(DisasContext *ctx, arg_muld *a)
   {
       REQUIRE_128BIT(ctx);
-    REQUIRE_EXT(ctx, RVM);
+    REQUIRE_M_OR_ZMMUL(ctx);
       ctx->ol = MXL_RV64;
       return gen_arith(ctx, a, EXT_SIGN, tcg_gen_mul_tl, NULL);
   }
--
2.17.1