diff mbox series

[v3,02/13] RISC-V: EEW64 checking

Message ID 1a57710b-44d4-4852-91b1-01665b8f383c@suse.com
State New
Headers
Series RISC-V: assorted fixes and (hopefully) improvements |

Commit Message

Jan Beulich June 19, 2026, 11:47 a.m. UTC 
  Like the assembler, the disassembler should also use the INSN_V_EEW64
flag. Respectively marked insns shouldn't be disassembled normally when
Zve64x isn't enabled.

While there also drop the redundant Zve32x check from gas: All affected
insns are INSN_CLASS_V, which means Zve32x was already checked to be
enabled (both V and Zve64x imply Zve32x).
---
Question is why a flag was used in the first place: The property could be
expressed by a new INSN_CLASS_ZVE64X, couldn't it?

Comments

Nelson Chu July 21, 2026, 1:30 a.m. UTC | #1 
On Fri, Jun 19, 2026 at 7:47 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> Like the assembler, the disassembler should also use the INSN_V_EEW64
> flag. Respectively marked insns shouldn't be disassembled normally when
> Zve64x isn't enabled.
>
> While there also drop the redundant Zve32x check from gas: All affected
> insns are INSN_CLASS_V, which means Zve32x was already checked to be
> enabled (both V and Zve64x imply Zve32x).

make sense.

> Question is why a flag was used in the first place: The property could be
> expressed by a new INSN_CLASS_ZVE64X, couldn't it?

I forgot, probably just because people argued they want to
dis-assembler old objects for segment load/store with EEW64 but
without zve64x since this check may be added later that, but it
doesn't matter now.  I think using a new INSN_CLASS_ZVE64X should be
the right way to go.

Thanks
Nelson

> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -2964,10 +2964,9 @@ riscv_ip (char *str, struct riscv_cl_ins
>                       insn_with_csr = false;
>                     }
>
> -                 /* The (segmant) load and store with EEW 64 cannot be used
> +                 /* The (segment) load and store with EEW 64 cannot be used
>                      when zve32x is enabled.  */
> -                 if (ip->insn_mo->pinfo & INSN_V_EEW64
> -                     && riscv_subset_supports (&riscv_rps_as, "zve32x")
> +                 if ((ip->insn_mo->pinfo & INSN_V_EEW64)
>                       && !riscv_subset_supports (&riscv_rps_as, "zve64x"))
>                     {
>                       error.msg = _("illegal opcode for zve32x");
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -1018,10 +1018,16 @@ riscv_disassemble_insn (bfd_vma memaddr,
>               && (op->xlen_requirement != pd->xlen))
>             continue;
>           /* Is this instruction supported by the current architecture?  */
> -         if (!pd->all_ext
> -             && !riscv_multi_subset_supports (&pd->riscv_rps_dis,
> -                                              op->insn_class))
> -           continue;
> +         if (!pd->all_ext)
> +           {
> +             if (!riscv_multi_subset_supports (&pd->riscv_rps_dis,
> +                                               op->insn_class))
> +               continue;
> +
> +             if ((op->pinfo & INSN_V_EEW64)
> +                 && !riscv_subset_supports (&pd->riscv_rps_dis, "zve64x"))
> +               continue;
> +           }
>
>           /* It's a match.  */
>           (*info->fprintf_styled_func) (info->stream, dis_style_mnemonic,
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/eew64.d
> @@ -0,0 +1,13 @@
> +#as: -march=rv32i
> +#objdump: -d
> +
> +.*:[   ]+file format .*
> +
> +Disassembly of section .text:
> +
> +0+000 <eew32>:
> +[      ]+0:[   ]+020ff007[     ]+\.insn[       ]+4, ?0x020ff007
> +
> +0+004 <eew64>:
> +[      ]+4:[   ]+020ff007[     ]+vle64\.v[     ]+v0,\(t6\)
> +#pass
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/eew64.s
> @@ -0,0 +1,12 @@
> +       .option arch, rv32i
> +       .option arch, +zve32x
> +
> +eew32:
> +       # vle64.v v0, (x31)
> +       .insn 4, 0x020ff007
> +
> +       .option arch, rv32i
> +       .option arch, +zve64x
> +
> +eew64:
> +       vle64.v v0, (x31)
>
Jan Beulich July 21, 2026, 6:09 a.m. UTC | #2 
On 21.07.2026 03:30, Nelson Chu wrote:
> On Fri, Jun 19, 2026 at 7:47 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> Like the assembler, the disassembler should also use the INSN_V_EEW64
>> flag. Respectively marked insns shouldn't be disassembled normally when
>> Zve64x isn't enabled.
>>
>> While there also drop the redundant Zve32x check from gas: All affected
>> insns are INSN_CLASS_V, which means Zve32x was already checked to be
>> enabled (both V and Zve64x imply Zve32x).
> 
> make sense.
> 
>> Question is why a flag was used in the first place: The property could be
>> expressed by a new INSN_CLASS_ZVE64X, couldn't it?
> 
> I forgot, probably just because people argued they want to
> dis-assembler old objects for segment load/store with EEW64 but
> without zve64x since this check may be added later that, but it
> doesn't matter now.  I think using a new INSN_CLASS_ZVE64X should be
> the right way to go.

Okay, I'll make a follow-on patch then.

Jan
diff mbox series

Patch

--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -2964,10 +2964,9 @@  riscv_ip (char *str, struct riscv_cl_ins
 		      insn_with_csr = false;
 		    }
 
-		  /* The (segmant) load and store with EEW 64 cannot be used
+		  /* The (segment) load and store with EEW 64 cannot be used
 		     when zve32x is enabled.  */
-		  if (ip->insn_mo->pinfo & INSN_V_EEW64
-		      && riscv_subset_supports (&riscv_rps_as, "zve32x")
+		  if ((ip->insn_mo->pinfo & INSN_V_EEW64)
 		      && !riscv_subset_supports (&riscv_rps_as, "zve64x"))
 		    {
 		      error.msg = _("illegal opcode for zve32x");
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -1018,10 +1018,16 @@  riscv_disassemble_insn (bfd_vma memaddr,
 	      && (op->xlen_requirement != pd->xlen))
 	    continue;
 	  /* Is this instruction supported by the current architecture?  */
-	  if (!pd->all_ext
-	      && !riscv_multi_subset_supports (&pd->riscv_rps_dis,
-					       op->insn_class))
-	    continue;
+	  if (!pd->all_ext)
+	    {
+	      if (!riscv_multi_subset_supports (&pd->riscv_rps_dis,
+						op->insn_class))
+		continue;
+
+ 	      if ((op->pinfo & INSN_V_EEW64)
+		  && !riscv_subset_supports (&pd->riscv_rps_dis, "zve64x"))
+		continue;
+	    }
 
 	  /* It's a match.  */
 	  (*info->fprintf_styled_func) (info->stream, dis_style_mnemonic,
--- /dev/null
+++ b/gas/testsuite/gas/riscv/eew64.d
@@ -0,0 +1,13 @@ 
+#as: -march=rv32i
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+Disassembly of section .text:
+
+0+000 <eew32>:
+[ 	]+0:[ 	]+020ff007[ 	]+\.insn[ 	]+4, ?0x020ff007
+
+0+004 <eew64>:
+[ 	]+4:[ 	]+020ff007[ 	]+vle64\.v[ 	]+v0,\(t6\)
+#pass
--- /dev/null
+++ b/gas/testsuite/gas/riscv/eew64.s
@@ -0,0 +1,12 @@ 
+	.option arch, rv32i
+	.option arch, +zve32x
+
+eew32:
+	# vle64.v v0, (x31)
+	.insn 4, 0x020ff007
+
+	.option arch, rv32i
+	.option arch, +zve64x
+
+eew64:
+	vle64.v	v0, (x31)