public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/1] RISC-V: Make XVentanaCondOps RV64 only
@ 2023-08-30  1:38 Tsukasa OI
  2023-08-30  1:38 ` [PATCH 1/1] " Tsukasa OI
  2023-08-30  3:21 ` [PATCH 0/1] " Greg Favor
  0 siblings, 2 replies; 6+ messages in thread
From: Tsukasa OI @ 2023-08-30  1:38 UTC (permalink / raw)
  To: Tsukasa OI, Palmer Dabbelt, Andrew Waterman, Jim Wilson,
	Nelson Chu, Kito Cheng, Jeff Law, Greg Favor
  Cc: binutils

Hello,

I noticed that two instructions in the XVentanaCondOps vendor extension --
"vt.maskc" and "vt.maskcn" -- are defined for all XLEN values.

This is against the manual and LLVM.


1. The instruction manual

<https://github.com/ventanamicro/ventana-custom-extensions/releases/download/v1.0.0/ventana-custom-extensions-v1.0.0.pdf>

Currently defines the instructions only for RV64.

> All current cores by Ventana Micro implement RV64 and are designed as
> 64-bit only, the RV32-column is marked "n/a".

But it also says it's (in theory) XLEN-agonistic.

> The instructions in the XVentanaCondOps extension are defined to operate
> on XLEN and would thus be directly applicable to RV32.


2. LLVM (llvm/lib/Target/RISCV/RISCVInstrInfoXVentana.td)

> let Predicates = [IsRV64, HasVendorXVentanaCondOps]

It indicates that XVentanaCondOps instructions are only enabled on RV64.



Unless Ventana is working on some RV32 processors (and soon to be released
), I think disabling XVentanaCondOps instructions on RV32 would be safer
(to prevent possible misuses).  I would like to hear thoughts especially
from Ventana employees since I am just a volunteer.

I also chose not to reject XVentanaCondOps + RV32 because it is not stated
that is illegal (unlike Zcf + RV64).  This patch set makes XVentanaCondOps
+ RV32 empty (yet legal).


Sincerely,
Tsukasa




Tsukasa OI (1):
  RISC-V: Make XVentanaCondOps RV64 only

 gas/testsuite/gas/riscv/x-ventana-condops-32.d | 3 +++
 gas/testsuite/gas/riscv/x-ventana-condops-32.l | 3 +++
 opcodes/riscv-opc.c                            | 4 ++--
 3 files changed, 8 insertions(+), 2 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/x-ventana-condops-32.d
 create mode 100644 gas/testsuite/gas/riscv/x-ventana-condops-32.l


base-commit: 0637da3c7325b28a2c05f016d7f290513b1cd19b
-- 
2.42.0


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

* [PATCH 1/1] RISC-V: Make XVentanaCondOps RV64 only
  2023-08-30  1:38 [PATCH 0/1] RISC-V: Make XVentanaCondOps RV64 only Tsukasa OI
