public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] RISC-V: QUICK fix on Li's Zhinx implementation
@ 2022-06-05  4:43 Tsukasa OI
  2022-06-05  4:43 ` [PATCH 1/2] RISC-V: Add 'H' to canonical extension ordering Tsukasa OI
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Tsukasa OI @ 2022-06-05  4:43 UTC (permalink / raw)
  To: Tsukasa OI, Palmer Dabbelt, Kito Cheng, Nelson Chu, Weiwei Li; +Cc: binutils

Hello,

I understand that my Zfinx fixes + Zfhmin/Zhinxmin patchset requires
some time to review.

Meanwhile, there's a quick fix to Li's Zhinx implementation.  My
patchset already contains equivalent fix but I extracted the most
important one (that should definately be fixed before 2.39).

My combined float patchset:
<https://sourceware.org/pipermail/binutils/2022-June/121138.html>

Li Weiwei's Zhinx implementation:
<https://sourceware.org/pipermail/binutils/2022-May/121038.html>



PATCH 1:

Again, this is a duplicate of:
<https://sourceware.org/pipermail/binutils/2022-June/121115.html>
<https://sourceware.org/pipermail/binutils/2022-June/121139.html> (in my combined fixes)

PATCH 2 required a test to give -march including **both** Zqinx and
Zhinx.  As a result, this patch is mandatory for the next patch is to be
applied.  Otherwise, it would need likely non-canonical ordering on
-march.



PATCH 2:

Following instructions have INSN_CLASS_D_AND_ZFH_INX:

-   fcvt.h.d
-   fcvt.d.h

Following instructions have INSN_CLASS_Q_AND_ZFH_INX:

-   fcvt.h.q
-   fcvt.q.h

I noticed that feature gates on those instruction classes are most
likely incorrect.

Quoting ISA Manual 24.5 "Zhinxmin":
> If the Zdinx extension is present, the FCVT.D.H and FCVT.H.D
> instructions are also included.

Although no such limitation is explicitly specified on "Zhinx" itself,
it's very unlikely that FCVT.D.H and FCVT.H.D can be supported without
Zdinx extension.

Quoting Manual 16.3 "Half-Precision Conversion and Move Instructions" from Zfh:
> If the D extension is present, FCVT.D.H or FCVT.H.D converts a half-
> precision floating-point number to a double-precision floating-point
> number, or vice-versa, respectively. If the Q extension is present,
> FCVT.Q.H or FCVT.H.Q converts a half-precision floating-point number
> to a quad-precision floating-point number, or vice-versa,
> respectively.

And 24.4 "Zhinx":
> The Zhinx extension adds all of the instructions that the Zfh
> extension adds, except for the transfer instructions FLH, FSH,
> FMV.H.X, and FMV.X.H. The Zhinx variants of these Zfh-extension
> instructions have the same semantics, except that (cont...)


On INSN_CLASS_D_AND_ZFH_INX:

Li's implementation        : (D && Zfh) || Zhinx
This patch                 : (D && Zfh) || (Zdinx && Zhinx)
My combined float patchset : (D && Zfhmin) || (Zdinx && Zhinxmin)

This commit fixes feature gates and their diagnostics.



Thanks,
Tsukasa




Tsukasa OI (2):
  RISC-V: Add 'H' to canonical extension ordering
  RISC-V: Fix requirement handling on Zhinx+{D,Q}

 bfd/elfxx-riscv.c                        | 32 +++++++++++++++++++-----
 gas/testsuite/gas/riscv/fp-zhinx-insns.d |  2 +-
 2 files changed, 27 insertions(+), 7 deletions(-)


base-commit: c8eab1d7c92ad72089c98e5753ebc96419e3674a
-- 
2.34.1


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

* [PATCH 1/2] RISC-V: Add 'H' to canonical extension ordering
  2022-06-05  4:43 [PATCH 0/2] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
@ 2022-06-05  4:43 ` Tsukasa OI
  2022-06-22 10:39   ` Nelson Chu
  2022-06-05  4:43 ` [PATCH 2/2] RISC-V: Fix requirement handling on Zhinx+{D,Q} Tsukasa OI
  2022-06-24  2:59 ` [PATCH v2 0/1] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
  2 siblings, 1 reply; 12+ messages in thread
From: Tsukasa OI @ 2022-06-05  4:43 UTC (permalink / raw)
  To: Tsukasa OI, Palmer Dabbelt, Kito Cheng, Nelson Chu, Weiwei Li; +Cc: binutils

This commit adds 'H' to canonical extension ordering based on current
consensus (not officially ratified as a new ISA specification manual
but discussion for software compatibility is made).

bfd/ChangeLog

	* elfxx-riscv.c (riscv_ext_canonical_order): Add 'H' for
	canonical extension ordering based on current consensus.
---
 bfd/elfxx-riscv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index 92ad03feea0..5c2c616a760 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -1345,7 +1345,7 @@ riscv_recognized_prefixed_ext (const char *ext)
 }
 
 /* Canonical order for single letter extensions.  */
-static const char riscv_ext_canonical_order[] = "eigmafdqlcbkjtpvn";
+static const char riscv_ext_canonical_order[] = "eigmafdqlcbkjtpvnh";
 
 /* Array is used to compare the orders of standard extensions quickly.  */
 static int riscv_ext_order[26] = {0};
-- 
2.34.1


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

* [PATCH 2/2] RISC-V: Fix requirement handling on Zhinx+{D,Q}
  2022-06-05  4:43 [PATCH 0/2] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
  2022-06-05  4:43 ` [PATCH 1/2] RISC-V: Add 'H' to canonical extension ordering Tsukasa OI
