[PATCH 0/1] RISC-V: Add `OP_V' to .insn named opcodes

[PATCH 0/1] RISC-V: Add `OP_V' to .insn named opcodes

[Tsukasa OI]

Hello,

This small patch adds OP_V to named opcode list for .insn directive.

Tracker on GitHub:
<https://github.com/a4lg/binutils-gdb/wiki/riscv_gas_insn_opv>

    Sidenote:
    I started listing my Binutils submissions on my GitHub Wiki:
    <https://github.com/a4lg/binutils-gdb/wiki/Patch-Queue>
    hoping that current status and conflicting patches are clear.


OP-V (0x57) is an opcode for vector instructions (defined on now
ratified V extension).  It adds OP_V to named constants of .insn directive
(note that replacing - with _ is standard on GNU Binutils' .insn).

Although vector instruction encoding is not implemented in .insn directive,
it will help future implementation of custom vector .insn.

If Zp* extensions are ratified, we could add OP_P (0x77) likewise.


Thanks,
Tsukasa




Tsukasa OI (1):
  RISC-V: Add `OP_V' to .insn named opcodes

 gas/config/tc-riscv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: d2acd4b0c5bab349aaa152d60268bc144634a844
-- 
2.34.1

[PATCH 1/1] RISC-V: Add `OP_V' to .insn named opcodes

[Tsukasa OI]

This commit adds `OP_V' (OP-V: vector instruction opcode for now
ratified `V' extension) to .insn opcode name list.  Although vector
instruction encoding is not implemented in `.insn' directive, it will
help future implementation of custom vector `.insn'.

gas/ChangeLog:

	* config/tc-riscv.c (opcode_name_list): Add `OP_V'.
---
 gas/config/tc-riscv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index a0e8456a0d1..1c53e66f9f5 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -749,7 +749,7 @@ static const struct opcode_name_t opcode_name_list[] =
   {"NMADD",     0x4f},
   {"NMSUB",     0x4b},
   {"OP_FP",     0x53},
-  /*reserved    0x57.  */
+  {"OP_V",      0x57},
   {"CUSTOM_2",  0x5b},
   /* 48b        0x5f.  */
 
-- 
2.34.1

Re: [PATCH 0/1] RISC-V: Add `OP_V' to .insn named opcodes

[Tsukasa OI]

Ping.

I noticed that LLVM (14 or later) already implements this named opcode.

sample.s:
    .attribute arch, "rv64gcv"
    # Both vadd.vv v1,v2,v3
    .insn r 0x57, 0, 1, x1, x3, x2
    .insn r OP_V, 0, 1, x1, x3, x2

$ llvm-mc -filetype=obj -triple=riscv64 -o sample.o sample.s
$ riscv64-unknown-elf-objdump -d sample.o
(...)
0000000000000000 <.text>:
   0:   022180d7                vadd.vv v1,v2,v3
   4:   022180d7                vadd.vv v1,v2,v3

Even after this patch, following assembly file generates an error
(unlike LLVM).  I'm going to raise an issue later.

sample2.s:
    .attribute arch, "rv64gcv"
    # vadd.vv v1,v2,v3 (okay on LLVM but error on Binutils)
    .insn r OP_V, 0, 1, v1, v3, v2

Thanks,
Tsukasa


On 2022/07/09 12:51, Tsukasa OI wrote:
> Hello,
> 
> This small patch adds OP_V to named opcode list for .insn directive.
> 
> Tracker on GitHub:
> <https://github.com/a4lg/binutils-gdb/wiki/riscv_gas_insn_opv>
> 
>     Sidenote:
>     I started listing my Binutils submissions on my GitHub Wiki:
>     <https://github.com/a4lg/binutils-gdb/wiki/Patch-Queue>
>     hoping that current status and conflicting patches are clear.
> 
> 
> OP-V (0x57) is an opcode for vector instructions (defined on now
> ratified V extension).  It adds OP_V to named constants of .insn directive
> (note that replacing - with _ is standard on GNU Binutils' .insn).
> 
> Although vector instruction encoding is not implemented in .insn directive,
> it will help future implementation of custom vector .insn.
> 
> If Zp* extensions are ratified, we could add OP_P (0x77) likewise.
> 
> 
> Thanks,
> Tsukasa
> 
> 
> 
> 
> Tsukasa OI (1):
>   RISC-V: Add `OP_V' to .insn named opcodes
> 
>  gas/config/tc-riscv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> base-commit: d2acd4b0c5bab349aaa152d60268bc144634a844

Re: [PATCH 0/1] RISC-V: Add `OP_V' to .insn named opcodes

[Kito Cheng]

Hi Tsukasa:

Could you add your sample.s as testcase?

Thanks

On Thu, Jul 28, 2022 at 7:46 PM Tsukasa OI <research_trasio@irq.a4lg.com>
wrote:

> Ping.
>
> I noticed that LLVM (14 or later) already implements this named opcode.
>
> sample.s:
>     .attribute arch, "rv64gcv"
>     # Both vadd.vv v1,v2,v3
>     .insn r 0x57, 0, 1, x1, x3, x2
>     .insn r OP_V, 0, 1, x1, x3, x2
>
> $ llvm-mc -filetype=obj -triple=riscv64 -o sample.o sample.s
> $ riscv64-unknown-elf-objdump -d sample.o
> (...)
> 0000000000000000 <.text>:
>    0:   022180d7                vadd.vv v1,v2,v3
>    4:   022180d7                vadd.vv v1,v2,v3
>
> Even after this patch, following assembly file generates an error
> (unlike LLVM).  I'm going to raise an issue later.
>
> sample2.s:
>     .attribute arch, "rv64gcv"
>     # vadd.vv v1,v2,v3 (okay on LLVM but error on Binutils)
>     .insn r OP_V, 0, 1, v1, v3, v2
>
> Thanks,
> Tsukasa
>
>
> On 2022/07/09 12:51, Tsukasa OI wrote:
> > Hello,
> >
> > This small patch adds OP_V to named opcode list for .insn directive.
> >
> > Tracker on GitHub:
> > <https://github.com/a4lg/binutils-gdb/wiki/riscv_gas_insn_opv>
> >
> >     Sidenote:
> >     I started listing my Binutils submissions on my GitHub Wiki:
> >     <https://github.com/a4lg/binutils-gdb/wiki/Patch-Queue>
> >     hoping that current status and conflicting patches are clear.
> >
> >
> > OP-V (0x57) is an opcode for vector instructions (defined on now
> > ratified V extension).  It adds OP_V to named constants of .insn
> directive
> > (note that replacing - with _ is standard on GNU Binutils' .insn).
> >
> > Although vector instruction encoding is not implemented in .insn
> directive,
> > it will help future implementation of custom vector .insn.
> >
> > If Zp* extensions are ratified, we could add OP_P (0x77) likewise.
> >
> >
> > Thanks,
> > Tsukasa
> >
> >
> >
> >
> > Tsukasa OI (1):
> >   RISC-V: Add `OP_V' to .insn named opcodes
> >
> >  gas/config/tc-riscv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >
> > base-commit: d2acd4b0c5bab349aaa152d60268bc144634a844
>

[PATCH v2 0/1] RISC-V: Add `OP_V' to .insn named opcodes

[Tsukasa OI]

Hello,

This small patch adds OP_V to named opcode list for .insn directive.

Changes (v1 -> v2):
-   Rebase
-   Add testcase


Tracker on GitHub:
<https://github.com/a4lg/binutils-gdb/wiki/riscv_gas_insn_opv>

    Sidenote:
    I started listing my Binutils submissions on my GitHub Wiki:
    <https://github.com/a4lg/binutils-gdb/wiki/Patch-Queue>
    hoping that current status and conflicting patches are clear.


OP-V (0x57) is an opcode for vector instructions (defined on now
ratified V extension).  It adds OP_V to named constants of .insn directive
(note that replacing - with _ is standard on GNU Binutils' .insn).

Although vector instruction encoding is not implemented in .insn directive,
it will help future implementation of custom vector .insn.

If Zp* extensions are ratified, we could add OP_P (0x77) likewise.


Thanks,
Tsukasa




Tsukasa OI (1):
  RISC-V: Add `OP_V' to .insn named opcodes

 gas/config/tc-riscv.c                |  2 +-
 gas/testsuite/gas/riscv/insn-dwarf.d | 17 +++++++++--------
 gas/testsuite/gas/riscv/insn.d       |  3 ++-
 gas/testsuite/gas/riscv/insn.s       |  2 ++
 4 files changed, 14 insertions(+), 10 deletions(-)


base-commit: 2eb132bdfb915c9721294871ce857c4dad522ce1
-- 
2.34.1

[PATCH v2 1/1] RISC-V: Add `OP_V' to .insn named opcodes

[Tsukasa OI]

This commit adds `OP_V' (OP-V: vector instruction opcode for now
ratified `V' extension) to .insn opcode name list.  Although vector
instruction encoding is not implemented in `.insn' directive, it will
help future implementation of custom vector `.insn'.

gas/ChangeLog:

	* config/tc-riscv.c (opcode_name_list): Add `OP_V'.
	* testsuite/gas/riscv/insn.s: Add testcase.
	* testsuite/gas/riscv/insn.d: Likewise.
	* testsuite/gas/riscv/insn-dwarf.d: Reflect insn.s update.
---
 gas/config/tc-riscv.c                |  2 +-
 gas/testsuite/gas/riscv/insn-dwarf.d | 17 +++++++++--------
 gas/testsuite/gas/riscv/insn.d       |  3 ++-
 gas/testsuite/gas/riscv/insn.s       |  2 ++
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 291d07f6d8f..ecdffbef589 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -749,7 +749,7 @@ static const struct opcode_name_t opcode_name_list[] =
   {"NMADD",     0x4f},
   {"NMSUB",     0x4b},
   {"OP_FP",     0x53},
-  /*reserved    0x57.  */
+  {"OP_V",      0x57},
   {"CUSTOM_2",  0x5b},
   /* 48b        0x5f.  */
 
diff --git a/gas/testsuite/gas/riscv/insn-dwarf.d b/gas/testsuite/gas/riscv/insn-dwarf.d
index 72d54d47acf..a975b1464c0 100644
--- a/gas/testsuite/gas/riscv/insn-dwarf.d
+++ b/gas/testsuite/gas/riscv/insn-dwarf.d
@@ -60,12 +60,13 @@ insn.s +53 +0x9a.*
 insn.s +54 +0x9e.*
 insn.s +55 +0xa2.*
 insn.s +57 +0xa6.*
-insn.s +58 +0xa8.*
-insn.s +59 +0xac.*
-insn.s +60 +0xb2.*
-insn.s +61 +0xba.*
-insn.s +62 +0xbc.*
-insn.s +63 +0xc0.*
-insn.s +64 +0xc6.*
-insn.s +- +0xce
+insn.s +59 +0xaa.*
+insn.s +60 +0xac.*
+insn.s +61 +0xb0.*
+insn.s +62 +0xb6.*
+insn.s +63 +0xbe.*
+insn.s +64 +0xc0.*
+insn.s +65 +0xc4.*
+insn.s +66 +0xca.*
+insn.s +- +0xd2
 #pass
diff --git a/gas/testsuite/gas/riscv/insn.d b/gas/testsuite/gas/riscv/insn.d
index b5780f42861..d177f313d23 100644
--- a/gas/testsuite/gas/riscv/insn.d
+++ b/gas/testsuite/gas/riscv/insn.d
@@ -1,4 +1,4 @@
-#as: -march=rv32ifc
+#as: -march=rv32ifdcv
 #objdump: -dr
 
 .*:[ 	]+file format .*
@@ -69,6 +69,7 @@ Disassembly of section .text:
 [^:]+:[ 	]+00c58533[ 	]+add[ 	]+a0,a1,a2
 [^:]+:[ 	]+00c58533[ 	]+add[ 	]+a0,a1,a2
 [^:]+:[ 	]+00c58533[ 	]+add[ 	]+a0,a1,a2
+[^:]+:[ 	]+022180d7[ 	]+vadd\.vv[ 	]+v1,v2,v3
 [^:]+:[ 	]+0001[ 	]+nop
 [^:]+:[ 	]+00000013[ 	]+nop
 [^:]+:[ 	]+001f 0000 0000[ 	].*
diff --git a/gas/testsuite/gas/riscv/insn.s b/gas/testsuite/gas/riscv/insn.s
index ec41acb30bd..993615eb747 100644
--- a/gas/testsuite/gas/riscv/insn.s
+++ b/gas/testsuite/gas/riscv/insn.s
@@ -54,6 +54,8 @@ target:
 	.insn r  0x33,  0,  0, a0, fa1, fa2
 	.insn r  0x33,  0,  0, fa0, fa1, fa2
 
+	.insn r  OP_V, 0, 1, x1, x3, x2
+
 	.insn 0x0001
 	.insn 0x00000013
 	.insn 0x0000001f
-- 
2.34.1

Re: [PATCH v2 1/1] RISC-V: Add `OP_V' to .insn named opcodes

[Kito Cheng]

LGTM

On Thu, Jul 28, 2022 at 9:03 PM Tsukasa OI via Binutils
<binutils@sourceware.org> wrote:
>
> This commit adds `OP_V' (OP-V: vector instruction opcode for now
> ratified `V' extension) to .insn opcode name list.  Although vector
> instruction encoding is not implemented in `.insn' directive, it will
> help future implementation of custom vector `.insn'.
>
> gas/ChangeLog:
>
>         * config/tc-riscv.c (opcode_name_list): Add `OP_V'.
>         * testsuite/gas/riscv/insn.s: Add testcase.
>         * testsuite/gas/riscv/insn.d: Likewise.
>         * testsuite/gas/riscv/insn-dwarf.d: Reflect insn.s update.
> ---
>  gas/config/tc-riscv.c                |  2 +-
>  gas/testsuite/gas/riscv/insn-dwarf.d | 17 +++++++++--------
>  gas/testsuite/gas/riscv/insn.d       |  3 ++-
>  gas/testsuite/gas/riscv/insn.s       |  2 ++
>  4 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 291d07f6d8f..ecdffbef589 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -749,7 +749,7 @@ static const struct opcode_name_t opcode_name_list[] =
>    {"NMADD",     0x4f},
>    {"NMSUB",     0x4b},
>    {"OP_FP",     0x53},
> -  /*reserved    0x57.  */
> +  {"OP_V",      0x57},
>    {"CUSTOM_2",  0x5b},
>    /* 48b        0x5f.  */
>
> diff --git a/gas/testsuite/gas/riscv/insn-dwarf.d b/gas/testsuite/gas/riscv/insn-dwarf.d
> index 72d54d47acf..a975b1464c0 100644
> --- a/gas/testsuite/gas/riscv/insn-dwarf.d
> +++ b/gas/testsuite/gas/riscv/insn-dwarf.d
> @@ -60,12 +60,13 @@ insn.s +53 +0x9a.*
>  insn.s +54 +0x9e.*
>  insn.s +55 +0xa2.*
>  insn.s +57 +0xa6.*
> -insn.s +58 +0xa8.*
> -insn.s +59 +0xac.*
> -insn.s +60 +0xb2.*
> -insn.s +61 +0xba.*
> -insn.s +62 +0xbc.*
> -insn.s +63 +0xc0.*
> -insn.s +64 +0xc6.*
> -insn.s +- +0xce
> +insn.s +59 +0xaa.*
> +insn.s +60 +0xac.*
> +insn.s +61 +0xb0.*
> +insn.s +62 +0xb6.*
> +insn.s +63 +0xbe.*
> +insn.s +64 +0xc0.*
> +insn.s +65 +0xc4.*
> +insn.s +66 +0xca.*
> +insn.s +- +0xd2
>  #pass
> diff --git a/gas/testsuite/gas/riscv/insn.d b/gas/testsuite/gas/riscv/insn.d
> index b5780f42861..d177f313d23 100644
> --- a/gas/testsuite/gas/riscv/insn.d
> +++ b/gas/testsuite/gas/riscv/insn.d
> @@ -1,4 +1,4 @@
> -#as: -march=rv32ifc
> +#as: -march=rv32ifdcv
>  #objdump: -dr
>
>  .*:[   ]+file format .*
> @@ -69,6 +69,7 @@ Disassembly of section .text:
>  [^:]+:[        ]+00c58533[     ]+add[  ]+a0,a1,a2
>  [^:]+:[        ]+00c58533[     ]+add[  ]+a0,a1,a2
>  [^:]+:[        ]+00c58533[     ]+add[  ]+a0,a1,a2
> +[^:]+:[        ]+022180d7[     ]+vadd\.vv[     ]+v1,v2,v3
>  [^:]+:[        ]+0001[         ]+nop
>  [^:]+:[        ]+00000013[     ]+nop
>  [^:]+:[        ]+001f 0000 0000[       ].*
> diff --git a/gas/testsuite/gas/riscv/insn.s b/gas/testsuite/gas/riscv/insn.s
> index ec41acb30bd..993615eb747 100644
> --- a/gas/testsuite/gas/riscv/insn.s
> +++ b/gas/testsuite/gas/riscv/insn.s
> @@ -54,6 +54,8 @@ target:
>         .insn r  0x33,  0,  0, a0, fa1, fa2
>         .insn r  0x33,  0,  0, fa0, fa1, fa2
>
> +       .insn r  OP_V, 0, 1, x1, x3, x2
> +
>         .insn 0x0001
>         .insn 0x00000013
>         .insn 0x0000001f
> --
> 2.34.1
>

Re: [PATCH v2 1/1] RISC-V: Add `OP_V' to .insn named opcodes

[Nelson Chu]

On Thu, Jul 28, 2022 at 9:29 PM Kito Cheng <kito.cheng@gmail.com> wrote:
>
> LGTM

Thanks for helping reviewing.

> On Thu, Jul 28, 2022 at 9:03 PM Tsukasa OI via Binutils
> <binutils@sourceware.org> wrote:
> >
> > This commit adds `OP_V' (OP-V: vector instruction opcode for now
> > ratified `V' extension) to .insn opcode name list.  Although vector
> > instruction encoding is not implemented in `.insn' directive, it will
> > help future implementation of custom vector `.insn'.

Yes, the original discussion was raised by Jim, and here is the link,
https://github.com/riscv/riscv-v-spec/issues/93

As mentioned in the link, we also have similar supports in the riscv github,
https://github.com/riscv-collab/riscv-binutils-gdb/pull/184/files

Using r type here makes sense, since we don't have any instruction
format names for rvv.  However, maybe it is worth adding Vd/Vs/Vt
registers for r type when v is enabled, just like what we did for fpr
registers.  But I think it is minor, and we can add it in the future
if needed, so this patch is committed by passing the regressions.

Thanks
Nelson

> > gas/ChangeLog:
> >
> >         * config/tc-riscv.c (opcode_name_list): Add `OP_V'.
> >         * testsuite/gas/riscv/insn.s: Add testcase.
> >         * testsuite/gas/riscv/insn.d: Likewise.
> >         * testsuite/gas/riscv/insn-dwarf.d: Reflect insn.s update.
> > ---
> >  gas/config/tc-riscv.c                |  2 +-
> >  gas/testsuite/gas/riscv/insn-dwarf.d | 17 +++++++++--------
> >  gas/testsuite/gas/riscv/insn.d       |  3 ++-
> >  gas/testsuite/gas/riscv/insn.s       |  2 ++
> >  4 files changed, 14 insertions(+), 10 deletions(-)
> >
> > diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> > index 291d07f6d8f..ecdffbef589 100644
> > --- a/gas/config/tc-riscv.c
> > +++ b/gas/config/tc-riscv.c
> > @@ -749,7 +749,7 @@ static const struct opcode_name_t opcode_name_list[] =
> >    {"NMADD",     0x4f},
> >    {"NMSUB",     0x4b},
> >    {"OP_FP",     0x53},
> > -  /*reserved    0x57.  */
> > +  {"OP_V",      0x57},
> >    {"CUSTOM_2",  0x5b},
> >    /* 48b        0x5f.  */
> >
> > diff --git a/gas/testsuite/gas/riscv/insn-dwarf.d b/gas/testsuite/gas/riscv/insn-dwarf.d
> > index 72d54d47acf..a975b1464c0 100644
> > --- a/gas/testsuite/gas/riscv/insn-dwarf.d
> > +++ b/gas/testsuite/gas/riscv/insn-dwarf.d
> > @@ -60,12 +60,13 @@ insn.s +53 +0x9a.*
> >  insn.s +54 +0x9e.*
> >  insn.s +55 +0xa2.*
> >  insn.s +57 +0xa6.*
> > -insn.s +58 +0xa8.*
> > -insn.s +59 +0xac.*
> > -insn.s +60 +0xb2.*
> > -insn.s +61 +0xba.*
> > -insn.s +62 +0xbc.*
> > -insn.s +63 +0xc0.*
> > -insn.s +64 +0xc6.*
> > -insn.s +- +0xce
> > +insn.s +59 +0xaa.*
> > +insn.s +60 +0xac.*
> > +insn.s +61 +0xb0.*
> > +insn.s +62 +0xb6.*
> > +insn.s +63 +0xbe.*
> > +insn.s +64 +0xc0.*
> > +insn.s +65 +0xc4.*
> > +insn.s +66 +0xca.*
> > +insn.s +- +0xd2
> >  #pass
> > diff --git a/gas/testsuite/gas/riscv/insn.d b/gas/testsuite/gas/riscv/insn.d
> > index b5780f42861..d177f313d23 100644
> > --- a/gas/testsuite/gas/riscv/insn.d
> > +++ b/gas/testsuite/gas/riscv/insn.d
> > @@ -1,4 +1,4 @@
> > -#as: -march=rv32ifc
> > +#as: -march=rv32ifdcv

Just a minor thing that we don't need to add d-ext here.

> >  #objdump: -dr
> >
> >  .*:[   ]+file format .*
> > @@ -69,6 +69,7 @@ Disassembly of section .text:
> >  [^:]+:[        ]+00c58533[     ]+add[  ]+a0,a1,a2
> >  [^:]+:[        ]+00c58533[     ]+add[  ]+a0,a1,a2
> >  [^:]+:[        ]+00c58533[     ]+add[  ]+a0,a1,a2
> > +[^:]+:[        ]+022180d7[     ]+vadd\.vv[     ]+v1,v2,v3
> >  [^:]+:[        ]+0001[         ]+nop
> >  [^:]+:[        ]+00000013[     ]+nop
> >  [^:]+:[        ]+001f 0000 0000[       ].*
> > diff --git a/gas/testsuite/gas/riscv/insn.s b/gas/testsuite/gas/riscv/insn.s
> > index ec41acb30bd..993615eb747 100644
> > --- a/gas/testsuite/gas/riscv/insn.s
> > +++ b/gas/testsuite/gas/riscv/insn.s
> > @@ -54,6 +54,8 @@ target:
> >         .insn r  0x33,  0,  0, a0, fa1, fa2
> >         .insn r  0x33,  0,  0, fa0, fa1, fa2
> >
> > +       .insn r  OP_V, 0, 1, x1, x3, x2
> > +
> >         .insn 0x0001
> >         .insn 0x00000013
> >         .insn 0x0000001f
> > --
> > 2.34.1
> >