[PATCH v3 1/1] RISC-V: Add SpacemiT vendor extensions xsmtvdot

Jan Beulich jbeulich@suse.com
Fri Apr 10 10:25:15 GMT 2026


On 10.04.2026 12:00, Mark Zhuang wrote:
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/x-smt-vdot-fail.l
> @@ -0,0 +1,3 @@
> +#as: -march=rv64gcv_xsmtvdot
> +#source: x-smt-vdot-fail.s
> +#error_output: x-smt-vdot-fail.l

Doesn't this file need to be named x-smt-vdot-fail.d, for the test to be
executed at all? And isn't the real x-smt-vdot-fail.l missing?

> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/x-smt-vdot.s
> @@ -0,0 +1,33 @@
> +target:
> +	smt.vmadot v2, v3, v4
> +	smt.vmadotu v2, v3, v4
> +	smt.vmadotsu v2, v3, v4
> +	smt.vmadotus v2, v3, v4
> +	smt.vmadot1u v2, v4, v5
> +	smt.vmadot1 v2, v4, v5
> +	smt.vmadot1su v2, v4, v5
> +	smt.vmadot1us v2, v4, v5
> +	smt.vmadot2u v2, v4, v5
> +	smt.vmadot2 v2, v4, v5
> +	smt.vmadot2su v2, v4, v5
> +	smt.vmadot2us v2, v4, v5
> +	smt.vmadot3u v2, v4, v5
> +	smt.vmadot3 v2, v4, v5
> +	smt.vmadot3su v2, v4, v5
> +	smt.vmadot3us v2, v4, v5
> +	smt.vmadot v2, v3, v4, i8
> +	smt.vmadotu v2, v3, v4, i8
> +	smt.vmadotsu v2, v3, v4, i8
> +	smt.vmadotus v2, v3, v4, i8
> +	smt.vmadot1u v2, v4, v5, i8
> +	smt.vmadot1 v2, v4, v5, i8
> +	smt.vmadot1su v2, v4, v5, i8
> +	smt.vmadot1us v2, v4, v5, i8
> +	smt.vmadot2u v2, v4, v5, i8
> +	smt.vmadot2 v2, v4, v5, i8
> +	smt.vmadot2su v2, v4, v5, i8
> +	smt.vmadot2us v2, v4, v5, i8
> +	smt.vmadot3u v2, v4, v5, i8
> +	smt.vmadot3 v2, v4, v5, i8
> +	smt.vmadot3su v2, v4, v5, i8
> +	smt.vmadot3us v2, v4, v5, i8

Only i8 being tested? (And no invalid i<N> in the -fail test?)

> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -917,6 +917,65 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
>  		  goto undefined_modifier;
>  		}
>  	      break;
> +	    case 'p': /* Vendor-specific (SpacemiT) operands.  */
> +	      {
> +		switch (*++oparg)
> +		  {
> +		  case 'V':
> +		    switch (*++oparg)
> +		      {
> +		      case 'd':
> +			unsigned vd = EXTRACT_OPERAND (SPACEMIT_IME_VD, l) * 2;
> +			print (info->stream, dis_style_register, "%s",
> +			       riscv_vecr_names_numeric[vd]);
> +			break;
> +		      case 's':
> +			unsigned vs = EXTRACT_OPERAND (SPACEMIT_IME_VS1, l) * 2;
> +			print (info->stream, dis_style_register, "%s",
> +			       riscv_vecr_names_numeric[vs]);
> +			break;
> +		      default:
> +			goto undefined_modifier;
> +		      }
> +		    break;
> +		  case 'w':
> +		    /* Xpw&S ... bits in S indicates whether corresponding
> +		       item is permitted.  */
> +		    if (*++oparg != '&')
> +		      goto undefined_modifier;
> +		    strtol (oparg + 1, (char **)&oparg, 16);
> +		    oparg--;
> +		    unsigned wi = EXTRACT_OPERAND (SPACEMIT_IME_WI, l);
> +		    /* Only print if not the default value (i8 = 3).  */
> +		    if (wi != 3)

In a situation like this, with already pretty deep indentation, can I talk
you into using

		    if (wi == 3)
		      break;

instead?

> +		      {
> +			print (info->stream, dis_style_text, ",");
> +			switch (wi)
> +			  {
> +			  case 0:
> +			    print (info->stream, dis_style_text, "i2");
> +			    break;
> +			  case 1:
> +			    print (info->stream, dis_style_text, "i16");
> +			    break;
> +			  case 2:
> +			    print (info->stream, dis_style_text, "i4");
> +			    break;
> +			  case 3:
> +			    print (info->stream, dis_style_text, "i8");
> +			    break;
> +			  default:
> +			    print (info->stream, dis_style_immediate, "%u", wi);
> +			      break;
> +			  }
> +		      }

The scope here then can go away, and indentation can be reduced by 2 levels.

The last of the "break" also looks mis-indented, but the question is
whether the default: case is needed at all. wi can't take any value other
than 0...3.

Jan


More information about the Binutils mailing list