[RFC PATCH v1] RISC-V: Support Zicond extension

[RFC PATCH v1] RISC-V: Support Zicond extension

[Philipp Tomsich]

*** Zicond is not FROZEN at this time. Do not merge until FROZEN. ***

This implements the Zicond (conditional integer operations) extension,
as of version 1.0-draft-20230120.

The Zicond extension acts as a building block for branchless sequences
including conditional-arithmetic, conditional-logic and
conditional-select/move.
The following instructions constitute Zicond:
  - czero.eqz rd, rs1, rs2  =>  rd = (rs2 == 0) ? 0 : rs1
  - czero.nez rd, rs1, rs2  =>  rd = (rs2 != 0) ? 0 : rs1

See
  https://github.com/riscv/riscv-zicond/releases/download/v1.0-draft-20230120/riscv-zicond_1.0-draft-20230120.pdf
for the proposed specification and usage details.

bfd/ChangeLog:

	* elfxx-riscv.c (riscv_multi_subset_supports): Recognize
        INSN_CLASS_XVENTANACONDOPS.
	(riscv_multi_subset_supports_ext): Recognize
	INSN_CLASS_XVENTANACONDOPS,

gas/ChangeLog:

	* testsuite/gas/riscv/zicond.d: New test.
	* testsuite/gas/riscv/zicond.s: New test.

include/ChangeLog:

	* opcode/riscv-opc.h (MATCH_CZERO_EQZ): Define.
	(MASK_CZERO_EQZ): Define.
	(MATCH_CZERO_NEZ): Define,
	(MASK_CZERO_NEZ): Define.
	(DECLARE_INSN): Add czero.eqz and czero.nez.
	* opcode/riscv.h (enum riscv_insn_class): Add
        INSN_CLASS_ZICOND

opcodes/ChangeLog:

	* riscv-opc.c: Add czero.eqz and czero.nez.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---

 bfd/elfxx-riscv.c                |  5 +++++
 gas/testsuite/gas/riscv/zicond.d | 12 ++++++++++++
 gas/testsuite/gas/riscv/zicond.s |  3 +++
 include/opcode/riscv-opc.h       |  8 ++++++++
 include/opcode/riscv.h           |  1 +
 opcodes/riscv-opc.c              |  4 ++++
 6 files changed, 33 insertions(+)
 create mode 100644 gas/testsuite/gas/riscv/zicond.d
 create mode 100644 gas/testsuite/gas/riscv/zicond.s

diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index a48d6300a1f..ec69c7fdc4d 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -1168,6 +1168,7 @@ static struct riscv_supported_ext riscv_supported_std_z_ext[] =
   {"zicbom",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
   {"zicbop",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
   {"zicboz",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
+  {"zicond",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
   {"zicsr",		ISA_SPEC_CLASS_20191213,	2, 0,  0 },
   {"zicsr",		ISA_SPEC_CLASS_20190608,	2, 0,  0 },
   {"zifencei",		ISA_SPEC_CLASS_20191213,	2, 0,  0 },
@@ -2267,6 +2268,8 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
       return riscv_subset_supports (rps, "zicbop");
     case INSN_CLASS_ZICBOZ:
       return riscv_subset_supports (rps, "zicboz");
+    case INSN_CLASS_ZICOND:
+      return riscv_subset_supports (rps, "zicond");
     case INSN_CLASS_ZICSR:
       return riscv_subset_supports (rps, "zicsr");
     case INSN_CLASS_ZIFENCEI:
@@ -2435,6 +2438,8 @@ riscv_multi_subset_supports_ext (riscv_parse_subset_t *rps,
       return "zicbop";
     case INSN_CLASS_ZICBOZ:
       return "zicboz";
+    case INSN_CLASS_ZICOND:
+      return "zicond";
     case INSN_CLASS_ZICSR:
       return "zicsr";
     case INSN_CLASS_ZIFENCEI:
diff --git a/gas/testsuite/gas/riscv/zicond.d b/gas/testsuite/gas/riscv/zicond.d
new file mode 100644
index 00000000000..7a79ee6a716
--- /dev/null
+++ b/gas/testsuite/gas/riscv/zicond.d
@@ -0,0 +1,12 @@
+#as: -march=rv64i_zicond
+#source: zicond.s
+#objdump: -d
+
+.*:[	 ]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <target>:
+[	 ]+0:[	 ]+0ec5d533[	 ]+czero.eqz[	 ]+a0,a1,a2
+[	 ]+4:[	 ]+0ee6f533[	 ]+czero.nez[	 ]+a0,a3,a4
diff --git a/gas/testsuite/gas/riscv/zicond.s b/gas/testsuite/gas/riscv/zicond.s
new file mode 100644
index 00000000000..4e5edd5056a
--- /dev/null
+++ b/gas/testsuite/gas/riscv/zicond.s
@@ -0,0 +1,3 @@
+target:
+	czero.eqz	a0, a1, a2
+	czero.nez	a0, a3, a4
diff --git a/include/opcode/riscv-opc.h b/include/opcode/riscv-opc.h
index 32b160660d3..c99756e47e1 100644
--- a/include/opcode/riscv-opc.h
+++ b/include/opcode/riscv-opc.h
@@ -2187,6 +2187,11 @@
 #define MASK_CBO_INVAL 0xfff07fff
 #define MATCH_CBO_ZERO 0x40200f
 #define MASK_CBO_ZERO 0xfff07fff
+/* Zicond instructions. */
+#define MATCH_CZERO_EQZ 0xe005033
+#define MASK_CZERO_EQZ 0xfe00707f
+#define MATCH_CZERO_NEZ 0xe007033
+#define MASK_CZERO_NEZ 0xfe00707f
 /* Zawrs intructions.  */
 #define MATCH_WRS_NTO 0x00d00073
 #define MASK_WRS_NTO 0xffffffff
@@ -3194,6 +3199,9 @@ DECLARE_INSN(cbo_clean, MATCH_CBO_CLEAN, MASK_CBO_CLEAN);
 DECLARE_INSN(cbo_flush, MATCH_CBO_FLUSH, MASK_CBO_FLUSH);
 DECLARE_INSN(cbo_inval, MATCH_CBO_INVAL, MASK_CBO_INVAL);
 DECLARE_INSN(cbo_zero, MATCH_CBO_ZERO, MASK_CBO_ZERO);
+/* Zicond instructions. */
+DECLARE_INSN(czero_eqz, MATCH_CZERO_EQZ, MASK_CZERO_EQZ)
+DECLARE_INSN(czero_nez, MATCH_CZERO_NEZ, MASK_CZERO_NEZ)
 /* Zawrs instructions.  */
 DECLARE_INSN(wrs_nto, MATCH_WRS_NTO, MASK_WRS_NTO)
 DECLARE_INSN(wrs_sto, MATCH_WRS_STO, MASK_WRS_STO)
diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
index 57bea48704d..dfab0502d49 100644
--- a/include/opcode/riscv.h
+++ b/include/opcode/riscv.h
@@ -379,6 +379,7 @@ enum riscv_insn_class
   INSN_CLASS_Q,
   INSN_CLASS_F_AND_C,
   INSN_CLASS_D_AND_C,
+  INSN_CLASS_ZICOND,
   INSN_CLASS_ZICSR,
   INSN_CLASS_ZIFENCEI,
   INSN_CLASS_ZIHINTPAUSE,
diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
index 639683e76b1..33cd4ab57d1 100644
--- a/opcodes/riscv-opc.c
+++ b/opcodes/riscv-opc.c
@@ -935,6 +935,10 @@ const struct riscv_opcode riscv_opcodes[] =
 {"cbo.inval",  0, INSN_CLASS_ZICBOM, "0(s)", MATCH_CBO_INVAL, MASK_CBO_INVAL, match_opcode, 0 },
 {"cbo.zero",   0, INSN_CLASS_ZICBOZ, "0(s)", MATCH_CBO_ZERO, MASK_CBO_ZERO, match_opcode, 0 },
 
+/* Zicond instructions.  */
+{"czero.eqz",  0, INSN_CLASS_ZICOND, "d,s,t", MATCH_CZERO_EQZ, MASK_CZERO_EQZ, match_opcode, 0 },
+{"czero.nez",  0, INSN_CLASS_ZICOND, "d,s,t", MATCH_CZERO_NEZ, MASK_CZERO_NEZ, match_opcode, 0 },
+
 /* Zawrs instructions.  */
 {"wrs.nto",    0, INSN_CLASS_ZAWRS, "", MATCH_WRS_NTO, MASK_WRS_NTO, match_opcode, 0 },
 {"wrs.sto",    0, INSN_CLASS_ZAWRS, "", MATCH_WRS_STO, MASK_WRS_STO, match_opcode, 0 },
-- 
2.34.1

Re: [RFC PATCH v1] RISC-V: Support Zicond extension

[Jeff Law]

On 1/20/23 17:29, Philipp Tomsich wrote:
> *** Zicond is not FROZEN at this time. Do not merge until FROZEN. ***
> 
> This implements the Zicond (conditional integer operations) extension,
> as of version 1.0-draft-20230120.
> 
> The Zicond extension acts as a building block for branchless sequences
> including conditional-arithmetic, conditional-logic and
> conditional-select/move.
> The following instructions constitute Zicond:
>    - czero.eqz rd, rs1, rs2  =>  rd = (rs2 == 0) ? 0 : rs1
>    - czero.nez rd, rs1, rs2  =>  rd = (rs2 != 0) ? 0 : rs1
> 
> See
>    https://github.com/riscv/riscv-zicond/releases/download/v1.0-draft-20230120/riscv-zicond_1.0-draft-20230120.pdf
> for the proposed specification and usage details.
> 
> bfd/ChangeLog:
> 
> 	* elfxx-riscv.c (riscv_multi_subset_supports): Recognize
>          INSN_CLASS_XVENTANACONDOPS.
> 	(riscv_multi_subset_supports_ext): Recognize
> 	INSN_CLASS_XVENTANACONDOPS,
> 
> gas/ChangeLog:
> 
> 	* testsuite/gas/riscv/zicond.d: New test.
> 	* testsuite/gas/riscv/zicond.s: New test.
> 
> include/ChangeLog:
> 
> 	* opcode/riscv-opc.h (MATCH_CZERO_EQZ): Define.
> 	(MASK_CZERO_EQZ): Define.
> 	(MATCH_CZERO_NEZ): Define,
> 	(MASK_CZERO_NEZ): Define.
> 	(DECLARE_INSN): Add czero.eqz and czero.nez.
> 	* opcode/riscv.h (enum riscv_insn_class): Add
>          INSN_CLASS_ZICOND
> 
> opcodes/ChangeLog:
> 
> 	* riscv-opc.c: Add czero.eqz and czero.nez.
Given this extension is derived from the Ventana condops extension, I 
may be somewhat biased.  The mnemonics and encoding is obviously 
different, but the behavior is the same (perhaps differing in timing 
characteristics, but I think that's outside of what we care about here).

I assume nobody cares about gdbsim, so nothing to do there.  With that 
assumption this is fine to go forward once the spec freezes.

jeff

Re: [RFC PATCH v1] RISC-V: Support Zicond extension

[Andrew Waterman]

On Wed, Jan 25, 2023 at 4:27 PM Jeff Law via Binutils
<binutils@sourceware.org> wrote:
>
>
>
> On 1/20/23 17:29, Philipp Tomsich wrote:
> > *** Zicond is not FROZEN at this time. Do not merge until FROZEN. ***
> >
> > This implements the Zicond (conditional integer operations) extension,
> > as of version 1.0-draft-20230120.
> >
> > The Zicond extension acts as a building block for branchless sequences
> > including conditional-arithmetic, conditional-logic and
> > conditional-select/move.
> > The following instructions constitute Zicond:
> >    - czero.eqz rd, rs1, rs2  =>  rd = (rs2 == 0) ? 0 : rs1
> >    - czero.nez rd, rs1, rs2  =>  rd = (rs2 != 0) ? 0 : rs1
> >
> > See
> >    https://github.com/riscv/riscv-zicond/releases/download/v1.0-draft-20230120/riscv-zicond_1.0-draft-20230120.pdf
> > for the proposed specification and usage details.
> >
> > bfd/ChangeLog:
> >
> >       * elfxx-riscv.c (riscv_multi_subset_supports): Recognize
> >          INSN_CLASS_XVENTANACONDOPS.
> >       (riscv_multi_subset_supports_ext): Recognize
> >       INSN_CLASS_XVENTANACONDOPS,
> >
> > gas/ChangeLog:
> >
> >       * testsuite/gas/riscv/zicond.d: New test.
> >       * testsuite/gas/riscv/zicond.s: New test.
> >
> > include/ChangeLog:
> >
> >       * opcode/riscv-opc.h (MATCH_CZERO_EQZ): Define.
> >       (MASK_CZERO_EQZ): Define.
> >       (MATCH_CZERO_NEZ): Define,
> >       (MASK_CZERO_NEZ): Define.
> >       (DECLARE_INSN): Add czero.eqz and czero.nez.
> >       * opcode/riscv.h (enum riscv_insn_class): Add
> >          INSN_CLASS_ZICOND
> >
> > opcodes/ChangeLog:
> >
> >       * riscv-opc.c: Add czero.eqz and czero.nez.
> Given this extension is derived from the Ventana condops extension, I
> may be somewhat biased.

This is very much a standardized version of Ventana's custom condops
extension, modulo some details.

> The mnemonics and encoding is obviously
> different, but the behavior is the same (perhaps differing in timing
> characteristics, but I think that's outside of what we care about here).
>
> I assume nobody cares about gdbsim, so nothing to do there.  With that
> assumption this is fine to go forward once the spec freezes.

As for a status update, the RVI architecture-review committee recently
voted to approve the extension, so the official freeze should be
forthcoming.

>
> jeff
>

Re: [RFC PATCH v1] RISC-V: Support Zicond extension

[Nelson Chu]

Hey Guys,

I had heard that the final ratification of the zicond extension is
coming soon, so it's time to see what we should do for those old
vendor mnemonics and encodings, according to the current gnu policy.
Not sure if maintaining compatibility of vendor stuff is worth it, but
considering that obsolete vendor stuff should be replaced by the
ratified zicond as soon as possible (or we should encourage everyone
to use the RVI extension rather than the vendor one), maybe just
removing them when the zicond committed is more intuitive.

Thanks
Nelson

On Thu, Jan 26, 2023 at 9:03 AM Andrew Waterman <andrew@sifive.com> wrote:
>
> On Wed, Jan 25, 2023 at 4:27 PM Jeff Law via Binutils
> <binutils@sourceware.org> wrote:
> >
> >
> >
> > On 1/20/23 17:29, Philipp Tomsich wrote:
> > > *** Zicond is not FROZEN at this time. Do not merge until FROZEN. ***
> > >
> > > This implements the Zicond (conditional integer operations) extension,
> > > as of version 1.0-draft-20230120.
> > >
> > > The Zicond extension acts as a building block for branchless sequences
> > > including conditional-arithmetic, conditional-logic and
> > > conditional-select/move.
> > > The following instructions constitute Zicond:
> > >    - czero.eqz rd, rs1, rs2  =>  rd = (rs2 == 0) ? 0 : rs1
> > >    - czero.nez rd, rs1, rs2  =>  rd = (rs2 != 0) ? 0 : rs1
> > >
> > > See
> > >    https://github.com/riscv/riscv-zicond/releases/download/v1.0-draft-20230120/riscv-zicond_1.0-draft-20230120.pdf
> > > for the proposed specification and usage details.
> > >
> > > bfd/ChangeLog:
> > >
> > >       * elfxx-riscv.c (riscv_multi_subset_supports): Recognize
> > >          INSN_CLASS_XVENTANACONDOPS.
> > >       (riscv_multi_subset_supports_ext): Recognize
> > >       INSN_CLASS_XVENTANACONDOPS,
> > >
> > > gas/ChangeLog:
> > >
> > >       * testsuite/gas/riscv/zicond.d: New test.
> > >       * testsuite/gas/riscv/zicond.s: New test.
> > >
> > > include/ChangeLog:
> > >
> > >       * opcode/riscv-opc.h (MATCH_CZERO_EQZ): Define.
> > >       (MASK_CZERO_EQZ): Define.
> > >       (MATCH_CZERO_NEZ): Define,
> > >       (MASK_CZERO_NEZ): Define.
> > >       (DECLARE_INSN): Add czero.eqz and czero.nez.
> > >       * opcode/riscv.h (enum riscv_insn_class): Add
> > >          INSN_CLASS_ZICOND
> > >
> > > opcodes/ChangeLog:
> > >
> > >       * riscv-opc.c: Add czero.eqz and czero.nez.
> > Given this extension is derived from the Ventana condops extension, I
> > may be somewhat biased.
>
> This is very much a standardized version of Ventana's custom condops
> extension, modulo some details.
>
> > The mnemonics and encoding is obviously
> > different, but the behavior is the same (perhaps differing in timing
> > characteristics, but I think that's outside of what we care about here).
> >
> > I assume nobody cares about gdbsim, so nothing to do there.  With that
> > assumption this is fine to go forward once the spec freezes.
>
> As for a status update, the RVI architecture-review committee recently
> voted to approve the extension, so the official freeze should be
> forthcoming.
>
> >
> > jeff
> >

Re: [RFC PATCH v1] RISC-V: Support Zicond extension

[Palmer Dabbelt]

On Tue, 28 Feb 2023 17:58:29 PST (-0800), nelson@rivosinc.com wrote:
> Hey Guys,
>
> I had heard that the final ratification of the zicond extension is
> coming soon, so it's time to see what we should do for those old
> vendor mnemonics and encodings, according to the current gnu policy.

Do you mean the Ventana cond ops?  IIUC those are in their chip, so we'd 
need to support them regardless of what happens at RVI.

> Not sure if maintaining compatibility of vendor stuff is worth it, but
> considering that obsolete vendor stuff should be replaced by the
> ratified zicond as soon as possible (or we should encourage everyone
> to use the RVI extension rather than the vendor one), maybe just
> removing them when the zicond committed is more intuitive.

Unless I'm missing something, neither the pre-freeze flavors of Zicond 
or the Ventana cond ops have landed upstream yet.  So we shouldn't need 
to remove anything (which would generally be a problem if it had been 
released).

>
> Thanks
> Nelson
>
> On Thu, Jan 26, 2023 at 9:03 AM Andrew Waterman <andrew@sifive.com> wrote:
>>
>> On Wed, Jan 25, 2023 at 4:27 PM Jeff Law via Binutils
>> <binutils@sourceware.org> wrote:
>> >
>> >
>> >
>> > On 1/20/23 17:29, Philipp Tomsich wrote:
>> > > *** Zicond is not FROZEN at this time. Do not merge until FROZEN. ***
>> > >
>> > > This implements the Zicond (conditional integer operations) extension,
>> > > as of version 1.0-draft-20230120.
>> > >
>> > > The Zicond extension acts as a building block for branchless sequences
>> > > including conditional-arithmetic, conditional-logic and
>> > > conditional-select/move.
>> > > The following instructions constitute Zicond:
>> > >    - czero.eqz rd, rs1, rs2  =>  rd = (rs2 == 0) ? 0 : rs1
>> > >    - czero.nez rd, rs1, rs2  =>  rd = (rs2 != 0) ? 0 : rs1
>> > >
>> > > See
>> > >    https://github.com/riscv/riscv-zicond/releases/download/v1.0-draft-20230120/riscv-zicond_1.0-draft-20230120.pdf
>> > > for the proposed specification and usage details.
>> > >
>> > > bfd/ChangeLog:
>> > >
>> > >       * elfxx-riscv.c (riscv_multi_subset_supports): Recognize
>> > >          INSN_CLASS_XVENTANACONDOPS.
>> > >       (riscv_multi_subset_supports_ext): Recognize
>> > >       INSN_CLASS_XVENTANACONDOPS,
>> > >
>> > > gas/ChangeLog:
>> > >
>> > >       * testsuite/gas/riscv/zicond.d: New test.
>> > >       * testsuite/gas/riscv/zicond.s: New test.
>> > >
>> > > include/ChangeLog:
>> > >
>> > >       * opcode/riscv-opc.h (MATCH_CZERO_EQZ): Define.
>> > >       (MASK_CZERO_EQZ): Define.
>> > >       (MATCH_CZERO_NEZ): Define,
>> > >       (MASK_CZERO_NEZ): Define.
>> > >       (DECLARE_INSN): Add czero.eqz and czero.nez.
>> > >       * opcode/riscv.h (enum riscv_insn_class): Add
>> > >          INSN_CLASS_ZICOND
>> > >
>> > > opcodes/ChangeLog:
>> > >
>> > >       * riscv-opc.c: Add czero.eqz and czero.nez.
>> > Given this extension is derived from the Ventana condops extension, I
>> > may be somewhat biased.
>>
>> This is very much a standardized version of Ventana's custom condops
>> extension, modulo some details.
>>
>> > The mnemonics and encoding is obviously
>> > different, but the behavior is the same (perhaps differing in timing
>> > characteristics, but I think that's outside of what we care about here).
>> >
>> > I assume nobody cares about gdbsim, so nothing to do there.  With that
>> > assumption this is fine to go forward once the spec freezes.
>>
>> As for a status update, the RVI architecture-review committee recently
>> voted to approve the extension, so the official freeze should be
>> forthcoming.
>>
>> >
>> > jeff
>> >

Re: [RFC PATCH v1] RISC-V: Support Zicond extension

[Philipp Tomsich]

Given that the 2.41 milestone is approaching and Zicond has passed the
FREEZE milestone (and is even out of public review already), I would
like to move this forward.
Is this OK for trunk?  Should I resubmit with an updated commit message?

Thanks,
Philipp.


On Thu, 26 Jan 2023 at 01:26, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 1/20/23 17:29, Philipp Tomsich wrote:
> > *** Zicond is not FROZEN at this time. Do not merge until FROZEN. ***
> >
> > This implements the Zicond (conditional integer operations) extension,
> > as of version 1.0-draft-20230120.
> >
> > The Zicond extension acts as a building block for branchless sequences
> > including conditional-arithmetic, conditional-logic and
> > conditional-select/move.
> > The following instructions constitute Zicond:
> >    - czero.eqz rd, rs1, rs2  =>  rd = (rs2 == 0) ? 0 : rs1
> >    - czero.nez rd, rs1, rs2  =>  rd = (rs2 != 0) ? 0 : rs1
> >
> > See
> >    https://github.com/riscv/riscv-zicond/releases/download/v1.0-draft-20230120/riscv-zicond_1.0-draft-20230120.pdf
> > for the proposed specification and usage details.
> >
> > bfd/ChangeLog:
> >
> >       * elfxx-riscv.c (riscv_multi_subset_supports): Recognize
> >          INSN_CLASS_XVENTANACONDOPS.
> >       (riscv_multi_subset_supports_ext): Recognize
> >       INSN_CLASS_XVENTANACONDOPS,
> >
> > gas/ChangeLog:
> >
> >       * testsuite/gas/riscv/zicond.d: New test.
> >       * testsuite/gas/riscv/zicond.s: New test.
> >
> > include/ChangeLog:
> >
> >       * opcode/riscv-opc.h (MATCH_CZERO_EQZ): Define.
> >       (MASK_CZERO_EQZ): Define.
> >       (MATCH_CZERO_NEZ): Define,
> >       (MASK_CZERO_NEZ): Define.
> >       (DECLARE_INSN): Add czero.eqz and czero.nez.
> >       * opcode/riscv.h (enum riscv_insn_class): Add
> >          INSN_CLASS_ZICOND
> >
> > opcodes/ChangeLog:
> >
> >       * riscv-opc.c: Add czero.eqz and czero.nez.
> Given this extension is derived from the Ventana condops extension, I
> may be somewhat biased.  The mnemonics and encoding is obviously
> different, but the behavior is the same (perhaps differing in timing
> characteristics, but I think that's outside of what we care about here).
>
> I assume nobody cares about gdbsim, so nothing to do there.  With that
> assumption this is fine to go forward once the spec freezes.
>
> jeff
>

Re: [RFC PATCH v1] RISC-V: Support Zicond extension

[Jeff Law]

On 6/19/23 08:21, Philipp Tomsich wrote:
> Given that the 2.41 milestone is approaching and Zicond has passed the
> FREEZE milestone (and is even out of public review already), I would
> like to move this forward.
> Is this OK for trunk?  Should I resubmit with an updated commit message?
Given they were acked  with "this is fine to go forward with the spec 
freezes", I think they can go forward now.  I think you just need to 
fixup the ChangeLog a bit since it references INSN_CLASS_XVENTANACONDOPS ;-)


Jeff

Re: [RFC PATCH v1] RISC-V: Support Zicond extension

[Palmer Dabbelt]

On Tue, 20 Jun 2023 21:25:12 PDT (-0700), binutils@sourceware.org wrote:
>
>
> On 6/19/23 08:21, Philipp Tomsich wrote:
>> Given that the 2.41 milestone is approaching and Zicond has passed the
>> FREEZE milestone (and is even out of public review already), I would
>> like to move this forward.
>> Is this OK for trunk?  Should I resubmit with an updated commit message?
> Given they were acked  with "this is fine to go forward with the spec
> freezes", I think they can go forward now.  I think you just need to
> fixup the ChangeLog a bit since it references INSN_CLASS_XVENTANACONDOPS ;-)

Can whomever (IIRC Philipp?) sent the first version just send a v2 with 
the various bits fixed up (ie, no RFC, no "don't merge this" header, and 
the review comments addressed)?  That way there's a chance folks will 
actually look at it.

Re: [RFC PATCH v1] RISC-V: Support Zicond extension

[Jeff Law]

On 6/20/23 22:38, Palmer Dabbelt wrote:
> On Tue, 20 Jun 2023 21:25:12 PDT (-0700), binutils@sourceware.org wrote:
>>
>>
>> On 6/19/23 08:21, Philipp Tomsich wrote:
>>> Given that the 2.41 milestone is approaching and Zicond has passed the
>>> FREEZE milestone (and is even out of public review already), I would
>>> like to move this forward.
>>> Is this OK for trunk?  Should I resubmit with an updated commit message?
>> Given they were acked  with "this is fine to go forward with the spec
>> freezes", I think they can go forward now.  I think you just need to
>> fixup the ChangeLog a bit since it references 
>> INSN_CLASS_XVENTANACONDOPS ;-)
> 
> Can whomever (IIRC Philipp?) sent the first version just send a v2 with 
> the various bits fixed up (ie, no RFC, no "don't merge this" header, and 
> the review comments addressed)?  That way there's a chance folks will 
> actually look at it.
Seems a bit silly to keep deferring this.  I went ahead and fixed up the 
ChangeLog entry to reference INSN_CLASS_ZICOND rather than 
INSN_CLASS_XVENTANACONDOPS as well as the spec URL and version number 
and pushed the result.

Jeff

Re: [RFC PATCH v1] RISC-V: Support Zicond extension

[Philipp Tomsich]

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

Thanks for taking care of this.
I had v2 in the queue for this week—even better if it took care of itself.

The updated GCC patches should go out right after the final (incorporating
comments from the public review) specification goes out.

Philipp.

On Tue 27. Jun 2023 at 06:27, Jeff Law <jeffreyalaw@gmail.com> wrote:

>
>
> On 6/20/23 22:38, Palmer Dabbelt wrote:
> > On Tue, 20 Jun 2023 21:25:12 PDT (-0700), binutils@sourceware.org wrote:
> >>
> >>
> >> On 6/19/23 08:21, Philipp Tomsich wrote:
> >>> Given that the 2.41 milestone is approaching and Zicond has passed the
> >>> FREEZE milestone (and is even out of public review already), I would
> >>> like to move this forward.
> >>> Is this OK for trunk?  Should I resubmit with an updated commit
> message?
> >> Given they were acked  with "this is fine to go forward with the spec
> >> freezes", I think they can go forward now.  I think you just need to
> >> fixup the ChangeLog a bit since it references
> >> INSN_CLASS_XVENTANACONDOPS ;-)
> >
> > Can whomever (IIRC Philipp?) sent the first version just send a v2 with
> > the various bits fixed up (ie, no RFC, no "don't merge this" header, and
> > the review comments addressed)?  That way there's a chance folks will
> > actually look at it.
> Seems a bit silly to keep deferring this.  I went ahead and fixed up the
> ChangeLog entry to reference INSN_CLASS_ZICOND rather than
> INSN_CLASS_XVENTANACONDOPS as well as the spec URL and version number
> and pushed the result.
>
> Jeff
>

Re: [RFC PATCH v1] RISC-V: Support Zicond extension

[Palmer Dabbelt]

On Tue, 27 Jun 2023 06:27:25 PDT (-0700), jeffreyalaw@gmail.com wrote:
>
>
> On 6/20/23 22:38, Palmer Dabbelt wrote:
>> On Tue, 20 Jun 2023 21:25:12 PDT (-0700), binutils@sourceware.org wrote:
>>>
>>>
>>> On 6/19/23 08:21, Philipp Tomsich wrote:
>>>> Given that the 2.41 milestone is approaching and Zicond has passed the
>>>> FREEZE milestone (and is even out of public review already), I would
>>>> like to move this forward.
>>>> Is this OK for trunk?  Should I resubmit with an updated commit message?
>>> Given they were acked  with "this is fine to go forward with the spec
>>> freezes", I think they can go forward now.  I think you just need to
>>> fixup the ChangeLog a bit since it references
>>> INSN_CLASS_XVENTANACONDOPS ;-)
>>
>> Can whomever (IIRC Philipp?) sent the first version just send a v2 with
>> the various bits fixed up (ie, no RFC, no "don't merge this" header, and
>> the review comments addressed)?  That way there's a chance folks will
>> actually look at it.
> Seems a bit silly to keep deferring this.  I went ahead and fixed up the
> ChangeLog entry to reference INSN_CLASS_ZICOND rather than
> INSN_CLASS_XVENTANACONDOPS as well as the spec URL and version number
> and pushed the result.

Works for me -- I just defer to the original poster, but when it's 
taking months and we're up for a release there's no reason to risk 
missing.

There was also a missing NEWS entry here, I just sent a patch for that 
and another one we forgot about.

>
> Jeff

Re: [RFC PATCH v1] RISC-V: Support Zicond extension

[Jeff Law]

On 6/27/23 08:32, Philipp Tomsich wrote:
> Thanks for taking care of this.
> I had v2 in the queue for this week—even better if it took care of itself.
:-)  Didn't seem to be any value in waiting given its state and the 
release branch being cut next week.


> 
> The updated GCC patches should go out right after the final 
> (incorporating comments from the public review) specification goes out.
Sounds good.

Per a discussion earlier today, Zfa is frozen, so I think we're at a 
point where Zfa can move forward for binutils as well (thus paving the 
way for GCC to support Zfa).  I was just looking at the V5 patch from 
Christoph and I don't see anything concerning in there.  Raphael 
double-checked the MATCH/MASKs a month or so ago.

Any objections to me pushing that forward in binutils?

jeff

Re: [RFC PATCH v1] RISC-V: Support Zicond extension

[Palmer Dabbelt]

On Tue, 27 Jun 2023 16:12:05 PDT (-0700), jeffreyalaw@gmail.com wrote:
>
>
> On 6/27/23 08:32, Philipp Tomsich wrote:
>> Thanks for taking care of this.
>> I had v2 in the queue for this week—even better if it took care of itself.
> :-)  Didn't seem to be any value in waiting given its state and the
> release branch being cut next week.
>
>
>>
>> The updated GCC patches should go out right after the final
>> (incorporating comments from the public review) specification goes out.
> Sounds good.
>
> Per a discussion earlier today, Zfa is frozen, so I think we're at a
> point where Zfa can move forward for binutils as well (thus paving the
> way for GCC to support Zfa).  I was just looking at the V5 patch from
> Christoph and I don't see anything concerning in there.  Raphael
> double-checked the MATCH/MASKs a month or so ago.
>
> Any objections to me pushing that forward in binutils?

It might be better to post on that patch?  IIRC the only blocker left 
for Zfa was <https://github.com/riscv-non-isa/riscv-asm-manual/pull/85>, 
which is still open.

>
> jeff