@ 2023-08-30  1:38 ` Tsukasa OI
  2023-08-30  2:45   ` Nelson Chu
  2023-08-30  3:21 ` [PATCH 0/1] " Greg Favor
  1 sibling, 1 reply; 6+ messages in thread
From: Tsukasa OI @ 2023-08-30  1:38 UTC (permalink / raw)
  To: Tsukasa OI, Palmer Dabbelt, Andrew Waterman, Jim Wilson,
	Nelson Chu, Kito Cheng, Jeff Law, Greg Favor
  Cc: binutils

From: Tsukasa OI <research_trasio@irq.a4lg.com>

Although XVentanaCondOps instructions are XLEN-agonistic, Ventana's manual
only defines them only for RV64 (because all Ventana's processors implement
RV64).

This commit limits XVentanaCondOps instructions RV64-only to match the
behavior of the manual and LLVM.

Note that this commit alone will not make XVentanaCondOps extension with
RV32 invalid (it just makes XVentanaCondOps on RV32 empty).

opcodes/ChangeLog:

	* riscv-opc.c (riscv_opcodes): Restrict "vt.maskc" and "vt.maskcn"
	to XLEN=64.

gas/ChangeLog:

	* testsuite/gas/riscv/x-ventana-condops-32.d: New failure test.
	* testsuite/gas/riscv/x-ventana-condops-32.l: Likewise.
---
 gas/testsuite/gas/riscv/x-ventana-condops-32.d | 3 +++
 gas/testsuite/gas/riscv/x-ventana-condops-32.l | 3 +++
 opcodes/riscv-opc.c                            | 4 ++--
 3 files changed, 8 insertions(+), 2 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/x-ventana-condops-32.d
 create mode 100644 gas/testsuite/gas/riscv/x-ventana-condops-32.l

diff --git a/gas/testsuite/gas/riscv/x-ventana-condops-32.d b/gas/testsuite/gas/riscv/x-ventana-condops-32.d
new file mode 100644
index 000000000000..ea67515da0e3
--- /dev/null
+++ b/gas/testsuite/gas/riscv/x-ventana-condops-32.d
@@ -0,0 +1,3 @@
+#as: -march=rv32i_xventanacondops
+#source: x-ventana-condops.s
+#error_output: x-ventana-condops-32.l
diff --git a/gas/testsuite/gas/riscv/x-ventana-condops-32.l b/gas/testsuite/gas/riscv/x-ventana-condops-32.l
new file mode 100644
index 000000000000..e434caf15f60
--- /dev/null
+++ b/gas/testsuite/gas/riscv/x-ventana-condops-32.l
@@ -0,0 +1,3 @@
+.*Assembler messages:
+.*Error: unrecognized opcode `vt.maskc a0,a1,a2'
+.*Error: unrecognized opcode `vt.maskcn a0,a3,a4'
diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
index 067e9fdb611f..f5416605dcc3 100644
--- a/opcodes/riscv-opc.c
+++ b/opcodes/riscv-opc.c
@@ -2174,8 +2174,8 @@ const struct riscv_opcode riscv_opcodes[] =
 {"th.sync.s",        0, INSN_CLASS_XTHEADSYNC,  "",   MATCH_TH_SYNC_S,        MASK_TH_SYNC_S,        match_opcode, 0},
 
 /* Vendor-specific (Ventana Microsystems) XVentanaCondOps instructions */
-{"vt.maskc",    0, INSN_CLASS_XVENTANACONDOPS, "d,s,t", MATCH_VT_MASKC, MASK_VT_MASKC, match_opcode, 0 },
-{"vt.maskcn",   0, INSN_CLASS_XVENTANACONDOPS, "d,s,t", MATCH_VT_MASKCN, MASK_VT_MASKCN, match_opcode, 0 },
+{"vt.maskc",   64, INSN_CLASS_XVENTANACONDOPS, "d,s,t", MATCH_VT_MASKC, MASK_VT_MASKC, match_opcode, 0 },
+{"vt.maskcn",  64, INSN_CLASS_XVENTANACONDOPS, "d,s,t", MATCH_VT_MASKCN, MASK_VT_MASKCN, match_opcode, 0 },
 
 /* Terminate the list.  */
 {0, 0, INSN_CLASS_NONE, 0, 0, 0, 0, 0}
-- 
2.42.0


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

* Re: [PATCH 1/1] RISC-V: Make XVentanaCondOps RV64 only
  2023-08-30  1:38 ` [PATCH 1/1] " Tsukasa OI
@ 2023-08-30  2:45   ` Nelson Chu
  2023-08-30  2:48     ` Tsukasa OI
  0 siblings, 1 reply; 6+ messages in thread
From: Nelson Chu @ 2023-08-30  2:45 UTC (permalink / raw)
  To: Tsukasa OI
  Cc: Palmer Dabbelt, Andrew Waterman, Jim Wilson, Kito Cheng,
	Jeff Law, Greg Favor, binutils

