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

[Alistair Francis]

On Mon, May 23, 2022 at 6:10 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
>
> 在 2022/5/23 下午2:34, Alistair Francis 写道:
> > On Wed, May 18, 2022 at 11:54 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
> >>   - includes all multiplication operations for M extension
> >>
> >> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> >> Signed-off-by: Junqiang Wang <wangjunqiang@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

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