@ 2022-06-05  4:43 ` Tsukasa OI
  2022-06-24  2:59 ` [PATCH v2 0/1] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
  2 siblings, 0 replies; 12+ messages in thread
From: Tsukasa OI @ 2022-06-05  4:43 UTC (permalink / raw)
  To: Tsukasa OI, Palmer Dabbelt, Kito Cheng, Nelson Chu, Weiwei Li; +Cc: binutils

This commit fixes how instructions are masked on Zhinx+Z{d,q}inx.
fcvt.h.d and fcvt.d.h require ((D&&Zfh)||(Zdinx&&Zhinx)) and
fcvt.h.q and fcvt.q.h require ((Q&&Zfh)||(Zqinx&&Zhinx)).

bfd/ChangeLog:

	* elfxx-riscv.c (riscv_multi_subset_supports): Fix feature gate
	on INSN_CLASS_{D,Q}_AND_ZFH_INX.
	(riscv_multi_subset_supports_ext): Fix feature gate diagnostics
	on INSN_CLASS_{D,Q}_AND_ZFH_INX.

gas/ChangeLog:

	* testsuite/gas/riscv/fp-zhinx-insns.d: Add Zqinx to -march
	for proper testing.
---
 bfd/elfxx-riscv.c                        | 30 ++++++++++++++++++++----
 gas/testsuite/gas/riscv/fp-zhinx-insns.d |  2 +-
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index 5c2c616a760..39b68c92317 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -2370,15 +2370,17 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
       return riscv_subset_supports (rps, "zfh");
     case INSN_CLASS_ZFH_OR_ZHINX:
       return riscv_subset_supports (rps, "zfh")
-        || riscv_subset_supports (rps, "zhinx");
+	     || riscv_subset_supports (rps, "zhinx");
     case INSN_CLASS_D_AND_ZFH_INX:
       return (riscv_subset_supports (rps, "d")
 	      && riscv_subset_supports (rps, "zfh"))
-           || riscv_subset_supports (rps, "zhinx");
+	     || (riscv_subset_supports (rps, "zdinx")
+		 && riscv_subset_supports (rps, "zhinx"));
     case INSN_CLASS_Q_AND_ZFH_INX:
       return (riscv_subset_supports (rps, "q")
 	      && riscv_subset_supports (rps, "zfh"))
-           || riscv_subset_supports (rps, "zhinx");
+	     || (riscv_subset_supports (rps, "zqinx")
+		 && riscv_subset_supports (rps, "zhinx"));
     case INSN_CLASS_ZBA:
       return riscv_subset_supports (rps, "zba");
     case INSN_CLASS_ZBB:
@@ -2522,9 +2524,27 @@ riscv_multi_subset_supports_ext (riscv_parse_subset_t *rps,
     case INSN_CLASS_ZFH_OR_ZHINX:
       return "zfh' or 'zhinx";
     case INSN_CLASS_D_AND_ZFH_INX:
-      return "('d' and 'zfh') or 'zhinx";
+      if (riscv_subset_supports (rps, "zfh"))
+	return "d";
+      else if (riscv_subset_supports (rps, "d"))
+	return "zfh";
+      else if (riscv_subset_supports (rps, "zhinx"))
+	return "zdinx";
+      else if (riscv_subset_supports (rps, "zdinx"))
+	return "zhinx";
+      else
+	return "zfh' and `d', or `zhinx' and `zdinx";
     case INSN_CLASS_Q_AND_ZFH_INX:
-      return "('q' and 'zfh') or 'zhinx";
+      if (riscv_subset_supports (rps, "zfh"))
+	return "q";
+      else if (riscv_subset_supports (rps, "q"))
+	return "zfh";
+      else if (riscv_subset_supports (rps, "zhinx"))
+	return "zqinx";
+      else if (riscv_subset_supports (rps, "zqinx"))
+	return "zhinx";
+      else
+	return "zfh' and `q', or `zhinx' and `zqinx";
     default:
       rps->error_handler
         (_("internal: unreachable INSN_CLASS_*"));
diff --git a/gas/testsuite/gas/riscv/fp-zhinx-insns.d b/gas/testsuite/gas/riscv/fp-zhinx-insns.d
index 6e1c40e65f5..2592d8c74e2 100644
--- a/gas/testsuite/gas/riscv/fp-zhinx-insns.d
+++ b/gas/testsuite/gas/riscv/fp-zhinx-insns.d
@@ -1,4 +1,4 @@
-#as: -march=rv64ima_zhinx
+#as: -march=rv64ima_zqinx_zhinx
 #source: fp-zhinx-insns.s
 #objdump: -dr
 
-- 
2.34.1


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

* Re: [PATCH 1/2] RISC-V: Add 'H' to canonical extension ordering
  2022-06-05  4:43 ` [PATCH 1/2] RISC-V: Add 'H' to canonical extension ordering Tsukasa OI
@ 2022-06-22 10:39   ` Nelson Chu
  2022-06-22 11:08     ` Tsukasa OI
  0 siblings, 1 reply; 12+ messages in thread
From: Nelson Chu @ 2022-06-22 10:39 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: Palmer Dabbelt, Kito Cheng, Weiwei Li, Binutils

Sorry for being late, I have been busy recently.  I didn't notice that
the isa spec had been updated, anyway, the h is a single extension
after v, rather than the multi-letter extension for now.  So LGTM,
committed.

Besides, please give me some time to review other patches, I've been
busy with other things for a while...

Thanks
Nelson

On Sun, Jun 5, 2022 at 12:43 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> This commit adds 'H' to canonical extension ordering based on current
> consensus (not officially ratified as a new ISA specification manual
> but discussion for software compatibility is made).
>
> bfd/ChangeLog
>
>         * elfxx-riscv.c (riscv_ext_canonical_order): Add 'H' for
>         canonical extension ordering based on current consensus.
> ---
>  bfd/elfxx-riscv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index 92ad03feea0..5c2c616a760 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -1345,7 +1345,7 @@ riscv_recognized_prefixed_ext (const char *ext)
>  }
>
>  /* Canonical order for single letter extensions.  */
> -static const char riscv_ext_canonical_order[] = "eigmafdqlcbkjtpvn";
> +static const char riscv_ext_canonical_order[] = "eigmafdqlcbkjtpvnh";
>
>  /* Array is used to compare the orders of standard extensions quickly.  */
>  static int riscv_ext_order[26] = {0};
> --
> 2.34.1
>

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

* Re: [PATCH 1/2] RISC-V: Add 'H' to canonical extension ordering
  2022-06-22 10:39   ` Nelson Chu
@ 2022-06-22 11:08     ` Tsukasa OI
  0 siblings, 0 replies; 12+ messages in thread
From: Tsukasa OI @ 2022-06-22 11:08 UTC (permalink / raw)
  To: Nelson Chu; +Cc: Binutils

On 2022/06/22 19:39, Nelson Chu wrote:
> Sorry for being late, I have been busy recently.  I didn't notice that
> the isa spec had been updated, anyway, the h is a single extension
> after v, rather than the multi-letter extension for now.  So LGTM,
> committed.
> 
> Besides, please give me some time to review other patches, I've been
> busy with other things for a while...

Merging smallest one will be great but I understand the situation.
Besides, some of my patchsets are not only large but complex.

And... since you added 'H' support to GNU Binutils, I think I need to
rewrite my old CSR patchset (yes, it's going to be big).

Anyway, I will submit following patchsets later (so you don't need
patchsets below until I submit newer ones):

1a. Combined floating point enhancements and fixes
1b. Minimal Zfinx fixes (incompatible with 1a)
2.  CSR addition (Smstateen/Sscofpmf/Sstc)

Thanks,
Tsukasa

> 
> Thanks
> Nelson
> 
> On Sun, Jun 5, 2022 at 12:43 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>>
>> This commit adds 'H' to canonical extension ordering based on current
>> consensus (not officially ratified as a new ISA specification manual
>> but discussion for software compatibility is made).
>>
>> bfd/ChangeLog
>>
>>         * elfxx-riscv.c (riscv_ext_canonical_order): Add 'H' for
>>         canonical extension ordering based on current consensus.
>> ---
>>  bfd/elfxx-riscv.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
>> index 92ad03feea0..5c2c616a760 100644
>> --- a/bfd/elfxx-riscv.c
>> +++ b/bfd/elfxx-riscv.c
>> @@ -1345,7 +1345,7 @@ riscv_recognized_prefixed_ext (const char *ext)
>>  }
>>
>>  /* Canonical order for single letter extensions.  */
>> -static const char riscv_ext_canonical_order[] = "eigmafdqlcbkjtpvn";
>> +static const char riscv_ext_canonical_order[] = "eigmafdqlcbkjtpvnh";
>>
>>  /* Array is used to compare the orders of standard extensions quickly.  */
>>  static int riscv_ext_order[26] = {0};
>> --
>> 2.34.1
>>
> 

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

* [PATCH v2 0/1] RISC-V: QUICK fix on Li's Zhinx implementation
  2022-06-05  4:43 [PATCH 0/2] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
  2022-06-05  4:43 ` [PATCH 1/2] RISC-V: Add 'H' to canonical extension ordering Tsukasa OI
  2022-06-05  4:43 ` [PATCH 2/2] RISC-V: Fix requirement handling on Zhinx+{D,Q} Tsukasa OI
@ 2022-06-24  2:59 ` Tsukasa OI
  2022-06-24  2:59   ` [PATCH v2 1/1] RISC-V: Fix requirement handling on Zhinx+{D,Q} Tsukasa OI
  2022-06-24  3:51   ` [PATCH v3 0/2] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
  2 siblings, 2 replies; 12+ messages in thread
From: Tsukasa OI @ 2022-06-24  2:59 UTC (permalink / raw)
  To: Tsukasa OI, Palmer Dabbelt, Kito Cheng, Nelson Chu, Weiwei Li; +Cc: binutils

This is a backup plan if my main Zfinx fixes will not make it to
Binutils 2.39 and contains the most important fix (to most likely wrong
feature gate) to Li's Zhinx implementation.

Because of this, this is incompatible with my main patchset
("RISC-V: Combined floating point enhancements") which contains all of
Zfinx/Zhinx fixes.

v1:
<https://sourceware.org/pipermail/binutils/2022-June/121170.html>
CURRENT (GitHub):
<https://github.com/a4lg/binutils-gdb/tree/riscv-zhinx-quick-fix>


This is functionally equivalent to v1 but contains a minor change.

[CHANGES: v1 -> v2]
-   Change error message to enable i18n.
-   Removed original PATCH 1 because it is merged.
    <https://sourceware.org/pipermail/binutils/2022-June/121342.html>




Tsukasa OI (1):
  RISC-V: Fix requirement handling on Zhinx+{D,Q}

 bfd/elfxx-riscv.c                        | 30 ++++++++++++++++++++----
 gas/testsuite/gas/riscv/fp-zhinx-insns.d |  2 +-
 2 files changed, 26 insertions(+), 6 deletions(-)


base-commit: 54603ee2aeaf248220f0f440c322ff02e98cd403
-- 
2.34.1


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

* [PATCH v2 1/1] RISC-V: Fix requirement handling on Zhinx+{D,Q}
  2022-06-24  2:59 ` [PATCH v2 0/1] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
@ 2022-06-24  2:59   ` Tsukasa OI
  2022-06-24  3:51   ` [PATCH v3 0/2] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
  1 sibling, 0 replies; 12+ messages in thread
From: Tsukasa OI @ 2022-06-24  2:59 UTC (permalink / raw)
  To: Tsukasa OI, Palmer Dabbelt, Kito Cheng, Nelson Chu, Weiwei Li; +Cc: binutils

This commit fixes how instructions are masked on Zhinx+Z{d,q}inx.
fcvt.h.d and fcvt.d.h require ((D&&Zfh)||(Zdinx&&Zhinx)) and
fcvt.h.q and fcvt.q.h require ((Q&&Zfh)||(Zqinx&&Zhinx)).

bfd/ChangeLog:

	* elfxx-riscv.c (riscv_multi_subset_supports): Fix feature gate
	on INSN_CLASS_{D,Q}_AND_ZFH_INX.
	(riscv_multi_subset_supports_ext): Fix feature gate diagnostics
	on INSN_CLASS_{D,Q}_AND_ZFH_INX.

gas/ChangeLog:

	* testsuite/gas/riscv/fp-zhinx-insns.d: Add Zqinx to -march
	for proper testing.
---
 bfd/elfxx-riscv.c                        | 30 ++++++++++++++++++++----
 gas/testsuite/gas/riscv/fp-zhinx-insns.d |  2 +-
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index f920e0ce9ff..308516c3e60 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -2337,15 +2337,17 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
       return riscv_subset_supports (rps, "zfh");
     case INSN_CLASS_ZFH_OR_ZHINX:
       return riscv_subset_supports (rps, "zfh")
-        || riscv_subset_supports (rps, "zhinx");
+	     || riscv_subset_supports (rps, "zhinx");
     case INSN_CLASS_D_AND_ZFH_INX:
       return (riscv_subset_supports (rps, "d")
 	      && riscv_subset_supports (rps, "zfh"))
-           || riscv_subset_supports (rps, "zhinx");
+	     || (riscv_subset_supports (rps, "zdinx")
+		 && riscv_subset_supports (rps, "zhinx"));
     case INSN_CLASS_Q_AND_ZFH_INX:
       return (riscv_subset_supports (rps, "q")
 	      && riscv_subset_supports (rps, "zfh"))
-           || riscv_subset_supports (rps, "zhinx");
+	     || (riscv_subset_supports (rps, "zqinx")
+		 && riscv_subset_supports (rps, "zhinx"));
     case INSN_CLASS_ZBA:
       return riscv_subset_supports (rps, "zba");
     case INSN_CLASS_ZBB:
@@ -2492,9 +2494,27 @@ riscv_multi_subset_supports_ext (riscv_parse_subset_t *rps,
     case INSN_CLASS_ZFH_OR_ZHINX:
       return _("zfh' or 'zhinx");
     case INSN_CLASS_D_AND_ZFH_INX:
-      return _("('d' and 'zfh') or 'zhinx");
+      if (riscv_subset_supports (rps, "zfh"))
+	return "d";
+      else if (riscv_subset_supports (rps, "d"))
+	return "zfh";
+      else if (riscv_subset_supports (rps, "zhinx"))
+	return "zdinx";
+      else if (riscv_subset_supports (rps, "zdinx"))
+	return "zhinx";
+      else
+	return _("zfh' and `d', or `zhinx' and `zdinx");
     case INSN_CLASS_Q_AND_ZFH_INX:
-      return _("('q' and 'zfh') or 'zhinx");
+      if (riscv_subset_supports (rps, "zfh"))
+	return "q";
+      else if (riscv_subset_supports (rps, "q"))
+	return "zfh";
+      else if (riscv_subset_supports (rps, "zhinx"))
+	return "zqinx";
+      else if (riscv_subset_supports (rps, "zqinx"))
+	return "zhinx";
+      else
+	return _("zfh' and `q', or `zhinx' and `zqinx");
     case INSN_CLASS_H:
       return _("h");
     default:
diff --git a/gas/testsuite/gas/riscv/fp-zhinx-insns.d b/gas/testsuite/gas/riscv/fp-zhinx-insns.d
index 6e1c40e65f5..2592d8c74e2 100644
--- a/gas/testsuite/gas/riscv/fp-zhinx-insns.d
+++ b/gas/testsuite/gas/riscv/fp-zhinx-insns.d
@@ -1,4 +1,4 @@
-#as: -march=rv64ima_zhinx
+#as: -march=rv64ima_zqinx_zhinx
 #source: fp-zhinx-insns.s
 #objdump: -dr
 
-- 
2.34.1


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

* [PATCH v3 0/2] RISC-V: QUICK fix on Li's Zhinx implementation
  2022-06-24  2:59 ` [PATCH v2 0/1] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
  2022-06-24  2:59   ` [PATCH v2 1/1] RISC-V: Fix requirement handling on Zhinx+{D,Q} Tsukasa OI
@ 2022-06-24  3:51   ` Tsukasa OI
  2022-06-24  3:51     ` [PATCH v3 1/2] RISC-V: Fix requirement handling on Zhinx+{D,Q} Tsukasa OI
  2022-06-24  3:51     ` [PATCH v3 2/2] RISC-V: Reorder Zhinx extension Tsukasa OI
  1 sibling, 2 replies; 12+ messages in thread
From: Tsukasa OI @ 2022-06-24  3:51 UTC (permalink / raw)
  To: Tsukasa OI, Palmer Dabbelt, Kito Cheng, Nelson Chu, Weiwei Li; +Cc: binutils

This is a backup plan if my main Zfinx fixes will not make it to
Binutils 2.39 and contains the most important fix (to most likely wrong
feature gate) to Li's Zhinx implementation.

Because of this, this is incompatible with my main patchset
("RISC-V: Combined floating point enhancements") which contains all of
Zfinx/Zhinx fixes.

v1:
<https://sourceware.org/pipermail/binutils/2022-June/121170.html>
v2:
<https://sourceware.org/pipermail/binutils/2022-June/121414.html>
CURRENT (GitHub):
<https://github.com/a4lg/binutils-gdb/tree/riscv-zhinx-quick-fix>


This is functionally equivalent to v1 but contains a minor change.

[CHANGES: v1 -> v2]
-   Change error message to enable i18n.
-   Removed original PATCH 1 because it is merged.
    <https://sourceware.org/pipermail/binutils/2022-June/121342.html>

[CHANGES: v2 -> v3]
-   Added PATCH 2
    (Zhinx extension definition should be reordered)




Tsukasa OI (2):
  RISC-V: Fix requirement handling on Zhinx+{D,Q}
  RISC-V: Reorder Zhinx extension

 bfd/elfxx-riscv.c                        | 32 +++++++++++++++++++-----
 gas/testsuite/gas/riscv/fp-zhinx-insns.d |  2 +-
 2 files changed, 27 insertions(+), 7 deletions(-)


base-commit: 54603ee2aeaf248220f0f440c322ff02e98cd403
-- 
2.34.1


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

* [PATCH v3 1/2] RISC-V: Fix requirement handling on Zhinx+{D,Q}
  2022-06-24  3:51   ` [PATCH v3 0/2] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
@ 2022-06-24  3:51     ` Tsukasa OI
  2022-07-07  3:02       ` Kito Cheng
  2022-06-24  3:51     ` [PATCH v3 2/2] RISC-V: Reorder Zhinx extension Tsukasa OI
  1 sibling, 1 reply; 12+ messages in thread
From: Tsukasa OI @ 2022-06-24  3:51 UTC (permalink / raw)
  To: Tsukasa OI, Palmer Dabbelt, Kito Cheng, Nelson Chu, Weiwei Li; +Cc: binutils

This commit fixes how instructions are masked on Zhinx+Z{d,q}inx.
fcvt.h.d and fcvt.d.h require ((D&&Zfh)||(Zdinx&&Zhinx)) and
fcvt.h.q and fcvt.q.h require ((Q&&Zfh)||(Zqinx&&Zhinx)).

bfd/ChangeLog:

	* elfxx-riscv.c (riscv_multi_subset_supports): Fix feature gate
	on INSN_CLASS_{D,Q}_AND_ZFH_INX.
	(riscv_multi_subset_supports_ext): Fix feature gate diagnostics
	on INSN_CLASS_{D,Q}_AND_ZFH_INX.

gas/ChangeLog:

	* testsuite/gas/riscv/fp-zhinx-insns.d: Add Zqinx to -march
	for proper testing.
---
 bfd/elfxx-riscv.c                        | 30 ++++++++++++++++++++----
 gas/testsuite/gas/riscv/fp-zhinx-insns.d |  2 +-
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index f920e0ce9ff..308516c3e60 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -2337,15 +2337,17 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
       return riscv_subset_supports (rps, "zfh");
     case INSN_CLASS_ZFH_OR_ZHINX:
       return riscv_subset_supports (rps, "zfh")
-        || riscv_subset_supports (rps, "zhinx");
+	     || riscv_subset_supports (rps, "zhinx");
     case INSN_CLASS_D_AND_ZFH_INX:
       return (riscv_subset_supports (rps, "d")
 	      && riscv_subset_supports (rps, "zfh"))
-           || riscv_subset_supports (rps, "zhinx");
+	     || (riscv_subset_supports (rps, "zdinx")
+		 && riscv_subset_supports (rps, "zhinx"));
     case INSN_CLASS_Q_AND_ZFH_INX:
       return (riscv_subset_supports (rps, "q")
 	      && riscv_subset_supports (rps, "zfh"))
-           || riscv_subset_supports (rps, "zhinx");
+	     || (riscv_subset_supports (rps, "zqinx")
+		 && riscv_subset_supports (rps, "zhinx"));
     case INSN_CLASS_ZBA:
       return riscv_subset_supports (rps, "zba");
     case INSN_CLASS_ZBB:
@@ -2492,9 +2494,27 @@ riscv_multi_subset_supports_ext (riscv_parse_subset_t *rps,
     case INSN_CLASS_ZFH_OR_ZHINX:
       return _("zfh' or 'zhinx");
     case INSN_CLASS_D_AND_ZFH_INX:
-      return _("('d' and 'zfh') or 'zhinx");
+      if (riscv_subset_supports (rps, "zfh"))
+	return "d";
+      else if (riscv_subset_supports (rps, "d"))
+	return "zfh";
+      else if (riscv_subset_supports (rps, "zhinx"))
+	return "zdinx";
+      else if (riscv_subset_supports (rps, "zdinx"))
+	return "zhinx";
+      else
+	return _("zfh' and `d', or `zhinx' and `zdinx");
     case INSN_CLASS_Q_AND_ZFH_INX:
-      return _("('q' and 'zfh') or 'zhinx");
+      if (riscv_subset_supports (rps, "zfh"))
+	return "q";
+      else if (riscv_subset_supports (rps, "q"))
+	return "zfh";
+      else if (riscv_subset_supports (rps, "zhinx"))
+	return "zqinx";
+      else if (riscv_subset_supports (rps, "zqinx"))
+	return "zhinx";
+      else
+	return _("zfh' and `q', or `zhinx' and `zqinx");
     case INSN_CLASS_H:
       return _("h");
     default:
diff --git a/gas/testsuite/gas/riscv/fp-zhinx-insns.d b/gas/testsuite/gas/riscv/fp-zhinx-insns.d
index 6e1c40e65f5..2592d8c74e2 100644
--- a/gas/testsuite/gas/riscv/fp-zhinx-insns.d
+++ b/gas/testsuite/gas/riscv/fp-zhinx-insns.d
@@ -1,4 +1,4 @@
-#as: -march=rv64ima_zhinx
+#as: -march=rv64ima_zqinx_zhinx
 #source: fp-zhinx-insns.s
 #objdump: -dr
 
-- 
2.34.1


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

* [PATCH v3 2/2] RISC-V: Reorder Zhinx extension
  2022-06-24  3:51   ` [PATCH v3 0/2] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
  2022-06-24  3:51     ` [PATCH v3 1/2] RISC-V: Fix requirement handling on Zhinx+{D,Q} Tsukasa OI
@ 2022-06-24  3:51     ` Tsukasa OI
  1 sibling, 0 replies; 12+ messages in thread
From: Tsukasa OI @ 2022-06-24  3:51 UTC (permalink / raw)
  To: Tsukasa OI, Palmer Dabbelt, Kito Cheng, Nelson Chu, Weiwei Li; +Cc: binutils

Since riscv_supported_std_z_ext must be in canonical order, Zhinx
definition must be moved.

bfd/ChangeLog:

	* elfxx-riscv.c (riscv_supported_std_z_ext): Move Zhinx
	definition.
---
 bfd/elfxx-riscv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index 308516c3e60..562e4d41eba 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -1190,7 +1190,6 @@ static struct riscv_supported_ext riscv_supported_std_z_ext[] =
   {"zfinx",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
   {"zdinx",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
   {"zqinx",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
-  {"zhinx",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
   {"zbb",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
   {"zba",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
   {"zbc",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
@@ -1226,6 +1225,7 @@ static struct riscv_supported_ext riscv_supported_std_z_ext[] =
   {"zvl16384b",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
   {"zvl32768b",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
   {"zvl65536b",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
+  {"zhinx",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
   {NULL, 0, 0, 0, 0}
 };
 
-- 
2.34.1


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

* Re: [PATCH v3 1/2] RISC-V: Fix requirement handling on Zhinx+{D,Q}
  2022-06-24  3:51     ` [PATCH v3 1/2] RISC-V: Fix requirement handling on Zhinx+{D,Q} Tsukasa OI
@ 2022-07-07  3:02       ` Kito Cheng
  2022-07-07  4:25         ` Nelson Chu
  0 siblings, 1 reply; 12+ messages in thread
From: Kito Cheng @ 2022-07-07  3:02 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: Palmer Dabbelt, Kito Cheng, Nelson Chu, Weiwei Li, Binutils

LGTM

On Fri, Jun 24, 2022 at 11:52 AM Tsukasa OI via Binutils
<binutils@sourceware.org> wrote:
>
> This commit fixes how instructions are masked on Zhinx+Z{d,q}inx.
> fcvt.h.d and fcvt.d.h require ((D&&Zfh)||(Zdinx&&Zhinx)) and
> fcvt.h.q and fcvt.q.h require ((Q&&Zfh)||(Zqinx&&Zhinx)).
>
> bfd/ChangeLog:
>
>         * elfxx-riscv.c (riscv_multi_subset_supports): Fix feature gate
>         on INSN_CLASS_{D,Q}_AND_ZFH_INX.
>         (riscv_multi_subset_supports_ext): Fix feature gate diagnostics
>         on INSN_CLASS_{D,Q}_AND_ZFH_INX.
>
> gas/ChangeLog:
>
>         * testsuite/gas/riscv/fp-zhinx-insns.d: Add Zqinx to -march
>         for proper testing.
> ---
>  bfd/elfxx-riscv.c                        | 30 ++++++++++++++++++++----
>  gas/testsuite/gas/riscv/fp-zhinx-insns.d |  2 +-
>  2 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index f920e0ce9ff..308516c3e60 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -2337,15 +2337,17 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
>        return riscv_subset_supports (rps, "zfh");
>      case INSN_CLASS_ZFH_OR_ZHINX:
>        return riscv_subset_supports (rps, "zfh")
> -        || riscv_subset_supports (rps, "zhinx");
> +            || riscv_subset_supports (rps, "zhinx");
>      case INSN_CLASS_D_AND_ZFH_INX:
>        return (riscv_subset_supports (rps, "d")
>               && riscv_subset_supports (rps, "zfh"))
> -           || riscv_subset_supports (rps, "zhinx");
> +            || (riscv_subset_supports (rps, "zdinx")
> +                && riscv_subset_supports (rps, "zhinx"));
>      case INSN_CLASS_Q_AND_ZFH_INX:
>        return (riscv_subset_supports (rps, "q")
>               && riscv_subset_supports (rps, "zfh"))
> -           || riscv_subset_supports (rps, "zhinx");
> +            || (riscv_subset_supports (rps, "zqinx")
> +                && riscv_subset_supports (rps, "zhinx"));
>      case INSN_CLASS_ZBA:
>        return riscv_subset_supports (rps, "zba");
>      case INSN_CLASS_ZBB:
> @@ -2492,9 +2494,27 @@ riscv_multi_subset_supports_ext (riscv_parse_subset_t *rps,
>      case INSN_CLASS_ZFH_OR_ZHINX:
>        return _("zfh' or 'zhinx");
>      case INSN_CLASS_D_AND_ZFH_INX:
> -      return _("('d' and 'zfh') or 'zhinx");
> +      if (riscv_subset_supports (rps, "zfh"))
> +       return "d";
> +      else if (riscv_subset_supports (rps, "d"))
> +       return "zfh";
> +      else if (riscv_subset_supports (rps, "zhinx"))
> +       return "zdinx";
> +      else if (riscv_subset_supports (rps, "zdinx"))
> +       return "zhinx";
> +      else
> +       return _("zfh' and `d', or `zhinx' and `zdinx");
>      case INSN_CLASS_Q_AND_ZFH_INX:
> -      return _("('q' and 'zfh') or 'zhinx");
> +      if (riscv_subset_supports (rps, "zfh"))
> +       return "q";
> +      else if (riscv_subset_supports (rps, "q"))
> +       return "zfh";
> +      else if (riscv_subset_supports (rps, "zhinx"))
> +       return "zqinx";
> +      else if (riscv_subset_supports (rps, "zqinx"))
> +       return "zhinx";
> +      else
> +       return _("zfh' and `q', or `zhinx' and `zqinx");
>      case INSN_CLASS_H:
>        return _("h");
>      default:
> diff --git a/gas/testsuite/gas/riscv/fp-zhinx-insns.d b/gas/testsuite/gas/riscv/fp-zhinx-insns.d
> index 6e1c40e65f5..2592d8c74e2 100644
> --- a/gas/testsuite/gas/riscv/fp-zhinx-insns.d
> +++ b/gas/testsuite/gas/riscv/fp-zhinx-insns.d
> @@ -1,4 +1,4 @@
> -#as: -march=rv64ima_zhinx
> +#as: -march=rv64ima_zqinx_zhinx
>  #source: fp-zhinx-insns.s
>  #objdump: -dr
>
> --
> 2.34.1
>

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

* Re: [PATCH v3 1/2] RISC-V: Fix requirement handling on Zhinx+{D,Q}
  2022-07-07  3:02       ` Kito Cheng
@ 2022-07-07  4:25         ` Nelson Chu
  0 siblings, 0 replies; 12+ messages in thread
From: Nelson Chu @ 2022-07-07  4:25 UTC (permalink / raw)
  To: Kito Cheng; +Cc: Tsukasa OI, Palmer Dabbelt, Kito Cheng, Weiwei Li, Binutils

OK, regressions passed so committed.

Thanks
Nelson

On Thu, Jul 7, 2022 at 11:02 AM Kito Cheng <kito.cheng@gmail.com> wrote:
>
> LGTM
>
> On Fri, Jun 24, 2022 at 11:52 AM Tsukasa OI via Binutils
> <binutils@sourceware.org> wrote:
> >
> > This commit fixes how instructions are masked on Zhinx+Z{d,q}inx.
> > fcvt.h.d and fcvt.d.h require ((D&&Zfh)||(Zdinx&&Zhinx)) and
> > fcvt.h.q and fcvt.q.h require ((Q&&Zfh)||(Zqinx&&Zhinx)).
> >
> > bfd/ChangeLog:
> >
> >         * elfxx-riscv.c (riscv_multi_subset_supports): Fix feature gate
> >         on INSN_CLASS_{D,Q}_AND_ZFH_INX.
> >         (riscv_multi_subset_supports_ext): Fix feature gate diagnostics
> >         on INSN_CLASS_{D,Q}_AND_ZFH_INX.
> >
> > gas/ChangeLog:
> >
> >         * testsuite/gas/riscv/fp-zhinx-insns.d: Add Zqinx to -march
> >         for proper testing.
> > ---
> >  bfd/elfxx-riscv.c                        | 30 ++++++++++++++++++++----
> >  gas/testsuite/gas/riscv/fp-zhinx-insns.d |  2 +-
> >  2 files changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> > index f920e0ce9ff..308516c3e60 100644
> > --- a/bfd/elfxx-riscv.c
> > +++ b/bfd/elfxx-riscv.c
> > @@ -2337,15 +2337,17 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
> >        return riscv_subset_supports (rps, "zfh");
> >      case INSN_CLASS_ZFH_OR_ZHINX:
> >        return riscv_subset_supports (rps, "zfh")
> > -        || riscv_subset_supports (rps, "zhinx");
> > +            || riscv_subset_supports (rps, "zhinx");
> >      case INSN_CLASS_D_AND_ZFH_INX:
> >        return (riscv_subset_supports (rps, "d")
> >               && riscv_subset_supports (rps, "zfh"))
> > -           || riscv_subset_supports (rps, "zhinx");
> > +            || (riscv_subset_supports (rps, "zdinx")
> > +                && riscv_subset_supports (rps, "zhinx"));
> >      case INSN_CLASS_Q_AND_ZFH_INX:
> >        return (riscv_subset_supports (rps, "q")
> >               && riscv_subset_supports (rps, "zfh"))
> > -           || riscv_subset_supports (rps, "zhinx");
> > +            || (riscv_subset_supports (rps, "zqinx")
> > +                && riscv_subset_supports (rps, "zhinx"));
> >      case INSN_CLASS_ZBA:
> >        return riscv_subset_supports (rps, "zba");
> >      case INSN_CLASS_ZBB:
> > @@ -2492,9 +2494,27 @@ riscv_multi_subset_supports_ext (riscv_parse_subset_t *rps,
> >      case INSN_CLASS_ZFH_OR_ZHINX:
> >        return _("zfh' or 'zhinx");
> >      case INSN_CLASS_D_AND_ZFH_INX:
> > -      return _("('d' and 'zfh') or 'zhinx");
> > +      if (riscv_subset_supports (rps, "zfh"))
> > +       return "d";
> > +      else if (riscv_subset_supports (rps, "d"))
> > +       return "zfh";
> > +      else if (riscv_subset_supports (rps, "zhinx"))
> > +       return "zdinx";
> > +      else if (riscv_subset_supports (rps, "zdinx"))
> > +       return "zhinx";
> > +      else
> > +       return _("zfh' and `d', or `zhinx' and `zdinx");
> >      case INSN_CLASS_Q_AND_ZFH_INX:
> > -      return _("('q' and 'zfh') or 'zhinx");
> > +      if (riscv_subset_supports (rps, "zfh"))
> > +       return "q";
> > +      else if (riscv_subset_supports (rps, "q"))
> > +       return "zfh";
> > +      else if (riscv_subset_supports (rps, "zhinx"))
> > +       return "zqinx";
> > +      else if (riscv_subset_supports (rps, "zqinx"))
> > +       return "zhinx";
> > +      else
> > +       return _("zfh' and `q', or `zhinx' and `zqinx");
> >      case INSN_CLASS_H:
> >        return _("h");
> >      default:
> > diff --git a/gas/testsuite/gas/riscv/fp-zhinx-insns.d b/gas/testsuite/gas/riscv/fp-zhinx-insns.d
> > index 6e1c40e65f5..2592d8c74e2 100644
> > --- a/gas/testsuite/gas/riscv/fp-zhinx-insns.d
> > +++ b/gas/testsuite/gas/riscv/fp-zhinx-insns.d
> > @@ -1,4 +1,4 @@
> > -#as: -march=rv64ima_zhinx
> > +#as: -march=rv64ima_zqinx_zhinx
> >  #source: fp-zhinx-insns.s
> >  #objdump: -dr
> >
> > --
> > 2.34.1
> >

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

end of thread, other threads:[~2022-07-07  4:25 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-05  4:43 [PATCH 0/2] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
2022-06-05  4:43 ` [PATCH 1/2] RISC-V: Add 'H' to canonical extension ordering Tsukasa OI
2022-06-22 10:39   ` Nelson Chu
2022-06-22 11:08     ` Tsukasa OI
2022-06-05  4:43 ` [PATCH 2/2] RISC-V: Fix requirement handling on Zhinx+{D,Q} Tsukasa OI
2022-06-24  2:59 ` [PATCH v2 0/1] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
2022-06-24  2:59   ` [PATCH v2 1/1] RISC-V: Fix requirement handling on Zhinx+{D,Q} Tsukasa OI
2022-06-24  3:51   ` [PATCH v3 0/2] RISC-V: QUICK fix on Li's Zhinx implementation Tsukasa OI
2022-06-24  3:51     ` [PATCH v3 1/2] RISC-V: Fix requirement handling on Zhinx+{D,Q} Tsukasa OI
2022-07-07  3:02       ` Kito Cheng
2022-07-07  4:25         ` Nelson Chu
2022-06-24  3:51     ` [PATCH v3 2/2] RISC-V: Reorder Zhinx extension 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).