public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/1] RISC-V: Move supervisor instructions after all unprivileged ones
@ 2022-10-01  5:39 Tsukasa OI
  2022-10-01  5:39 ` [PATCH 1/1] " Tsukasa OI
  0 siblings, 1 reply; 4+ messages in thread
From: Tsukasa OI @ 2022-10-01  5:39 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: binutils

Hello,

GitHub tracker:
<https://github.com/a4lg/binutils-gdb/wiki/riscv_opcode_tidying_csr_1>

This is a small tidying patchset.

This location of supervisor instructions is out of place (because many other
privileged instructions are located at the tail but after the supervisor
instructions, we have many unprivileged instructions including bit
manipulation / crypto / vector instructions).

Not only that, this is harmful to implement pseudoinstructions in the latest
'P'-extension proposal (CLROV and RDOV).

This patchset moves supervisor instructions after all unprivileged
instructions and adjusts some indents.


Thanks,
Tsukasa




Tsukasa OI (1):
  RISC-V: Move supervisor instructions after all unprivileged ones

 opcodes/riscv-opc.c | 64 ++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 32 deletions(-)


base-commit: 53a265a1f14d17a6f7b106082f610994c5d546e0
-- 
2.34.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/1] RISC-V: Move supervisor instructions after all unprivileged ones
  2022-10-01  5:39 [PATCH 0/1] RISC-V: Move supervisor instructions after all unprivileged ones Tsukasa OI
@ 2022-10-01  5:39 ` Tsukasa OI
  2022-10-01  7:45   ` Nelson Chu
  0 siblings, 1 reply; 4+ messages in thread
From: Tsukasa OI @ 2022-10-01  5:39 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: binutils

This location of supervisor instructions is out of place (because many other
privileged instructions are located at the tail but after the supervisor
instructions, we have many unprivileged instructions including bit
manipulation / crypto / vector instructions).

Not only that, this is harmful to implement pseudoinstructions in the latest
'P'-extension proposal (CLROV and RDOV).  This commit moves supervisor
instructions after all unprivileged instructions.

opcodes/ChangeLog:

	* riscv-opc.c (riscv_opcodes): Adjust indents.  Move supervisor
	instructions after all unprivileged instructions.
---
 opcodes/riscv-opc.c | 64 ++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
index 7e95f645c5c..3eb69cdc34c 100644
--- a/opcodes/riscv-opc.c
+++ b/opcodes/riscv-opc.c
@@ -912,38 +912,6 @@ const struct riscv_opcode riscv_opcodes[] =
 {"c.fswsp",   32, INSN_CLASS_F_AND_C, "CT,CM(Cc)", MATCH_C_FSWSP, MASK_C_FSWSP, match_opcode, INSN_DREF|INSN_4_BYTE },
 {"c.fsw",     32, INSN_CLASS_F_AND_C, "CD,Ck(Cs)", MATCH_C_FSW, MASK_C_FSW, match_opcode, INSN_DREF|INSN_4_BYTE },
 
-/* Supervisor instructions.  */
-{"csrr",       0, INSN_CLASS_ZICSR,"d,E",      MATCH_CSRRS, MASK_CSRRS|MASK_RS1, match_opcode, INSN_ALIAS },
-{"csrw",       0, INSN_CLASS_ZICSR,"E,s",      MATCH_CSRRW, MASK_CSRRW|MASK_RD, match_opcode, INSN_ALIAS },
-{"csrw",       0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS },
-{"csrwi",      0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS },
-{"csrs",       0, INSN_CLASS_ZICSR,"E,s",      MATCH_CSRRS, MASK_CSRRS|MASK_RD, match_opcode, INSN_ALIAS },
-{"csrs",       0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS },
-{"csrsi",      0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS },
-{"csrc",       0, INSN_CLASS_ZICSR,"E,s",      MATCH_CSRRC, MASK_CSRRC|MASK_RD, match_opcode, INSN_ALIAS },
-{"csrc",       0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS },
-{"csrci",      0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS },
-{"csrrw",      0, INSN_CLASS_ZICSR,"d,E,s",    MATCH_CSRRW, MASK_CSRRW, match_opcode, 0 },
-{"csrrw",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRWI, MASK_CSRRWI, match_opcode, INSN_ALIAS },
-{"csrrwi",     0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRWI, MASK_CSRRWI, match_opcode, 0 },
-{"csrrs",      0, INSN_CLASS_ZICSR,"d,E,s",    MATCH_CSRRS, MASK_CSRRS, match_opcode, 0 },
-{"csrrs",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRSI, MASK_CSRRSI, match_opcode, INSN_ALIAS },
-{"csrrsi",     0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRSI, MASK_CSRRSI, match_opcode, 0 },
-{"csrrc",      0, INSN_CLASS_ZICSR,"d,E,s",    MATCH_CSRRC, MASK_CSRRC, match_opcode, 0 },
-{"csrrc",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRCI, MASK_CSRRCI, match_opcode, INSN_ALIAS },
-{"csrrci",     0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRCI, MASK_CSRRCI, match_opcode, 0 },
-{"uret",       0, INSN_CLASS_I,    "",         MATCH_URET, MASK_URET, match_opcode, 0 },
-{"sret",       0, INSN_CLASS_I,    "",         MATCH_SRET, MASK_SRET, match_opcode, 0 },
-{"hret",       0, INSN_CLASS_I,    "",         MATCH_HRET, MASK_HRET, match_opcode, 0 },
-{"mret",       0, INSN_CLASS_I,    "",         MATCH_MRET, MASK_MRET, match_opcode, 0 },
-{"dret",       0, INSN_CLASS_I,    "",         MATCH_DRET, MASK_DRET, match_opcode, 0 },
-{"sfence.vm",  0, INSN_CLASS_I,    "",         MATCH_SFENCE_VM, MASK_SFENCE_VM | MASK_RS1, match_opcode, 0 },
-{"sfence.vm",  0, INSN_CLASS_I,    "s",        MATCH_SFENCE_VM, MASK_SFENCE_VM, match_opcode, 0 },
-{"sfence.vma", 0, INSN_CLASS_I,    "",         MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS },
-{"sfence.vma", 0, INSN_CLASS_I,    "s",        MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS2, match_opcode, INSN_ALIAS },
-{"sfence.vma", 0, INSN_CLASS_I,    "s,t",      MATCH_SFENCE_VMA, MASK_SFENCE_VMA, match_opcode, 0 },
-{"wfi",        0, INSN_CLASS_I,    "",         MATCH_WFI, MASK_WFI, match_opcode, 0 },
-
 /* Zicbom and Zicboz instructions.  */
 {"cbo.clean",  0, INSN_CLASS_ZICBOM, "0(s)", MATCH_CBO_CLEAN, MASK_CBO_CLEAN, match_opcode, 0 },
 {"cbo.flush",  0, INSN_CLASS_ZICBOM, "0(s)", MATCH_CBO_FLUSH, MASK_CBO_FLUSH, match_opcode, 0 },
@@ -1830,6 +1798,38 @@ const struct riscv_opcode riscv_opcodes[] =
 {"vmv4r.v",    0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV4RV, MASK_VMV4RV, match_opcode, 0},
 {"vmv8r.v",    0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV8RV, MASK_VMV8RV, match_opcode, 0},
 
+/* Supervisor instructions.  */
+{"csrr",       0, INSN_CLASS_ZICSR, "d,E",   MATCH_CSRRS, MASK_CSRRS|MASK_RS1, match_opcode, INSN_ALIAS },
+{"csrw",       0, INSN_CLASS_ZICSR, "E,s",   MATCH_CSRRW, MASK_CSRRW|MASK_RD, match_opcode, INSN_ALIAS },
+{"csrw",       0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS },
+{"csrwi",      0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS },
+{"csrs",       0, INSN_CLASS_ZICSR, "E,s",   MATCH_CSRRS, MASK_CSRRS|MASK_RD, match_opcode, INSN_ALIAS },
+{"csrs",       0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS },
+{"csrsi",      0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS },
+{"csrc",       0, INSN_CLASS_ZICSR, "E,s",   MATCH_CSRRC, MASK_CSRRC|MASK_RD, match_opcode, INSN_ALIAS },
+{"csrc",       0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS },
+{"csrci",      0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS },
+{"csrrw",      0, INSN_CLASS_ZICSR, "d,E,s", MATCH_CSRRW, MASK_CSRRW, match_opcode, 0 },
+{"csrrw",      0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRWI, MASK_CSRRWI, match_opcode, INSN_ALIAS },
+{"csrrwi",     0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRWI, MASK_CSRRWI, match_opcode, 0 },
+{"csrrs",      0, INSN_CLASS_ZICSR, "d,E,s", MATCH_CSRRS, MASK_CSRRS, match_opcode, 0 },
+{"csrrs",      0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRSI, MASK_CSRRSI, match_opcode, INSN_ALIAS },
+{"csrrsi",     0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRSI, MASK_CSRRSI, match_opcode, 0 },
+{"csrrc",      0, INSN_CLASS_ZICSR, "d,E,s", MATCH_CSRRC, MASK_CSRRC, match_opcode, 0 },
+{"csrrc",      0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRCI, MASK_CSRRCI, match_opcode, INSN_ALIAS },
+{"csrrci",     0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRCI, MASK_CSRRCI, match_opcode, 0 },
+{"uret",       0, INSN_CLASS_I, "",    MATCH_URET, MASK_URET, match_opcode, 0 },
+{"sret",       0, INSN_CLASS_I, "",    MATCH_SRET, MASK_SRET, match_opcode, 0 },
+{"hret",       0, INSN_CLASS_I, "",    MATCH_HRET, MASK_HRET, match_opcode, 0 },
+{"mret",       0, INSN_CLASS_I, "",    MATCH_MRET, MASK_MRET, match_opcode, 0 },
+{"dret",       0, INSN_CLASS_I, "",    MATCH_DRET, MASK_DRET, match_opcode, 0 },
+{"sfence.vm",  0, INSN_CLASS_I, "",    MATCH_SFENCE_VM, MASK_SFENCE_VM | MASK_RS1, match_opcode, 0 },
+{"sfence.vm",  0, INSN_CLASS_I, "s",   MATCH_SFENCE_VM, MASK_SFENCE_VM, match_opcode, 0 },
+{"sfence.vma", 0, INSN_CLASS_I, "",    MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS },
+{"sfence.vma", 0, INSN_CLASS_I, "s",   MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS2, match_opcode, INSN_ALIAS },
+{"sfence.vma", 0, INSN_CLASS_I, "s,t", MATCH_SFENCE_VMA, MASK_SFENCE_VMA, match_opcode, 0 },
+{"wfi",        0, INSN_CLASS_I, "",    MATCH_WFI, MASK_WFI, match_opcode, 0 },
+
 /* Svinval instructions.  */
 {"sinval.vma",      0, INSN_CLASS_SVINVAL, "s,t", MATCH_SINVAL_VMA, MASK_SINVAL_VMA, match_opcode, 0 },
 {"sfence.w.inval",  0, INSN_CLASS_SVINVAL, "",    MATCH_SFENCE_W_INVAL, MASK_SFENCE_W_INVAL, match_opcode, 0 },
-- 
2.34.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] RISC-V: Move supervisor instructions after all unprivileged ones
  2022-10-01  5:39 ` [PATCH 1/1] " Tsukasa OI
