public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] RISC-V: correct FCVT.Q.L[U]
@ 2022-03-04 12:41 Jan Beulich
  2022-03-28  8:52 ` Ping: " Jan Beulich
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2022-03-04 12:41 UTC (permalink / raw)
  To: Binutils

While the spec isn't explicit about this, it pointing out the similarity
with the D extension ought to extend to the ignoring of a meaningless
rounding mode: "Note FCVT.D.W[U] always produces an exact result and is
unaffected by rounding mode." Hence the chosen encodings also ought to
match.

Note that to avoid breaking existing code the forms with a 3rd operand
are not removed, which means there continues to be a difference to
FCVT.D.W[U].
---
Oddly enough there's looks to be no testing of the Q (or D/F) extension
beyond what was recently added for Zqinx (Zdinx/Zfinx).
---
v2: Re-base.

--- a/gas/testsuite/gas/riscv/zqinx.d
+++ b/gas/testsuite/gas/riscv/zqinx.d
@@ -28,8 +28,8 @@ Disassembly of section .text:
 [ 	]+[0-9a-f]+:[ 	]+46158553[ 	]+fcvt.q.d[ 	]+a0,a1
 [ 	]+[0-9a-f]+:[ 	]+d6058553[ 	]+fcvt.q.w[ 	]+a0,a1
 [ 	]+[0-9a-f]+:[ 	]+d6158553[ 	]+fcvt.q.wu[ 	]+a0,a1
-[ 	]+[0-9a-f]+:[ 	]+d625f553[ 	]+fcvt.q.l[ 	]+a0,a1
-[ 	]+[0-9a-f]+:[ 	]+d635f553[ 	]+fcvt.q.lu[ 	]+a0,a1
+[ 	]+[0-9a-f]+:[ 	]+d6258553[ 	]+fcvt.q.l[ 	]+a0,a1
+[ 	]+[0-9a-f]+:[ 	]+d6358553[ 	]+fcvt.q.lu[ 	]+a0,a1
 [ 	]+[0-9a-f]+:[ 	]+26c58553[ 	]+fsgnj.q[ 	]+a0,a1,a2
 [ 	]+[0-9a-f]+:[ 	]+26c59553[ 	]+fsgnjn.q[ 	]+a0,a1,a2
 [ 	]+[0-9a-f]+:[ 	]+26c5a553[ 	]+fsgnjx.q[ 	]+a0,a1,a2
--- a/opcodes/riscv-opc.c
+++ b/opcodes/riscv-opc.c
@@ -763,9 +763,9 @@ const struct riscv_opcode riscv_opcodes[
 {"fcvt.l.q",  64, INSN_CLASS_Q_OR_ZQINX,   "d,S,m",     MATCH_FCVT_L_Q, MASK_FCVT_L_Q, match_opcode, 0 },
 {"fcvt.lu.q", 64, INSN_CLASS_Q_OR_ZQINX,   "d,S",       MATCH_FCVT_LU_Q|MASK_RM, MASK_FCVT_LU_Q|MASK_RM, match_opcode, 0 },
 {"fcvt.lu.q", 64, INSN_CLASS_Q_OR_ZQINX,   "d,S,m",     MATCH_FCVT_LU_Q, MASK_FCVT_LU_Q, match_opcode, 0 },
-{"fcvt.q.l",  64, INSN_CLASS_Q_OR_ZQINX,   "D,s",       MATCH_FCVT_Q_L|MASK_RM, MASK_FCVT_Q_L|MASK_RM, match_opcode, 0 },
+{"fcvt.q.l",  64, INSN_CLASS_Q_OR_ZQINX,   "D,s",       MATCH_FCVT_Q_L, MASK_FCVT_Q_L|MASK_RM, match_opcode, 0 },
 {"fcvt.q.l",  64, INSN_CLASS_Q_OR_ZQINX,   "D,s,m",     MATCH_FCVT_Q_L, MASK_FCVT_Q_L, match_opcode, 0 },
-{"fcvt.q.lu", 64, INSN_CLASS_Q_OR_ZQINX,   "D,s",       MATCH_FCVT_Q_LU|MASK_RM, MASK_FCVT_Q_LU|MASK_RM, match_opcode, 0 },
+{"fcvt.q.lu", 64, INSN_CLASS_Q_OR_ZQINX,   "D,s",       MATCH_FCVT_Q_LU, MASK_FCVT_Q_LU|MASK_RM, match_opcode, 0 },
 {"fcvt.q.lu", 64, INSN_CLASS_Q_OR_ZQINX,   "D,s,m",     MATCH_FCVT_Q_LU, MASK_FCVT_Q_LU, match_opcode, 0 },
 
 /* Compressed instructions.  */


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

* Ping: [PATCH v2] RISC-V: correct FCVT.Q.L[U]
  2022-03-04 12:41 [PATCH v2] RISC-V: correct FCVT.Q.L[U] Jan Beulich
@ 2022-03-28  8:52 ` Jan Beulich
  2022-03-28 20:46   ` Andrew Waterman
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2022-03-28  8:52 UTC (permalink / raw)
  To: Palmer Dabbelt, Andrew Waterman, Nelson Chu, Jim Wilson; +Cc: Binutils

On 04.03.2022 13:41, Jan Beulich via Binutils wrote:
> While the spec isn't explicit about this, it pointing out the similarity
> with the D extension ought to extend to the ignoring of a meaningless
> rounding mode: "Note FCVT.D.W[U] always produces an exact result and is
> unaffected by rounding mode." Hence the chosen encodings also ought to
> match.
> 
> Note that to avoid breaking existing code the forms with a 3rd operand
> are not removed, which means there continues to be a difference to
> FCVT.D.W[U].

I'd prefer to not commit such without arch maintainer approval; I guess
I'll give it another week and the will assume the absence of comments to
mean consent.

Thanks, Jan

> ---
> Oddly enough there's looks to be no testing of the Q (or D/F) extension
> beyond what was recently added for Zqinx (Zdinx/Zfinx).
> ---
> v2: Re-base.
> 
> --- a/gas/testsuite/gas/riscv/zqinx.d
> +++ b/gas/testsuite/gas/riscv/zqinx.d
> @@ -28,8 +28,8 @@ Disassembly of section .text:
>  [ 	]+[0-9a-f]+:[ 	]+46158553[ 	]+fcvt.q.d[ 	]+a0,a1
>  [ 	]+[0-9a-f]+:[ 	]+d6058553[ 	]+fcvt.q.w[ 	]+a0,a1
>  [ 	]+[0-9a-f]+:[ 	]+d6158553[ 	]+fcvt.q.wu[ 	]+a0,a1
> -[ 	]+[0-9a-f]+:[ 	]+d625f553[ 	]+fcvt.q.l[ 	]+a0,a1
> -[ 	]+[0-9a-f]+:[ 	]+d635f553[ 	]+fcvt.q.lu[ 	]+a0,a1
> +[ 	]+[0-9a-f]+:[ 	]+d6258553[ 	]+fcvt.q.l[ 	]+a0,a1
> +[ 	]+[0-9a-f]+:[ 	]+d6358553[ 	]+fcvt.q.lu[ 	]+a0,a1
>  [ 	]+[0-9a-f]+:[ 	]+26c58553[ 	]+fsgnj.q[ 	]+a0,a1,a2
>  [ 	]+[0-9a-f]+:[ 	]+26c59553[ 	]+fsgnjn.q[ 	]+a0,a1,a2
>  [ 	]+[0-9a-f]+:[ 	]+26c5a553[ 	]+fsgnjx.q[ 	]+a0,a1,a2
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -763,9 +763,9 @@ const struct riscv_opcode riscv_opcodes[
>  {"fcvt.l.q",  64, INSN_CLASS_Q_OR_ZQINX,   "d,S,m",     MATCH_FCVT_L_Q, MASK_FCVT_L_Q, match_opcode, 0 },
>  {"fcvt.lu.q", 64, INSN_CLASS_Q_OR_ZQINX,   "d,S",       MATCH_FCVT_LU_Q|MASK_RM, MASK_FCVT_LU_Q|MASK_RM, match_opcode, 0 },
>  {"fcvt.lu.q", 64, INSN_CLASS_Q_OR_ZQINX,   "d,S,m",     MATCH_FCVT_LU_Q, MASK_FCVT_LU_Q, match_opcode, 0 },
> -{"fcvt.q.l",  64, INSN_CLASS_Q_OR_ZQINX,   "D,s",       MATCH_FCVT_Q_L|MASK_RM, MASK_FCVT_Q_L|MASK_RM, match_opcode, 0 },
> +{"fcvt.q.l",  64, INSN_CLASS_Q_OR_ZQINX,   "D,s",       MATCH_FCVT_Q_L, MASK_FCVT_Q_L|MASK_RM, match_opcode, 0 },
>  {"fcvt.q.l",  64, INSN_CLASS_Q_OR_ZQINX,   "D,s,m",     MATCH_FCVT_Q_L, MASK_FCVT_Q_L, match_opcode, 0 },
> -{"fcvt.q.lu", 64, INSN_CLASS_Q_OR_ZQINX,   "D,s",       MATCH_FCVT_Q_LU|MASK_RM, MASK_FCVT_Q_LU|MASK_RM, match_opcode, 0 },
> +{"fcvt.q.lu", 64, INSN_CLASS_Q_OR_ZQINX,   "D,s",       MATCH_FCVT_Q_LU, MASK_FCVT_Q_LU|MASK_RM, match_opcode, 0 },
>  {"fcvt.q.lu", 64, INSN_CLASS_Q_OR_ZQINX,   "D,s,m",     MATCH_FCVT_Q_LU, MASK_FCVT_Q_LU, match_opcode, 0 },
>  
>  /* Compressed instructions.  */
> 


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

* Re: Ping: [PATCH v2] RISC-V: correct FCVT.Q.L[U]
  2022-03-28  8:52 ` Ping: " Jan Beulich
