From: Nelson Chu <nelson@rivosinc.com>
To: Jin Ma <jinma@linux.alibaba.com>
Cc: binutils@sourceware.org, christoph.muellner@vrull.eu,
lifang_xia@linux.alibaba.com, jinma.contrib@gmail.com
Subject: Re: [PATCH v2] RISC-V: T-HEAD: Fix wrong instruction encoding for th.vsetvli
Date: Fri, 5 Jan 2024 10:06:31 +0800 [thread overview]
Message-ID: <CAPpQWtDWt5b06T3pTuJ3rGp7XnikPZORceHRMSLtFr91zMHVvw@mail.gmail.com> (raw)
In-Reply-To: <20240104021740.1203-1-jinma@linux.alibaba.com>
[-- Attachment #1: Type: text/plain, Size: 12083 bytes --]
Cool, looks good, committed.
Thanks
Nelson
On Thu, Jan 4, 2024 at 10:17 AM Jin Ma <jinma@linux.alibaba.com> wrote:
> Since the particularity of "th.vsetvli" was not taken into account in the
> initial support patches for XTheadVector, the program operation failed
> due to instruction coding errors. According to T-Head SPEC ([1]), the
> "vsetvl" in the XTheadVector extension consists of SEW, LMUL and EDIV,
> which is quite different from the "V" extension. Therefore, we cannot
> simply reuse the processing of vsetvl in V extension.
>
> We have set up tens of thousands of test cases to ensure that no
> further encoding issues are there, and and execute all compiled test
> files on real HW and make sure they don't trigger SIGILL.
>
> Ref:
> [1]
> https://github.com/T-head-Semi/thead-extension-spec/releases/download/2.3.0/xthead-2023-11-10-2.3.0.pdf
>
> Co-developed-by: Lifang Xia <lifang_xia@linux.alibaba.com>
> Co-developed-by: Christoph Müllner <christoph.muellner@vrull.eu>
>
> gas/ChangeLog:
>
> * config/tc-riscv.c (validate_riscv_insn): Add handling for
> th.vsetvli.
> (my_getThVsetvliExpression): New function.
> (riscv_ip): Likewise.
> * testsuite/gas/riscv/x-thead-vector.d: Likewise.
> * testsuite/gas/riscv/x-thead-vector.s: Likewise.
>
> include/ChangeLog:
>
> * opcode/riscv.h (OP_MASK_XTHEADVLMUL): New macro.
> (OP_SH_XTHEADVLMUL): Likewise.
> (OP_MASK_XTHEADVSEW): Likewise.
> (OP_SH_XTHEADVSEW): Likewise.
> (OP_MASK_XTHEADVEDIV): Likewise.
> (OP_SH_XTHEADVEDIV): Likewise.
> (OP_MASK_XTHEADVTYPE_RES): Likewise.
> (OP_SH_XTHEADVTYPE_RES): Likewise.
>
> opcodes/ChangeLog:
>
> * riscv-dis.c (print_insn_args): Likewise.
> * riscv-opc.c: Likewise.
> ---
> gas/config/tc-riscv.c | 79 ++++++++++++++++++++++++
> gas/testsuite/gas/riscv/x-thead-vector.d | 3 +-
> gas/testsuite/gas/riscv/x-thead-vector.s | 1 +
> include/opcode/riscv.h | 11 ++++
> opcodes/riscv-dis.c | 22 +++++++
> opcodes/riscv-opc.c | 14 ++++-
> 6 files changed, 128 insertions(+), 2 deletions(-)
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index bb5ea04e662..e2be3ac1138 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -1467,6 +1467,15 @@ validate_riscv_insn (const struct riscv_opcode
> *opc, int length)
> size_t s;
> switch (*++oparg)
> {
> + case 'V':
> + switch (*++oparg)
> + {
> + case 'c': /* Vtypei for th.vsetvli. */
> + used_bits |= ENCODE_RVV_VC_IMM (-1U); break;
> + default:
> + goto unknown_validate_operand;
> + }
> + break;
> case 'l': /* Integer immediate, literal. */
> oparg += strcspn(oparg, ",") - 1;
> break;
> @@ -2423,6 +2432,59 @@ my_getVsetvliExpression (expressionS *ep, char *str)
> }
> }
>
> +/* Parse string STR as a th.vsetvli operand. Store the expression in *EP.
> + On exit, EXPR_PARSE_END points to the first character after the
> + expression. */
> +
> +static void
> +my_getThVsetvliExpression (expressionS *ep, char *str)
> +{
> + unsigned int vsew_value = 0, vlen_value = 0, vediv_value = 0;
> + bfd_boolean vsew_found = FALSE, vlen_found = FALSE, vediv_found = FALSE;
> +
> + if (arg_lookup (&str, riscv_vsew, ARRAY_SIZE (riscv_vsew),
> + &vsew_value))
> + {
> + if (*str == ',')
> + ++str;
> + if (vsew_found)
> + as_bad (_("multiple vsew constants"));
> + vsew_found = TRUE;
> + }
> +
> + if (arg_lookup (&str, riscv_th_vlen, ARRAY_SIZE (riscv_th_vlen),
> + &vlen_value))
> + {
> + if (*str == ',')
> + ++str;
> + if (vlen_found)
> + as_bad (_("multiple vlen constants"));
> + vlen_found = TRUE;
> + }
> + if (arg_lookup (&str, riscv_th_vediv, ARRAY_SIZE (riscv_th_vediv),
> + &vediv_value))
> + {
> + if (*str == ',')
> + ++str;
> + if (vediv_found)
> + as_bad (_("multiple vediv constants"));
> + vediv_found = TRUE;
> + }
> +
> + if (vlen_found || vediv_found || vsew_found)
> + {
> + ep->X_op = O_constant;
> + ep->X_add_number
> + = (vediv_value << 5) | (vsew_value << 2) | (vlen_value);
> + expr_parse_end = str;
> + }
> + else
> + {
> + my_getExpression (ep, str);
> + str = expr_parse_end;
> + }
> +}
> +
> /* Detect and handle implicitly zero load-store offsets. For example,
> "lw t0, (t1)" is shorthand for "lw t0, 0(t1)". Return true if such
> an implicit offset was detected. */
> @@ -3624,6 +3686,23 @@ riscv_ip (char *str, struct riscv_cl_insn *ip,
> expressionS *imm_expr,
> bool sign;
> switch (*++oparg)
> {
> + case 'V':
> + /* Vtypei for th.vsetvli. */
> + ++oparg;
> + if (*oparg != 'c')
> + goto unknown_riscv_ip_operand;
> +
> + my_getThVsetvliExpression (imm_expr, asarg);
> + check_absolute_expr (ip, imm_expr, FALSE);
> + if (!VALID_RVV_VC_IMM (imm_expr->X_add_number))
> + as_bad (_("bad value for th.vsetvli immediate
> field, "
> + "value must be 0..2047"));
> + ip->insn_opcode
> + |= ENCODE_RVV_VC_IMM (imm_expr->X_add_number);
> + imm_expr->X_op = O_absent;
> + asarg = expr_parse_end;
> + continue;
> +
> case 'l': /* Integer immediate, literal. */
> n = strcspn (++oparg, ",");
> if (strncmp (oparg, asarg, n))
> diff --git a/gas/testsuite/gas/riscv/x-thead-vector.d
> b/gas/testsuite/gas/riscv/x-thead-vector.d
> index 441bd0e660f..836150a1498 100644
> --- a/gas/testsuite/gas/riscv/x-thead-vector.d
> +++ b/gas/testsuite/gas/riscv/x-thead-vector.d
> @@ -8,8 +8,9 @@ Disassembly of section .text:
>
> 0+000 <.text>:
> [ ]+[0-9a-f]+:[ ]+80c5f557[ ]+th.vsetvl[ ]+a0,a1,a2
> -[ ]+[0-9a-f]+:[ ]+0005f557[ ]+th.vsetvli[ ]+a0,a1,e8,m1,tu,mu
> +[ ]+[0-9a-f]+:[ ]+0005f557[ ]+th.vsetvli[ ]+a0,a1,e8,m1,d1
> [ ]+[0-9a-f]+:[ ]+7ff5f557[ ]+th.vsetvli[ ]+a0,a1,2047
> +[ ]+[0-9a-f]+:[ ]+0455f557[ ]+th.vsetvli[ ]+a0,a1,e16,m2,d4
> [ ]+[0-9a-f]+:[ ]+12050207[ ]+th.vlb.v[ ]+v4,\(a0\)
> [ ]+[0-9a-f]+:[ ]+12050207[ ]+th.vlb.v[ ]+v4,\(a0\)
> [ ]+[0-9a-f]+:[ ]+10050207[ ]+th.vlb.v[ ]+v4,\(a0\),v0.t
> diff --git a/gas/testsuite/gas/riscv/x-thead-vector.s
> b/gas/testsuite/gas/riscv/x-thead-vector.s
> index 413f4a83a94..92315b52b44 100644
> --- a/gas/testsuite/gas/riscv/x-thead-vector.s
> +++ b/gas/testsuite/gas/riscv/x-thead-vector.s
> @@ -1,6 +1,7 @@
> th.vsetvl a0, a1, a2
> th.vsetvli a0, a1, 0
> th.vsetvli a0, a1, 0x7ff
> + th.vsetvli a0, a1, e16,m2,d4
>
> th.vlb.v v4, (a0)
> th.vlb.v v4, 0(a0)
> diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> index 6687b434074..bc3e85e5bca 100644
> --- a/include/opcode/riscv.h
> +++ b/include/opcode/riscv.h
> @@ -328,6 +328,15 @@ static inline unsigned int riscv_insn_length (insn_t
> insn)
> #define OP_MASK_VWD 0x1
> #define OP_SH_VWD 26
>
> +#define OP_MASK_XTHEADVLMUL 0x3
> +#define OP_SH_XTHEADVLMUL 0
> +#define OP_MASK_XTHEADVSEW 0x7
> +#define OP_SH_XTHEADVSEW 2
> +#define OP_MASK_XTHEADVEDIV 0x3
> +#define OP_SH_XTHEADVEDIV 5
> +#define OP_MASK_XTHEADVTYPE_RES 0xf
> +#define OP_SH_XTHEADVTYPE_RES 7
> +
> #define NVECR 32
> #define NVECM 1
>
> @@ -595,6 +604,8 @@ extern const char * const riscv_vsew[8];
> extern const char * const riscv_vlmul[8];
> extern const char * const riscv_vta[2];
> extern const char * const riscv_vma[2];
> +extern const char * const riscv_th_vlen[4];
> +extern const char * const riscv_th_vediv[4];
> extern const char * const riscv_fli_symval[32];
> extern const float riscv_fli_numval[32];
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 68674380797..09c5bd3d4ae 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -653,6 +653,28 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma
> pc, disassemble_info *info
> bool sign;
> switch (*++oparg)
> {
> + case 'V':
> + ++oparg;
> + if (*oparg != 'c')
> + goto undefined_modifier;
> +
> + int imm = (*oparg == 'b') ? EXTRACT_RVV_VB_IMM (l)
> + : EXTRACT_RVV_VC_IMM (l);
> + unsigned int imm_vediv = EXTRACT_OPERAND (XTHEADVEDIV,
> imm);
> + unsigned int imm_vlmul = EXTRACT_OPERAND (XTHEADVLMUL,
> imm);
> + unsigned int imm_vsew = EXTRACT_OPERAND (XTHEADVSEW,
> imm);
> + unsigned int imm_vtype_res
> + = EXTRACT_OPERAND (XTHEADVTYPE_RES, imm);
> + if (imm_vsew < ARRAY_SIZE (riscv_vsew)
> + && imm_vlmul < ARRAY_SIZE (riscv_th_vlen)
> + && imm_vediv < ARRAY_SIZE (riscv_th_vediv)
> + && ! imm_vtype_res)
> + print (info->stream, dis_style_text, "%s,%s,%s",
> + riscv_vsew[imm_vsew],
> riscv_th_vlen[imm_vlmul],
> + riscv_th_vediv[imm_vediv]);
> + else
> + print (info->stream, dis_style_immediate, "%d", imm);
> + break;
> case 'l': /* Integer immediate, literal. */
> oparg++;
> while (*oparg && *oparg != ',')
> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> index 5441ec0dc28..2c969477bc6 100644
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -110,6 +110,18 @@ const char * const riscv_vma[2] =
> "mu", "ma"
> };
>
> +/* XTheadVector, List of vsetvli vlmul constants. */
> +const char * const riscv_th_vlen[4] =
> +{
> + "m1", "m2", "m4", "m8"
> +};
> +
> +/* XTheadVector, List of vsetvli vediv constants. */
> +const char * const riscv_th_vediv[4] =
> +{
> + "d1", "d2", "d4", "d8"
> +};
> +
> /* The FLI.[HSDQ] symbolic constants (NULL for numeric constant). */
> const char * const riscv_fli_symval[32] =
> {
> @@ -2236,7 +2248,7 @@ const struct riscv_opcode riscv_opcodes[] =
>
> /* Vendor-specific (T-Head) XTheadVector instructions. */
> {"th.vsetvl", 0, INSN_CLASS_XTHEADVECTOR, "d,s,t", MATCH_VSETVL,
> MASK_VSETVL, match_opcode, 0},
> -{"th.vsetvli", 0, INSN_CLASS_XTHEADVECTOR, "d,s,Vc", MATCH_VSETVLI,
> MASK_VSETVLI, match_opcode, 0},
> +{"th.vsetvli", 0, INSN_CLASS_XTHEADVECTOR, "d,s,XtVc", MATCH_VSETVLI,
> MASK_VSETVLI, match_opcode, 0},
> {"th.vlb.v", 0, INSN_CLASS_XTHEADVECTOR, "Vd,0(s)Vm",
> MATCH_TH_VLBV, MASK_TH_VLBV, match_opcode, INSN_DREF },
> {"th.vlh.v", 0, INSN_CLASS_XTHEADVECTOR, "Vd,0(s)Vm",
> MATCH_TH_VLHV, MASK_TH_VLHV, match_opcode, INSN_DREF },
> {"th.vlw.v", 0, INSN_CLASS_XTHEADVECTOR, "Vd,0(s)Vm",
> MATCH_TH_VLWV, MASK_TH_VLWV, match_opcode, INSN_DREF },
> --
> 2.17.1
>
>
prev parent reply other threads:[~2024-01-05 2:06 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-18 9:29 [PATCH] RISC-V: T-HEAD: Fix wrong instruction encoding for th.vsetvl Jin Ma
2023-12-29 0:57 ` Nelson Chu
2024-01-04 2:17 ` [PATCH v2] RISC-V: T-HEAD: Fix wrong instruction encoding for th.vsetvli Jin Ma
2024-01-05 2:06 ` Nelson Chu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPpQWtDWt5b06T3pTuJ3rGp7XnikPZORceHRMSLtFr91zMHVvw@mail.gmail.com \
--to=nelson@rivosinc.com \
--cc=binutils@sourceware.org \
--cc=christoph.muellner@vrull.eu \
--cc=jinma.contrib@gmail.com \
--cc=jinma@linux.alibaba.com \
--cc=lifang_xia@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).