@ 2022-10-01  7:45   ` Nelson Chu
  2022-10-01  8:15     ` Tsukasa OI
  0 siblings, 1 reply; 4+ messages in thread
From: Nelson Chu @ 2022-10-01  7:45 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: binutils

On Sat, Oct 1, 2022 at 1:41 PM Tsukasa OI via Binutils
<binutils@sourceware.org> wrote:
>
> This location of supervisor instructions is out of place (because many other
> privileged instructions are located at the tail but after the supervisor
> instructions, we have many unprivileged instructions including bit
> manipulation / crypto / vector instructions).
>
> Not only that, this is harmful to implement pseudoinstructions in the latest
> 'P'-extension proposal (CLROV and RDOV).  This commit moves supervisor
> instructions after all unprivileged instructions.

OK, since I don't have a strong opinion on this.

Not to reject this patch, just a minor thought - we cannot always
gather all extensions nearly, once some of them support the
instructions based on others.  So maybe just place CLROV and RDOV
before csrrs and csrrci is the easier way to do.

Thanks
Nelson

> opcodes/ChangeLog:
>
>         * riscv-opc.c (riscv_opcodes): Adjust indents.  Move supervisor
>         instructions after all unprivileged instructions.
> ---
>  opcodes/riscv-opc.c | 64 ++++++++++++++++++++++-----------------------
>  1 file changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> index 7e95f645c5c..3eb69cdc34c 100644
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -912,38 +912,6 @@ const struct riscv_opcode riscv_opcodes[] =
>  {"c.fswsp",   32, INSN_CLASS_F_AND_C, "CT,CM(Cc)", MATCH_C_FSWSP, MASK_C_FSWSP, match_opcode, INSN_DREF|INSN_4_BYTE },
>  {"c.fsw",     32, INSN_CLASS_F_AND_C, "CD,Ck(Cs)", MATCH_C_FSW, MASK_C_FSW, match_opcode, INSN_DREF|INSN_4_BYTE },
>
> -/* Supervisor instructions.  */
> -{"csrr",       0, INSN_CLASS_ZICSR,"d,E",      MATCH_CSRRS, MASK_CSRRS|MASK_RS1, match_opcode, INSN_ALIAS },
> -{"csrw",       0, INSN_CLASS_ZICSR,"E,s",      MATCH_CSRRW, MASK_CSRRW|MASK_RD, match_opcode, INSN_ALIAS },
> -{"csrw",       0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS },
> -{"csrwi",      0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS },
> -{"csrs",       0, INSN_CLASS_ZICSR,"E,s",      MATCH_CSRRS, MASK_CSRRS|MASK_RD, match_opcode, INSN_ALIAS },
> -{"csrs",       0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS },
> -{"csrsi",      0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS },
> -{"csrc",       0, INSN_CLASS_ZICSR,"E,s",      MATCH_CSRRC, MASK_CSRRC|MASK_RD, match_opcode, INSN_ALIAS },
> -{"csrc",       0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS },
> -{"csrci",      0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS },
> -{"csrrw",      0, INSN_CLASS_ZICSR,"d,E,s",    MATCH_CSRRW, MASK_CSRRW, match_opcode, 0 },
> -{"csrrw",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRWI, MASK_CSRRWI, match_opcode, INSN_ALIAS },
> -{"csrrwi",     0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRWI, MASK_CSRRWI, match_opcode, 0 },
> -{"csrrs",      0, INSN_CLASS_ZICSR,"d,E,s",    MATCH_CSRRS, MASK_CSRRS, match_opcode, 0 },
> -{"csrrs",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRSI, MASK_CSRRSI, match_opcode, INSN_ALIAS },
> -{"csrrsi",     0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRSI, MASK_CSRRSI, match_opcode, 0 },
> -{"csrrc",      0, INSN_CLASS_ZICSR,"d,E,s",    MATCH_CSRRC, MASK_CSRRC, match_opcode, 0 },
> -{"csrrc",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRCI, MASK_CSRRCI, match_opcode, INSN_ALIAS },
> -{"csrrci",     0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRCI, MASK_CSRRCI, match_opcode, 0 },
> -{"uret",       0, INSN_CLASS_I,    "",         MATCH_URET, MASK_URET, match_opcode, 0 },
> -{"sret",       0, INSN_CLASS_I,    "",         MATCH_SRET, MASK_SRET, match_opcode, 0 },
> -{"hret",       0, INSN_CLASS_I,    "",         MATCH_HRET, MASK_HRET, match_opcode, 0 },
> -{"mret",       0, INSN_CLASS_I,    "",         MATCH_MRET, MASK_MRET, match_opcode, 0 },
> -{"dret",       0, INSN_CLASS_I,    "",         MATCH_DRET, MASK_DRET, match_opcode, 0 },
> -{"sfence.vm",  0, INSN_CLASS_I,    "",         MATCH_SFENCE_VM, MASK_SFENCE_VM | MASK_RS1, match_opcode, 0 },
> -{"sfence.vm",  0, INSN_CLASS_I,    "s",        MATCH_SFENCE_VM, MASK_SFENCE_VM, match_opcode, 0 },
> -{"sfence.vma", 0, INSN_CLASS_I,    "",         MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS },
> -{"sfence.vma", 0, INSN_CLASS_I,    "s",        MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS2, match_opcode, INSN_ALIAS },
> -{"sfence.vma", 0, INSN_CLASS_I,    "s,t",      MATCH_SFENCE_VMA, MASK_SFENCE_VMA, match_opcode, 0 },
> -{"wfi",        0, INSN_CLASS_I,    "",         MATCH_WFI, MASK_WFI, match_opcode, 0 },
> -
>  /* Zicbom and Zicboz instructions.  */
>  {"cbo.clean",  0, INSN_CLASS_ZICBOM, "0(s)", MATCH_CBO_CLEAN, MASK_CBO_CLEAN, match_opcode, 0 },
>  {"cbo.flush",  0, INSN_CLASS_ZICBOM, "0(s)", MATCH_CBO_FLUSH, MASK_CBO_FLUSH, match_opcode, 0 },
> @@ -1830,6 +1798,38 @@ const struct riscv_opcode riscv_opcodes[] =
>  {"vmv4r.v",    0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV4RV, MASK_VMV4RV, match_opcode, 0},
>  {"vmv8r.v",    0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV8RV, MASK_VMV8RV, match_opcode, 0},
>
> +/* Supervisor instructions.  */
> +{"csrr",       0, INSN_CLASS_ZICSR, "d,E",   MATCH_CSRRS, MASK_CSRRS|MASK_RS1, match_opcode, INSN_ALIAS },
> +{"csrw",       0, INSN_CLASS_ZICSR, "E,s",   MATCH_CSRRW, MASK_CSRRW|MASK_RD, match_opcode, INSN_ALIAS },
> +{"csrw",       0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS },
> +{"csrwi",      0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS },
> +{"csrs",       0, INSN_CLASS_ZICSR, "E,s",   MATCH_CSRRS, MASK_CSRRS|MASK_RD, match_opcode, INSN_ALIAS },
> +{"csrs",       0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS },
> +{"csrsi",      0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS },
> +{"csrc",       0, INSN_CLASS_ZICSR, "E,s",   MATCH_CSRRC, MASK_CSRRC|MASK_RD, match_opcode, INSN_ALIAS },
> +{"csrc",       0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS },
> +{"csrci",      0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS },
> +{"csrrw",      0, INSN_CLASS_ZICSR, "d,E,s", MATCH_CSRRW, MASK_CSRRW, match_opcode, 0 },
> +{"csrrw",      0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRWI, MASK_CSRRWI, match_opcode, INSN_ALIAS },
> +{"csrrwi",     0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRWI, MASK_CSRRWI, match_opcode, 0 },
> +{"csrrs",      0, INSN_CLASS_ZICSR, "d,E,s", MATCH_CSRRS, MASK_CSRRS, match_opcode, 0 },
> +{"csrrs",      0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRSI, MASK_CSRRSI, match_opcode, INSN_ALIAS },
> +{"csrrsi",     0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRSI, MASK_CSRRSI, match_opcode, 0 },
> +{"csrrc",      0, INSN_CLASS_ZICSR, "d,E,s", MATCH_CSRRC, MASK_CSRRC, match_opcode, 0 },
> +{"csrrc",      0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRCI, MASK_CSRRCI, match_opcode, INSN_ALIAS },
> +{"csrrci",     0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRCI, MASK_CSRRCI, match_opcode, 0 },
> +{"uret",       0, INSN_CLASS_I, "",    MATCH_URET, MASK_URET, match_opcode, 0 },
> +{"sret",       0, INSN_CLASS_I, "",    MATCH_SRET, MASK_SRET, match_opcode, 0 },
> +{"hret",       0, INSN_CLASS_I, "",    MATCH_HRET, MASK_HRET, match_opcode, 0 },
> +{"mret",       0, INSN_CLASS_I, "",    MATCH_MRET, MASK_MRET, match_opcode, 0 },
> +{"dret",       0, INSN_CLASS_I, "",    MATCH_DRET, MASK_DRET, match_opcode, 0 },
> +{"sfence.vm",  0, INSN_CLASS_I, "",    MATCH_SFENCE_VM, MASK_SFENCE_VM | MASK_RS1, match_opcode, 0 },
> +{"sfence.vm",  0, INSN_CLASS_I, "s",   MATCH_SFENCE_VM, MASK_SFENCE_VM, match_opcode, 0 },
> +{"sfence.vma", 0, INSN_CLASS_I, "",    MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS },
> +{"sfence.vma", 0, INSN_CLASS_I, "s",   MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS2, match_opcode, INSN_ALIAS },
> +{"sfence.vma", 0, INSN_CLASS_I, "s,t", MATCH_SFENCE_VMA, MASK_SFENCE_VMA, match_opcode, 0 },
> +{"wfi",        0, INSN_CLASS_I, "",    MATCH_WFI, MASK_WFI, match_opcode, 0 },
> +
>  /* Svinval instructions.  */
>  {"sinval.vma",      0, INSN_CLASS_SVINVAL, "s,t", MATCH_SINVAL_VMA, MASK_SINVAL_VMA, match_opcode, 0 },
>  {"sfence.w.inval",  0, INSN_CLASS_SVINVAL, "",    MATCH_SFENCE_W_INVAL, MASK_SFENCE_W_INVAL, match_opcode, 0 },
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] RISC-V: Move supervisor instructions after all unprivileged ones
  2022-10-01  7:45   ` Nelson Chu
@ 2022-10-01  8:15     ` Tsukasa OI
  0 siblings, 0 replies; 4+ messages in thread