[-- Attachment #1: Type: text/plain, Size: 3033 bytes --]

OKay, thanks.

Nelson

On Wed, Aug 30, 2023 at 9:38 AM Tsukasa OI <research_trasio@irq.a4lg.com>
wrote:

> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>
> Although XVentanaCondOps instructions are XLEN-agonistic, Ventana's manual
> only defines them only for RV64 (because all Ventana's processors implement
> RV64).
>
> This commit limits XVentanaCondOps instructions RV64-only to match the
> behavior of the manual and LLVM.
>
> Note that this commit alone will not make XVentanaCondOps extension with
> RV32 invalid (it just makes XVentanaCondOps on RV32 empty).
>
> opcodes/ChangeLog:
>
>         * riscv-opc.c (riscv_opcodes): Restrict "vt.maskc" and "vt.maskcn"
>         to XLEN=64.
>
> gas/ChangeLog:
>
>         * testsuite/gas/riscv/x-ventana-condops-32.d: New failure test.
>         * testsuite/gas/riscv/x-ventana-condops-32.l: Likewise.
> ---
>  gas/testsuite/gas/riscv/x-ventana-condops-32.d | 3 +++
>  gas/testsuite/gas/riscv/x-ventana-condops-32.l | 3 +++
>  opcodes/riscv-opc.c                            | 4 ++--
>  3 files changed, 8 insertions(+), 2 deletions(-)
>  create mode 100644 gas/testsuite/gas/riscv/x-ventana-condops-32.d
>  create mode 100644 gas/testsuite/gas/riscv/x-ventana-condops-32.l
>
> diff --git a/gas/testsuite/gas/riscv/x-ventana-condops-32.d
> b/gas/testsuite/gas/riscv/x-ventana-condops-32.d
> new file mode 100644
> index 000000000000..ea67515da0e3
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/x-ventana-condops-32.d
> @@ -0,0 +1,3 @@
> +#as: -march=rv32i_xventanacondops
> +#source: x-ventana-condops.s
> +#error_output: x-ventana-condops-32.l
> diff --git a/gas/testsuite/gas/riscv/x-ventana-condops-32.l
> b/gas/testsuite/gas/riscv/x-ventana-condops-32.l
> new file mode 100644
> index 000000000000..e434caf15f60
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/x-ventana-condops-32.l
> @@ -0,0 +1,3 @@
> +.*Assembler messages:
> +.*Error: unrecognized opcode `vt.maskc a0,a1,a2'
> +.*Error: unrecognized opcode `vt.maskcn a0,a3,a4'
> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> index 067e9fdb611f..f5416605dcc3 100644
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -2174,8 +2174,8 @@ const struct riscv_opcode riscv_opcodes[] =
>  {"th.sync.s",        0, INSN_CLASS_XTHEADSYNC,  "",   MATCH_TH_SYNC_S,
>     MASK_TH_SYNC_S,        match_opcode, 0},
>
>  /* Vendor-specific (Ventana Microsystems) XVentanaCondOps instructions */
> -{"vt.maskc",    0, INSN_CLASS_XVENTANACONDOPS, "d,s,t", MATCH_VT_MASKC,
> MASK_VT_MASKC, match_opcode, 0 },
> -{"vt.maskcn",   0, INSN_CLASS_XVENTANACONDOPS, "d,s,t", MATCH_VT_MASKCN,
> MASK_VT_MASKCN, match_opcode, 0 },
> +{"vt.maskc",   64, INSN_CLASS_XVENTANACONDOPS, "d,s,t", MATCH_VT_MASKC,
> MASK_VT_MASKC, match_opcode, 0 },
> +{"vt.maskcn",  64, INSN_CLASS_XVENTANACONDOPS, "d,s,t", MATCH_VT_MASKCN,
> MASK_VT_MASKCN, match_opcode, 0 },
>
>  /* Terminate the list.  */
>  {0, 0, INSN_CLASS_NONE, 0, 0, 0, 0, 0}
> --
> 2.42.0
>
>

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

* Re: [PATCH 1/1] RISC-V: Make XVentanaCondOps RV64 only
  2023-08-30  2:45   ` Nelson Chu
@ 2023-08-30  2:48     ` Tsukasa OI
  0 siblings, 0 replies; 6+ messages in thread
From: Tsukasa OI @ 2023-08-30  2:48 UTC (permalink / raw)
  To: Nelson Chu; +Cc: Binutils

On 2023/08/30 11:45, Nelson Chu wrote:
> OKay, thanks.
> 
> Nelson

Approve confirmed.

Just to make sure, I'll wait for a few days (for feedback from Ventana)
before merging.

Thanks,
Tsukasa

> 
> On Wed, Aug 30, 2023 at 9:38 AM Tsukasa OI <research_trasio@irq.a4lg.com
> <mailto:research_trasio@irq.a4lg.com>> wrote:
> 
>     From: Tsukasa OI <research_trasio@irq.a4lg.com
>     <mailto:research_trasio@irq.a4lg.com>>
> 
>     Although XVentanaCondOps instructions are XLEN-agonistic, Ventana's
>     manual
>     only defines them only for RV64 (because all Ventana's processors
>     implement
>     RV64).
> 
>     This commit limits XVentanaCondOps instructions RV64-only to match the
>     behavior of the manual and LLVM.
> 
>     Note that this commit alone will not make XVentanaCondOps extension with
>     RV32 invalid (it just makes XVentanaCondOps on RV32 empty).
> 
>     opcodes/ChangeLog:
> 
>             * riscv-opc.c (riscv_opcodes): Restrict "vt.maskc" and
>     "vt.maskcn"
>             to XLEN=64.
> 
>     gas/ChangeLog:
> 
>             * testsuite/gas/riscv/x-ventana-condops-32.d: New failure test.
>             * testsuite/gas/riscv/x-ventana-condops-32.l: Likewise.
>     ---
>      gas/testsuite/gas/riscv/x-ventana-condops-32.d | 3 +++
>      gas/testsuite/gas/riscv/x-ventana-condops-32.l | 3 +++
>      opcodes/riscv-opc.c                            | 4 ++--
>      3 files changed, 8 insertions(+), 2 deletions(-)
>      create mode 100644 gas/testsuite/gas/riscv/x-ventana-condops-32.d
>      create mode 100644 gas/testsuite/gas/riscv/x-ventana-condops-32.l
> 
>     diff --git a/gas/testsuite/gas/riscv/x-ventana-condops-32.d
>     b/gas/testsuite/gas/riscv/x-ventana-condops-32.d
>     new file mode 100644
>     index 000000000000..ea67515da0e3
>     --- /dev/null
>     +++ b/gas/testsuite/gas/riscv/x-ventana-condops-32.d
>     @@ -0,0 +1,3 @@
>     +#as: -march=rv32i_xventanacondops
>     +#source: x-ventana-condops.s
>     +#error_output: x-ventana-condops-32.l
>     diff --git a/gas/testsuite/gas/riscv/x-ventana-condops-32.l
>     b/gas/testsuite/gas/riscv/x-ventana-condops-32.l
>     new file mode 100644
>     index 000000000000..e434caf15f60
>     --- /dev/null
>     +++ b/gas/testsuite/gas/riscv/x-ventana-condops-32.l
>     @@ -0,0 +1,3 @@
>     +.*Assembler messages:
>     +.*Error: unrecognized opcode `vt.maskc a0,a1,a2'
>     +.*Error: unrecognized opcode `vt.maskcn a0,a3,a4'
>     diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
>     index 067e9fdb611f..f5416605dcc3 100644
>     --- a/opcodes/riscv-opc.c
>     +++ b/opcodes/riscv-opc.c
>     @@ -2174,8 +2174,8 @@ const struct riscv_opcode riscv_opcodes[] =
>      {"th.sync.s",        0, INSN_CLASS_XTHEADSYNC,  "", 
>      MATCH_TH_SYNC_S,        MASK_TH_SYNC_S,        match_opcode, 0},
> 
>      /* Vendor-specific (Ventana Microsystems) XVentanaCondOps
>     instructions */
>     -{"vt.maskc",    0, INSN_CLASS_XVENTANACONDOPS, "d,s,t",
>     MATCH_VT_MASKC, MASK_VT_MASKC, match_opcode, 0 },
>     -{"vt.maskcn",   0, INSN_CLASS_XVENTANACONDOPS, "d,s,t",
>     MATCH_VT_MASKCN, MASK_VT_MASKCN, match_opcode, 0 },
>     +{"vt.maskc",   64, INSN_CLASS_XVENTANACONDOPS, "d,s,t",
>     MATCH_VT_MASKC, MASK_VT_MASKC, match_opcode, 0 },
>     +{"vt.maskcn",  64, INSN_CLASS_XVENTANACONDOPS, "d,s,t",
>     MATCH_VT_MASKCN, MASK_VT_MASKCN, match_opcode, 0 },
> 
>      /* Terminate the list.  */
>      {0, 0, INSN_CLASS_NONE, 0, 0, 0, 0, 0}
>     -- 
>     2.42.0
> 

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

* Re: [PATCH 0/1] RISC-V: Make XVentanaCondOps RV64 only
  2023-08-30  1:38 [PATCH 0/1] RISC-V: Make XVentanaCondOps RV64 only Tsukasa OI
  2023-08-30  1:38 ` [PATCH 1/1] " Tsukasa OI
@ 2023-08-30  3:21 ` Greg Favor
  2023-08-30  3:59   ` Tsukasa OI
  1 sibling, 1 reply; 6+ messages in thread
From: Greg Favor @ 2023-08-30  3:21 UTC (permalink / raw)
  To: Tsukasa OI
  Cc: Palmer Dabbelt, Andrew Waterman, Jim Wilson, Nelson Chu,
	Kito Cheng, Jeff Law, binutils

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

On Tue, Aug 29, 2023 at 6:38 PM Tsukasa OI <research_trasio@irq.a4lg.com>
wrote:

> Unless Ventana is working on some RV32 processors (and soon to be released
> ), I think disabling XVentanaCondOps instructions on RV32 would be safer
> (to prevent possible misuses).  I would like to hear thoughts especially
> from Ventana employees since I am just a volunteer.
>

Representing Ventana, what you say is correct, i.e. we have no plans to do
RV32 processors.

Greg


> I also chose not to reject XVentanaCondOps + RV32 because it is not stated
> that is illegal (unlike Zcf + RV64).  This patch set makes XVentanaCondOps
> + RV32 empty (yet legal).
>

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

* Re: [PATCH 0/1] RISC-V: Make XVentanaCondOps RV64 only
  2023-08-30  3:21 ` [PATCH 0/1] " Greg Favor
@ 2023-08-30  3:59   ` Tsukasa OI
  0 siblings, 0 replies; 6+ messages in thread
From: Tsukasa OI @ 2023-08-30  3:59 UTC (permalink / raw)
  To: Greg Favor
  Cc: Palmer Dabbelt, Andrew Waterman, Jim Wilson, Nelson Chu,
	Kito Cheng, Jeff Law, binutils

On 2023/08/30 12:21, Greg Favor wrote:
> On Tue, Aug 29, 2023 at 6:38 PM Tsukasa OI <research_trasio@irq.a4lg.com
> <mailto:research_trasio@irq.a4lg.com>> wrote:
> 
>     Unless Ventana is working on some RV32 processors (and soon to be
>     released
>     ), I think disabling XVentanaCondOps instructions on RV32 would be safer
>     (to prevent possible misuses).  I would like to hear thoughts especially
>     from Ventana employees since I am just a volunteer.
> 
> 
> Representing Ventana, what you say is correct, i.e. we have no plans to
> do RV32 processors.
> 
> Greg

Thanks for the official information.  Committing.

(we could make XVentanaCondOps invalid on RV32 considering your answer
but that's for another time.)

Tsukasa

>  
> 
>     I also chose not to reject XVentanaCondOps + RV32 because it is not
>     stated
>     that is illegal (unlike Zcf + RV64).  This patch set makes
>     XVentanaCondOps
>     + RV32 empty (yet legal).
> 

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

end of thread, other threads:[~2023-08-30  3:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-30  1:38 [PATCH 0/1] RISC-V: Make XVentanaCondOps RV64 only Tsukasa OI
2023-08-30  1:38 ` [PATCH 1/1] " Tsukasa OI
2023-08-30  2:45   ` Nelson Chu
2023-08-30  2:48     ` Tsukasa OI
2023-08-30  3:21 ` [PATCH 0/1] " Greg Favor
2023-08-30  3:59   ` 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).