@ 2022-03-28 20:46   ` Andrew Waterman
  0 siblings, 0 replies; 3+ messages in thread
From: Andrew Waterman @ 2022-03-28 20:46 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Palmer Dabbelt, Nelson Chu, Jim Wilson, Binutils

Approved.  The old mapping is harmless, but the new mapping honors the
recommendation in the ISA spec for instructions that don't round.
I'll also add the corresponding note in the ISA spec for these
instructions.

On Mon, Mar 28, 2022 at 1:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 04.03.2022 13:41, Jan Beulich via Binutils wrote:
> > While the spec isn't explicit about this, it pointing out the similarity
> > with the D extension ought to extend to the ignoring of a meaningless
> > rounding mode: "Note FCVT.D.W[U] always produces an exact result and is
> > unaffected by rounding mode." Hence the chosen encodings also ought to
> > match.
> >
> > Note that to avoid breaking existing code the forms with a 3rd operand
> > are not removed, which means there continues to be a difference to
> > FCVT.D.W[U].
>
> I'd prefer to not commit such without arch maintainer approval; I guess
> I'll give it another week and the will assume the absence of comments to
> mean consent.
>
> Thanks, Jan
>
> > ---
> > Oddly enough there's looks to be no testing of the Q (or D/F) extension
> > beyond what was recently added for Zqinx (Zdinx/Zfinx).
> > ---
> > v2: Re-base.
> >
> > --- a/gas/testsuite/gas/riscv/zqinx.d
> > +++ b/gas/testsuite/gas/riscv/zqinx.d
> > @@ -28,8 +28,8 @@ Disassembly of section .text:
> >  [    ]+[0-9a-f]+:[   ]+46158553[     ]+fcvt.q.d[     ]+a0,a1
> >  [    ]+[0-9a-f]+:[   ]+d6058553[     ]+fcvt.q.w[     ]+a0,a1
> >  [    ]+[0-9a-f]+:[   ]+d6158553[     ]+fcvt.q.wu[    ]+a0,a1
> > -[    ]+[0-9a-f]+:[   ]+d625f553[     ]+fcvt.q.l[     ]+a0,a1
> > -[    ]+[0-9a-f]+:[   ]+d635f553[     ]+fcvt.q.lu[    ]+a0,a1
> > +[    ]+[0-9a-f]+:[   ]+d6258553[     ]+fcvt.q.l[     ]+a0,a1
> > +[    ]+[0-9a-f]+:[   ]+d6358553[     ]+fcvt.q.lu[    ]+a0,a1
> >  [    ]+[0-9a-f]+:[   ]+26c58553[     ]+fsgnj.q[      ]+a0,a1,a2
> >  [    ]+[0-9a-f]+:[   ]+26c59553[     ]+fsgnjn.q[     ]+a0,a1,a2
> >  [    ]+[0-9a-f]+:[   ]+26c5a553[     ]+fsgnjx.q[     ]+a0,a1,a2
> > --- a/opcodes/riscv-opc.c
> > +++ b/opcodes/riscv-opc.c
> > @@ -763,9 +763,9 @@ const struct riscv_opcode riscv_opcodes[
> >  {"fcvt.l.q",  64, INSN_CLASS_Q_OR_ZQINX,   "d,S,m",     MATCH_FCVT_L_Q, MASK_FCVT_L_Q, match_opcode, 0 },
> >  {"fcvt.lu.q", 64, INSN_CLASS_Q_OR_ZQINX,   "d,S",       MATCH_FCVT_LU_Q|MASK_RM, MASK_FCVT_LU_Q|MASK_RM, match_opcode, 0 },
> >  {"fcvt.lu.q", 64, INSN_CLASS_Q_OR_ZQINX,   "d,S,m",     MATCH_FCVT_LU_Q, MASK_FCVT_LU_Q, match_opcode, 0 },
> > -{"fcvt.q.l",  64, INSN_CLASS_Q_OR_ZQINX,   "D,s",       MATCH_FCVT_Q_L|MASK_RM, MASK_FCVT_Q_L|MASK_RM, match_opcode, 0 },
> > +{"fcvt.q.l",  64, INSN_CLASS_Q_OR_ZQINX,   "D,s",       MATCH_FCVT_Q_L, MASK_FCVT_Q_L|MASK_RM, match_opcode, 0 },
> >  {"fcvt.q.l",  64, INSN_CLASS_Q_OR_ZQINX,   "D,s,m",     MATCH_FCVT_Q_L, MASK_FCVT_Q_L, match_opcode, 0 },
> > -{"fcvt.q.lu", 64, INSN_CLASS_Q_OR_ZQINX,   "D,s",       MATCH_FCVT_Q_LU|MASK_RM, MASK_FCVT_Q_LU|MASK_RM, match_opcode, 0 },
> > +{"fcvt.q.lu", 64, INSN_CLASS_Q_OR_ZQINX,   "D,s",       MATCH_FCVT_Q_LU, MASK_FCVT_Q_LU|MASK_RM, match_opcode, 0 },
> >  {"fcvt.q.lu", 64, INSN_CLASS_Q_OR_ZQINX,   "D,s,m",     MATCH_FCVT_Q_LU, MASK_FCVT_Q_LU, match_opcode, 0 },
> >
> >  /* Compressed instructions.  */
> >
>

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

end of thread, other threads:[~2022-03-28 20:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-04 12:41 [PATCH v2] RISC-V: correct FCVT.Q.L[U] Jan Beulich
2022-03-28  8:52 ` Ping: " Jan Beulich
2022-03-28 20:46   ` Andrew Waterman

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