[PATCH] RISC-V: Support Zmmul extension

[Tsukasa OI]

廖先生,

I submitted a patchset for pretty much the same purpose:
<https://sourceware.org/pipermail/binutils/2022-July/121685.html>
but my testcase part could coexist with yours.

Note that my and your patchset has a key technical difference:
-   In Tsukasa OI's patch, "M" implies "Zmmul" but
-   in LIAO Shihua's patch, it does not.

I have no (or a little) preference here as long as you clean the patch a
bit.



On 2022/07/11 16:30, shihua@iscas.ac.cn wrote:
> From: LiaoShihua <shihua@iscas.ac.cn>
> 
>     Zmmul extension is Multiply only extension for RISC-V.It implements the multiplication subset of the M extension. 
>     The encodings are identical to those of the corresponding M-extension instructions.
> 
> bfd\ChangeLog:

A backslash seems odd here.  Just for curiosity, is it what you get when
you run contrib/mklog.py on Windows?

> 
>         * elfxx-riscv.c (riscv_multi_subset_supports):Add support for Zmmul extension
>         (riscv_multi_subset_supports_ext):Ditto.
> 
> include\ChangeLog:
> 
>         * opcode/riscv.h (enum riscv_insn_class):Ditto.
> 
> opcodes\ChangeLog:
> 
>         * riscv-opc.c:Ditto.
> ---
>  bfd/elfxx-riscv.c      |  6 ++++++
>  include/opcode/riscv.h |  1 +
>  opcodes/riscv-opc.c    | 10 +++++-----
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index bf7dc20e892..3c9c961352a 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -1226,6 +1226,7 @@ static struct riscv_supported_ext riscv_supported_std_z_ext[] =
>    {"zvl16384b",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
>    {"zvl32768b",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
>    {"zvl65536b",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
> +  {"zmmul",		  ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },

Although complying the rule:

> /* The standard extensions must be added in canonical order.  */

is not strictly necessary (in fact, some Zvl* extensions are not
canonically ordered), it would be good to have added in pretty much
canonical order (put "zmmul" after "zihintpause").

>    {NULL, 0, 0, 0, 0}
>  };
>  
> @@ -2395,6 +2396,9 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
>        return riscv_subset_supports (rps, "svinval");
>      case INSN_CLASS_H:
>        return riscv_subset_supports (rps, "h");
> +    case INSN_CLASS_M_OR_ZMMUL:
> +      return (riscv_subset_supports (rps, "m")
> +	      || riscv_subset_supports (rps, "zmmul"));

How about putting INSN_CLASS_M_OR_ZMMUL just after INSN_CLASS_M?

>      default:
>        rps->error_handler
>          (_("internal: unreachable INSN_CLASS_*"));
> @@ -2500,6 +2504,8 @@ riscv_multi_subset_supports_ext (riscv_parse_subset_t *rps,
>        return _("('q' and 'zfh') or 'zhinx");
>      case INSN_CLASS_H:
>        return _("h");
> +    case INSN_CLASS_M_OR_ZMMUL:
> +      return _("m' or `zmmul");

Likewise.

>      default:
>        rps->error_handler
>          (_("internal: unreachable INSN_CLASS_*"));
> diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> index 808f05f3d7a..0021b7434ea 100644
> --- a/include/opcode/riscv.h
> +++ b/include/opcode/riscv.h
> @@ -396,6 +396,7 @@ enum riscv_insn_class
>    INSN_CLASS_ZICBOP,
>    INSN_CLASS_ZICBOZ,
>    INSN_CLASS_H,
> +  INSN_CLASS_M_OR_ZMMUL,

Because switch case ordering above is not the same as riscv_insn_class
ordering, we would have multiple candidates.  I chose to place
INSN_CLASS_ZMMUL right after INSN_CLASS_ZIHINTPAUSE but putting
INSN_CLASS_M_OR_ZMMUL right after INSN_CLASS_M seems equivalently good.


Thanks,
Tsukasa

>  };
>  
>  /* This structure holds information for a particular instruction.  */
> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> index d5cedbe176c..958364fd5c0 100644
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -558,15 +558,15 @@ const struct riscv_opcode riscv_opcodes[] =
>  {"amominu.d.aqrl", 64, INSN_CLASS_A, "d,t,0(s)", MATCH_AMOMINU_D|MASK_AQRL, MASK_AMOMINU_D|MASK_AQRL, match_opcode, INSN_DREF|INSN_8_BYTE },
>  
>  /* Multiply/Divide instruction subset.  */
> -{"mul",        0, INSN_CLASS_M,   "d,s,t",     MATCH_MUL, MASK_MUL, match_opcode, 0 },
> -{"mulh",       0, INSN_CLASS_M,   "d,s,t",     MATCH_MULH, MASK_MULH, match_opcode, 0 },
> -{"mulhu",      0, INSN_CLASS_M,   "d,s,t",     MATCH_MULHU, MASK_MULHU, match_opcode, 0 },
> -{"mulhsu",     0, INSN_CLASS_M,   "d,s,t",     MATCH_MULHSU, MASK_MULHSU, match_opcode, 0 },
> +{"mul",        0, INSN_CLASS_M_OR_ZMMUL,   "d,s,t",     MATCH_MUL, MASK_MUL, match_opcode, 0 },
> +{"mulh",       0, INSN_CLASS_M_OR_ZMMUL,   "d,s,t",     MATCH_MULH, MASK_MULH, match_opcode, 0 },
> +{"mulhu",      0, INSN_CLASS_M_OR_ZMMUL,   "d,s,t",     MATCH_MULHU, MASK_MULHU, match_opcode, 0 },
> +{"mulhsu",     0, INSN_CLASS_M_OR_ZMMUL,   "d,s,t",     MATCH_MULHSU, MASK_MULHSU, match_opcode, 0 },
>  {"div",        0, INSN_CLASS_M,   "d,s,t",     MATCH_DIV, MASK_DIV, match_opcode, 0 },
>  {"divu",       0, INSN_CLASS_M,   "d,s,t",     MATCH_DIVU, MASK_DIVU, match_opcode, 0 },
>  {"rem",        0, INSN_CLASS_M,   "d,s,t",     MATCH_REM, MASK_REM, match_opcode, 0 },
>  {"remu",       0, INSN_CLASS_M,   "d,s,t",     MATCH_REMU, MASK_REMU, match_opcode, 0 },
> -{"mulw",      64, INSN_CLASS_M,   "d,s,t",     MATCH_MULW, MASK_MULW, match_opcode, 0 },
> +{"mulw",      64, INSN_CLASS_M_OR_ZMMUL,   "d,s,t",     MATCH_MULW, MASK_MULW, match_opcode, 0 },
>  {"divw",      64, INSN_CLASS_M,   "d,s,t",     MATCH_DIVW, MASK_DIVW, match_opcode, 0 },
>  {"divuw",     64, INSN_CLASS_M,   "d,s,t",     MATCH_DIVUW, MASK_DIVUW, match_opcode, 0 },
>  {"remw",      64, INSN_CLASS_M,   "d,s,t",     MATCH_REMW, MASK_REMW, match_opcode, 0 },