From: Tsukasa OI @ 2022-10-01  8:15 UTC (permalink / raw)
  To: Nelson Chu; +Cc: binutils

On 2022/10/01 16:45, Nelson Chu wrote:
> On Sat, Oct 1, 2022 at 1:41 PM Tsukasa OI via Binutils
> <binutils@sourceware.org> wrote:
>>
>> This location of supervisor instructions is out of place (because many other
>> privileged instructions are located at the tail but after the supervisor
>> instructions, we have many unprivileged instructions including bit
>> manipulation / crypto / vector instructions).
>>
>> Not only that, this is harmful to implement pseudoinstructions in the latest
>> 'P'-extension proposal (CLROV and RDOV).  This commit moves supervisor
>> instructions after all unprivileged instructions.
> 
> OK, since I don't have a strong opinion on this.
> 
> Not to reject this patch, just a minor thought - we cannot always
> gather all extensions nearly, once some of them support the
> instructions based on others.  So maybe just place CLROV and RDOV
> before csrrs and csrrci is the easier way to do.

I agree that we cannot always gather all extensions.  Sometimes, we will
have to do just like you mentioned (on some instructions).  The reason
of moving this section is, we can satisfy all of them by doing this:

(1) Modification is self-explanatory (simple).
(2) Instruction order fits better.
(3) Placing CLROV and RDOV just before CSR instructions
    is no longer needed after doing this.

