[PATCH v2 0/2] RISC-V: Opcode Tidying - Operands (batch 1)

[PATCH v2 0/2] RISC-V: Opcode Tidying - Operands (batch 1)

[Tsukasa OI]

Hello,

This is a small patchset to clean / maintain RISC-V instruction
operand types.

[Changes: v1 -> v2]
-   Cleaning? (possibly unchanged? then this is a ping)


[PATCH 1/2]

Since `validate_riscv_insn' function lists three macro-only operand types,
this patch adds other three operands.  That means, `validate_riscv_insn'
lists all macro-only operand types with this patch.

Existing:
-   A
-   B
-   I

New in This Patch:
-   c
-   VM
-   VT

Note that `validate_riscv_insn' is called only for non-macros.  In the
future, we could reject (and make an internal error) when we encountered
those macro-only operand types on regular (non-macro) instructions.


[PATCH 2/2]

The operand type "b" has no good reasons to keep and should be removed.

-   It looks like an alias of the "s" operand type.
-   It hasn't used since the beginning.
-   Its role is not clear.

On the other hand, this patch keeps following unused operand types for now:

-   Cx     : future compressed instructions?
-   Vf, Ve : vector AMO instructions (instructions are not upstreamed
             but operand types are upstreamed already)
-   [, ]   : used by some vendors? At least, their role is clear.


Thanks,
Tsukasa




Tsukasa OI (2):
  RISC-V: Add macro-only operands to validate_riscv_insn
  RISC-V: Remove "b" operand type from disassembler

 gas/config/tc-riscv.c | 3 +++
 opcodes/riscv-dis.c   | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)


base-commit: 90eca7111355e4c6683c1ab10fd07107ea10f6d1
-- 
2.34.1

[PATCH v2 1/2] RISC-V: Add macro-only operands to validate_riscv_insn

[Tsukasa OI]

Although they are not (and should not be) reachable, following macro-only
operands are parsed in the `validate_riscv_insn' function and ignored.
That function also notes that they are macro-only.

-   "A"
-   "B"
-   "I"

Following this convention, this commit adds three remaining macro-only
operands to this function.  By doing this, we could instead choose to reject
those operands from appearing in regular instructions later.

-   "c"   (used by call, tail and jump macros)
-   "VM"  (used by vmsge.vx and vmsgeu.vx macros)
-   "VT"  (likewise)

gas/ChangeLog:

	* config/tc-riscv.c (validate_riscv_insn): Add "c", "VM" and "VT"
	macro-only operand types.
---
 gas/config/tc-riscv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 5411d68a401..0a14b2cecc8 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -1203,6 +1203,8 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
 	    case 'j':
 	    case 'k': USE_BITS (OP_MASK_VIMM, OP_SH_VIMM); break;
 	    case 'm': USE_BITS (OP_MASK_VMASK, OP_SH_VMASK); break;
+	    case 'M': break; /* Macro operand, must be a mask register.  */
+	    case 'T': break; /* Macro operand, must be a vector register.  */
 	    default:
 	      goto unknown_validate_operand;
 	    }
@@ -1214,6 +1216,7 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
 	case '>': USE_BITS (OP_MASK_SHAMT, OP_SH_SHAMT); break;
 	case 'A': break; /* Macro operand, must be symbol.  */
 	case 'B': break; /* Macro operand, must be symbol or constant.  */
+	case 'c': break; /* Macro operand, must be symbol or constant.  */
 	case 'I': break; /* Macro operand, must be constant.  */
 	case 'D': /* RD, floating point.  */
 	case 'd': USE_BITS (OP_MASK_RD, OP_SH_RD); break;
-- 
2.34.1

[PATCH v2 2/2] RISC-V: Remove "b" operand type from disassembler

[Tsukasa OI]

There are a few operand types not used by any RISC-V instructions.

-   Cx
-   Vf
-   Ve
-   [
-   ]
-   b

But most of them has a reasoning to keep them:

-   Cx     : Same as "Ct" except it has a constraint to have rd == rs2
	     (similar to "Cw").  Although it hasn't used, its role is clear
	     enough to implement a new instruction with this operand type.
-   Vf, Ve : Used by vector AMO instructions (not ratified and real
	     instructions are not upstreamed yet).
-   [, ]   : Unused tokenization symbols.  Reserving them is not harmful
	     and a vendor may use this symbol for special purposes.

... except "b".  I could not have found any reference to this operand type
except it works like the "s" operand type.  Historically, it seems... it's
just unused from the beginning.  Its role is not clear either.

On such cases, we should vacate this room for the new operand type with
much clearer roles.

opcodes/ChangeLog:

	* riscv-dis.c (print_insn_args): Remove 'b' operand type.
---
 opcodes/riscv-dis.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 7ae6e709290..99cebf37d9e 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -403,7 +403,6 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
 	    print (info->stream, dis_style_immediate, "0");
 	  break;
 
-	case 'b':
 	case 's':
 	  if ((l & MASK_JALR) == MATCH_JALR)
 	    maybe_print_address (pd, rs1, EXTRACT_ITYPE_IMM (l), 0);
-- 
2.34.1

Re: [PATCH v2 0/2] RISC-V: Opcode Tidying - Operands (batch 1)

[Nelson Chu]

On Thu, Sep 22, 2022 at 2:30 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> Hello,
>
> This is a small patchset to clean / maintain RISC-V instruction
> operand types.
>
> [Changes: v1 -> v2]
> -   Cleaning? (possibly unchanged? then this is a ping)

In fact you can reply and ping the original patch directly without
sending a new one if nothing changed.  Anyway, these two patch looks
good, please commit.

Thanks
Nelson

>
> [PATCH 1/2]
>
> Since `validate_riscv_insn' function lists three macro-only operand types,
> this patch adds other three operands.  That means, `validate_riscv_insn'
> lists all macro-only operand types with this patch.
>
> Existing:
> -   A
> -   B
> -   I
>
> New in This Patch:
> -   c
> -   VM
> -   VT
>
> Note that `validate_riscv_insn' is called only for non-macros.  In the
> future, we could reject (and make an internal error) when we encountered
> those macro-only operand types on regular (non-macro) instructions.
>
>
> [PATCH 2/2]
>
> The operand type "b" has no good reasons to keep and should be removed.
>
> -   It looks like an alias of the "s" operand type.
> -   It hasn't used since the beginning.
> -   Its role is not clear.
>
> On the other hand, this patch keeps following unused operand types for now:
>
> -   Cx     : future compressed instructions?
> -   Vf, Ve : vector AMO instructions (instructions are not upstreamed
>              but operand types are upstreamed already)
> -   [, ]   : used by some vendors? At least, their role is clear.
>
>
> Thanks,
> Tsukasa
>
>
>
>
> Tsukasa OI (2):
>   RISC-V: Add macro-only operands to validate_riscv_insn
>   RISC-V: Remove "b" operand type from disassembler
>
>  gas/config/tc-riscv.c | 3 +++
>  opcodes/riscv-dis.c   | 1 -
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
>
> base-commit: 90eca7111355e4c6683c1ab10fd07107ea10f6d1
> --
> 2.34.1
>