Note that I expect that 'P' instructions are placed before vector
instructions.

I submitted this patch because it can be a prerequisite for my version
of 'P'-extension implementation proposal but this idea is also
applicable to another topic: standard hints.

I think moving all standard hints (as a separate section) before any
standard instructions might be helpful since we don't need to taint
standard RVI instruction section by minor (yet standard) hints.

Thanks,
Tsukasa

> 
> Thanks
> Nelson
> 
>> opcodes/ChangeLog:
>>
>>         * riscv-opc.c (riscv_opcodes): Adjust indents.  Move supervisor
>>         instructions after all unprivileged instructions.
>> ---
>>  opcodes/riscv-opc.c | 64 ++++++++++++++++++++++-----------------------
>>  1 file changed, 32 insertions(+), 32 deletions(-)
>>
>> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
>> index 7e95f645c5c..3eb69cdc34c 100644
>> --- a/opcodes/riscv-opc.c
>> +++ b/opcodes/riscv-opc.c
>> @@ -912,38 +912,6 @@ const struct riscv_opcode riscv_opcodes[] =
>>  {"c.fswsp",   32, INSN_CLASS_F_AND_C, "CT,CM(Cc)", MATCH_C_FSWSP, MASK_C_FSWSP, match_opcode, INSN_DREF|INSN_4_BYTE },
>>  {"c.fsw",     32, INSN_CLASS_F_AND_C, "CD,Ck(Cs)", MATCH_C_FSW, MASK_C_FSW, match_opcode, INSN_DREF|INSN_4_BYTE },
>>
>> -/* Supervisor instructions.  */
>> -{"csrr",       0, INSN_CLASS_ZICSR,"d,E",      MATCH_CSRRS, MASK_CSRRS|MASK_RS1, match_opcode, INSN_ALIAS },
>> -{"csrw",       0, INSN_CLASS_ZICSR,"E,s",      MATCH_CSRRW, MASK_CSRRW|MASK_RD, match_opcode, INSN_ALIAS },
>> -{"csrw",       0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS },
>> -{"csrwi",      0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS },
>> -{"csrs",       0, INSN_CLASS_ZICSR,"E,s",      MATCH_CSRRS, MASK_CSRRS|MASK_RD, match_opcode, INSN_ALIAS },
>> -{"csrs",       0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS },
>> -{"csrsi",      0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS },
>> -{"csrc",       0, INSN_CLASS_ZICSR,"E,s",      MATCH_CSRRC, MASK_CSRRC|MASK_RD, match_opcode, INSN_ALIAS },
>> -{"csrc",       0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS },
>> -{"csrci",      0, INSN_CLASS_ZICSR,"E,Z",      MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS },
>> -{"csrrw",      0, INSN_CLASS_ZICSR,"d,E,s",    MATCH_CSRRW, MASK_CSRRW, match_opcode, 0 },
>> -{"csrrw",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRWI, MASK_CSRRWI, match_opcode, INSN_ALIAS },
>> -{"csrrwi",     0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRWI, MASK_CSRRWI, match_opcode, 0 },
>> -{"csrrs",      0, INSN_CLASS_ZICSR,"d,E,s",    MATCH_CSRRS, MASK_CSRRS, match_opcode, 0 },
>> -{"csrrs",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRSI, MASK_CSRRSI, match_opcode, INSN_ALIAS },
>> -{"csrrsi",     0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRSI, MASK_CSRRSI, match_opcode, 0 },
>> -{"csrrc",      0, INSN_CLASS_ZICSR,"d,E,s",    MATCH_CSRRC, MASK_CSRRC, match_opcode, 0 },
>> -{"csrrc",      0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRCI, MASK_CSRRCI, match_opcode, INSN_ALIAS },
>> -{"csrrci",     0, INSN_CLASS_ZICSR,"d,E,Z",    MATCH_CSRRCI, MASK_CSRRCI, match_opcode, 0 },
>> -{"uret",       0, INSN_CLASS_I,    "",         MATCH_URET, MASK_URET, match_opcode, 0 },
>> -{"sret",       0, INSN_CLASS_I,    "",         MATCH_SRET, MASK_SRET, match_opcode, 0 },
>> -{"hret",       0, INSN_CLASS_I,    "",         MATCH_HRET, MASK_HRET, match_opcode, 0 },
>> -{"mret",       0, INSN_CLASS_I,    "",         MATCH_MRET, MASK_MRET, match_opcode, 0 },
>> -{"dret",       0, INSN_CLASS_I,    "",         MATCH_DRET, MASK_DRET, match_opcode, 0 },
>> -{"sfence.vm",  0, INSN_CLASS_I,    "",         MATCH_SFENCE_VM, MASK_SFENCE_VM | MASK_RS1, match_opcode, 0 },
>> -{"sfence.vm",  0, INSN_CLASS_I,    "s",        MATCH_SFENCE_VM, MASK_SFENCE_VM, match_opcode, 0 },
>> -{"sfence.vma", 0, INSN_CLASS_I,    "",         MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS },
>> -{"sfence.vma", 0, INSN_CLASS_I,    "s",        MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS2, match_opcode, INSN_ALIAS },
>> -{"sfence.vma", 0, INSN_CLASS_I,    "s,t",      MATCH_SFENCE_VMA, MASK_SFENCE_VMA, match_opcode, 0 },
>> -{"wfi",        0, INSN_CLASS_I,    "",         MATCH_WFI, MASK_WFI, match_opcode, 0 },
>> -
>>  /* Zicbom and Zicboz instructions.  */
>>  {"cbo.clean",  0, INSN_CLASS_ZICBOM, "0(s)", MATCH_CBO_CLEAN, MASK_CBO_CLEAN, match_opcode, 0 },
>>  {"cbo.flush",  0, INSN_CLASS_ZICBOM, "0(s)", MATCH_CBO_FLUSH, MASK_CBO_FLUSH, match_opcode, 0 },
>> @@ -1830,6 +1798,38 @@ const struct riscv_opcode riscv_opcodes[] =
>>  {"vmv4r.v",    0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV4RV, MASK_VMV4RV, match_opcode, 0},
>>  {"vmv8r.v",    0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV8RV, MASK_VMV8RV, match_opcode, 0},
>>
>> +/* Supervisor instructions.  */
>> +{"csrr",       0, INSN_CLASS_ZICSR, "d,E",   MATCH_CSRRS, MASK_CSRRS|MASK_RS1, match_opcode, INSN_ALIAS },
>> +{"csrw",       0, INSN_CLASS_ZICSR, "E,s",   MATCH_CSRRW, MASK_CSRRW|MASK_RD, match_opcode, INSN_ALIAS },
>> +{"csrw",       0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS },
>> +{"csrwi",      0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS },
>> +{"csrs",       0, INSN_CLASS_ZICSR, "E,s",   MATCH_CSRRS, MASK_CSRRS|MASK_RD, match_opcode, INSN_ALIAS },
>> +{"csrs",       0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS },
>> +{"csrsi",      0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS },
>> +{"csrc",       0, INSN_CLASS_ZICSR, "E,s",   MATCH_CSRRC, MASK_CSRRC|MASK_RD, match_opcode, INSN_ALIAS },
>> +{"csrc",       0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS },
>> +{"csrci",      0, INSN_CLASS_ZICSR, "E,Z",   MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS },
>> +{"csrrw",      0, INSN_CLASS_ZICSR, "d,E,s", MATCH_CSRRW, MASK_CSRRW, match_opcode, 0 },
>> +{"csrrw",      0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRWI, MASK_CSRRWI, match_opcode, INSN_ALIAS },
>> +{"csrrwi",     0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRWI, MASK_CSRRWI, match_opcode, 0 },
>> +{"csrrs",      0, INSN_CLASS_ZICSR, "d,E,s", MATCH_CSRRS, MASK_CSRRS, match_opcode, 0 },
>> +{"csrrs",      0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRSI, MASK_CSRRSI, match_opcode, INSN_ALIAS },
>> +{"csrrsi",     0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRSI, MASK_CSRRSI, match_opcode, 0 },
>> +{"csrrc",      0, INSN_CLASS_ZICSR, "d,E,s", MATCH_CSRRC, MASK_CSRRC, match_opcode, 0 },
>> +{"csrrc",      0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRCI, MASK_CSRRCI, match_opcode, INSN_ALIAS },
>> +{"csrrci",     0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRCI, MASK_CSRRCI, match_opcode, 0 },
>> +{"uret",       0, INSN_CLASS_I, "",    MATCH_URET, MASK_URET, match_opcode, 0 },
>> +{"sret",       0, INSN_CLASS_I, "",    MATCH_SRET, MASK_SRET, match_opcode, 0 },
>> +{"hret",       0, INSN_CLASS_I, "",    MATCH_HRET, MASK_HRET, match_opcode, 0 },
>> +{"mret",       0, INSN_CLASS_I, "",    MATCH_MRET, MASK_MRET, match_opcode, 0 },
>> +{"dret",       0, INSN_CLASS_I, "",    MATCH_DRET, MASK_DRET, match_opcode, 0 },
>> +{"sfence.vm",  0, INSN_CLASS_I, "",    MATCH_SFENCE_VM, MASK_SFENCE_VM | MASK_RS1, match_opcode, 0 },
>> +{"sfence.vm",  0, INSN_CLASS_I, "s",   MATCH_SFENCE_VM, MASK_SFENCE_VM, match_opcode, 0 },
>> +{"sfence.vma", 0, INSN_CLASS_I, "",    MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS },
>> +{"sfence.vma", 0, INSN_CLASS_I, "s",   MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS2, match_opcode, INSN_ALIAS },
>> +{"sfence.vma", 0, INSN_CLASS_I, "s,t", MATCH_SFENCE_VMA, MASK_SFENCE_VMA, match_opcode, 0 },
>> +{"wfi",        0, INSN_CLASS_I, "",    MATCH_WFI, MASK_WFI, match_opcode, 0 },
>> +
>>  /* Svinval instructions.  */
>>  {"sinval.vma",      0, INSN_CLASS_SVINVAL, "s,t", MATCH_SINVAL_VMA, MASK_SINVAL_VMA, match_opcode, 0 },
>>  {"sfence.w.inval",  0, INSN_CLASS_SVINVAL, "",    MATCH_SFENCE_W_INVAL, MASK_SFENCE_W_INVAL, match_opcode, 0 },
>> --
>> 2.34.1
>>
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-10-01  8:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-01  5:39 [PATCH 0/1] RISC-V: Move supervisor instructions after all unprivileged ones Tsukasa OI
2022-10-01  5:39 ` [PATCH 1/1] " Tsukasa OI
2022-10-01  7:45   ` Nelson Chu
2022-10-01  8:15     ` Tsukasa OI

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).