Re: [PATCH v8] RISC-V: Add the 'zfa' extension, version 0.2.

[Jeff Law via Gcc-patches]

On 5/6/23 06:53, jinma wrote:

diff --git a/gcc/config/riscv/iterators.md b/gcc/config/riscv/iterators.md
index 9b767038452..c81b08e3cc5 100644
--- a/gcc/config/riscv/iterators.md
+++ b/gcc/config/riscv/iterators.md
@@ -288,3 +288,8 @@ (define_int_iterator QUIET_COMPARISON [UNSPEC_FLT_QUIET 
UNSPEC_FLE_QUIET])
   (define_int_attr quiet_pattern [(UNSPEC_FLT_QUIET "lt") (UNSPEC_FLE_QUIET 
"le")])
   (define_int_attr QUIET_PATTERN [(UNSPEC_FLT_QUIET "LT") (UNSPEC_FLE_QUIET 
"LE")])
   
+(define_int_iterator ROUND [UNSPEC_ROUND UNSPEC_FLOOR UNSPEC_CEIL UNSPEC_BTRUNC UNSPEC_ROUNDEVEN UNSPEC_NEARBYINT])

+(define_int_attr round_pattern [(UNSPEC_ROUND "round") (UNSPEC_FLOOR "floor") 
(UNSPEC_CEIL "ceil")
+   (UNSPEC_BTRUNC "btrunc") (UNSPEC_ROUNDEVEN "roundeven") 
(UNSPEC_NEARBYINT "nearbyint")])
+(define_int_attr round_rm [(UNSPEC_ROUND "rmm") (UNSPEC_FLOOR "rdn") (UNSPEC_CEIL 
"rup")
+  (UNSPEC_BTRUNC "rtz") (UNSPEC_ROUNDEVEN "rne") 
(UNSPEC_NEARBYINT "dyn")])

Do we really need to use unspecs for all these cases?  I would expect
some correspond to the trunc, round, ceil, nearbyint, etc well known RTX
codes.

In general, we should try to avoid unspecs when there is a clear
semantic match between the instruction and GCC's RTX opcodes.  So please
review the existing RTX code semantics to see if any match the new
instructions.  If there are matches, use those RTX codes rather than
UNSPECs.


I'll try, thanks.



I encountered some confusion about this. I checked gcc's documents and
found no RTX codes that can correspond to round, ceil, nearbyint, etc.
Only "(fix:m x)" seems to correspond to trunc, which can be expressed
as rounding towards zero, while others have not yet been found.
You're largely correct.  My bad.  There's named patterns for round to 
integer, nearbyint, etc, but no RTX codes.  So they need to be handled 
as unspecs.  Sorry fo the confusion.


Jeff

Re: [PATCH v8] RISC-V: Add the 'zfa' extension, version 0.2.

[jinma via Gcc-patches]

> > > diff --git a/gcc/config/riscv/iterators.md b/gcc/config/riscv/iterators.md
> > > index 9b767038452..c81b08e3cc5 100644
> > > --- a/gcc/config/riscv/iterators.md
> > > +++ b/gcc/config/riscv/iterators.md
> > > @@ -288,3 +288,8 @@ (define_int_iterator QUIET_COMPARISON 
> > > [UNSPEC_FLT_QUIET UNSPEC_FLE_QUIET])
> > >   (define_int_attr quiet_pattern [(UNSPEC_FLT_QUIET "lt") 
> > > (UNSPEC_FLE_QUIET "le")])
> > >   (define_int_attr QUIET_PATTERN [(UNSPEC_FLT_QUIET "LT") 
> > > (UNSPEC_FLE_QUIET "LE")])
> > >   
> > > +(define_int_iterator ROUND [UNSPEC_ROUND UNSPEC_FLOOR UNSPEC_CEIL 
> > > UNSPEC_BTRUNC UNSPEC_ROUNDEVEN UNSPEC_NEARBYINT])
> > > +(define_int_attr round_pattern [(UNSPEC_ROUND "round") (UNSPEC_FLOOR 
> > > "floor") (UNSPEC_CEIL "ceil")
> > > + (UNSPEC_BTRUNC "btrunc") (UNSPEC_ROUNDEVEN 
> > > "roundeven") (UNSPEC_NEARBYINT "nearbyint")])
> > > +(define_int_attr round_rm [(UNSPEC_ROUND "rmm") (UNSPEC_FLOOR "rdn") 
> > > (UNSPEC_CEIL "rup")
> > > +(UNSPEC_BTRUNC "rtz") (UNSPEC_ROUNDEVEN "rne") 
> > > (UNSPEC_NEARBYINT "dyn")])
> > Do we really need to use unspecs for all these cases?  I would expect 
> > some correspond to the trunc, round, ceil, nearbyint, etc well known RTX 
> > codes.
> > 
> > In general, we should try to avoid unspecs when there is a clear 
> > semantic match between the instruction and GCC's RTX opcodes.  So please 
> > review the existing RTX code semantics to see if any match the new 
> > instructions.  If there are matches, use those RTX codes rather than 
> > UNSPECs.
> 
> I'll try, thanks.


I encountered some confusion about this. I checked gcc's documents and
found no RTX codes that can correspond to round, ceil, nearbyint, etc.
Only "(fix:m x)" seems to correspond to trunc, which can be expressed
as rounding towards zero, while others have not yet been found.


In addition, I found that other architectures also seem to adopt the
unspecs for all these cases  on the latest master branch.
arm: 
https://github.com/gcc-mirror/gcc/commit/1dd4fe1fd892458ce29f15f3ca95125a11b2534f#diff-159a39276c509272adfaeef91c2110f54f65c38f7fd1ab2f1e750af0a7f86377R1251
rs6000: 
https://github.com/gcc-mirror/gcc/commit/7042fe5ef83ff0585eb91144817105f26d566d4c#diff-1a2d4976d867ead4556899cab1dbb39f5069574276e06a2976fb62b771ece2e3R6995
i386: 
https://github.com/gcc-mirror/gcc/commit/3e8c4b925a9825fdb8c81f47b621f63108894362#diff-f00b14a8846eb6aaeb981077e36ac3668160d7dabb490beeb1f62792afa83281R23332

Can you give me some advice?

> > > @@ -1580,6 +1609,26 @@ (define_insn 
> > > "l2"
> > > [(set_attr "type" "fcvt")
> > >  (set_attr "mode" "")])
> > >   
> > > +(define_insn "2"
> > > +  [(set (match_operand:ANYF 0 "register_operand" "=f")
> > > + (unspec:ANYF
> > > + [(match_operand:ANYF 1 "register_operand" " f")]
> > > + ROUND))]
> > > +  "TARGET_HARD_FLOAT && TARGET_ZFA"
> > > +  "fround.\t%0,%1,"
> > > +  [(set_attr "type" "fcvt")
> > > +   (set_attr "mode" "")])
> > > +
> > > +(define_insn "rint2"
> > > +  [(set (match_operand:ANYF 0 "register_operand" "=f")
> > > + (unspec:ANYF
> > > + [(match_operand:ANYF 1 "register_operand" " f")]
> > > + UNSPEC_RINT))]
> > > +  "TARGET_HARD_FLOAT && TARGET_ZFA"
> > > +  "froundnx.\t%0,%1"
> > > +  [(set_attr "type" "fcvt")
> > > +   (set_attr "mode" "")])
> > Please review the existing RTX codes and their semantics in the 
> > internals manual and if any of the new instructions match those existing 
> > primitives, implement them using those RTX codes rather than with an UNSPEC.
> >
> 
> I'll try, thanks.
> 

thanks.

Jin Ma

Re: [PATCH v8] RISC-V: Add the 'zfa' extension, version 0.2.

[Jin Ma via Gcc-patches]

On 4/19/23 03:57, Jin Ma wrote:
> This patch adds the 'Zfa' extension for riscv, which is based on:
>https://github.com/riscv/riscv-isa-manual/commits/zfb
>
https://github.com/riscv/riscv-isa-manual/commit/1f038182810727f5feca311072e630d6baac51da
> 
> The binutils-gdb for 'Zfa' extension:

>https://github.com/a4lg/binutils-gdb/commits/riscv-zfa
> 
> What needs special explanation is:

> 1, The immediate number of the instructions FLI.H/S/D is represented in the 
assembly as a
>floating-point value, with scientific counting when rs1 is 1,2, and 
decimal numbers for
>the rest.
> 
>Related llvm link:

>  https://reviews.llvm.org/D145645
>Related discussion link:
>  https://github.com/riscv/riscv-isa-manual/issues/980
Right.  I think the goal right now is to get the bulk of this reviewed 
now.  Ideally we'll get to the point where the only outstanding issue is 
the interface between the assembler & gcc.


I will send a new version referring to the latest binutils(v5) in the near 
future:
https://sourceware.org/pipermail/binutils/2023-April/127060.html



> 
> 2, According to riscv-spec, "The FCVTMO D.W.D instruction was added principally to

>accelerate the processing of JavaScript Numbers.", so it seems that no 
implementation
>is required.
Fair enough.  There's seems to be a general desire to wire up builtins 
for many things that aren't directly usable by the compiler.  So 
consider such a change as a follow-up.   I don't think something like 
this should hold up the blk of Zfa.


> 
> 3, The instructions FMINM and FMAXM correspond to C23 library function fminimum and fmaximum.

>Therefore, this patch has simply implemented the pattern of 
fminm3 and
>fmaxm3 to prepare for later.
Sounds good.


> 
> gcc/ChangeLog:
> 
> 	* common/config/riscv/riscv-common.cc: Add zfa extension version.

>* config/riscv/constraints.md (Zf): Constrain the floating point number 
that the
>instructions FLI.H/S/D can load.
>((TARGET_XTHEADFMV || TARGET_ZFA) ? FP_REGS : NO_REGS): enable FMVP.D.X 
and FMVH.X.D.
>* config/riscv/iterators.md (ceil): New.
>* config/riscv/riscv-protos.h (riscv_float_const_rtx_index_for_fli): New.
>* config/riscv/riscv.cc (find_index_in_array): New.
>(riscv_float_const_rtx_index_for_fli): Get the index of the floating-point 
number that
>the instructions FLI.H/S/D can mov.
>(riscv_cannot_force_const_mem): If instruction FLI.H/S/D can be used, 
memory is not applicable.
>(riscv_const_insns): The cost of FLI.H/S/D is 3.
>(riscv_legitimize_const_move): Likewise.
>(riscv_split_64bit_move_p): If instruction FLI.H/S/D can be used, no split 
is required.
>(riscv_output_move): Output the mov instructions in zfa extension.
>(riscv_print_operand): Output the floating-point value of the FLI.H/S/D 
immediate in assembly
>(riscv_secondary_memory_needed): Likewise.
>* config/riscv/riscv.h (GP_REG_RTX_P): New.
>* config/riscv/riscv.md (fminm3): New.
> 


> index c448e6b37e9..62d9094f966 100644
> --- a/gcc/config/riscv/constraints.md
> +++ b/gcc/config/riscv/constraints.md
> @@ -118,6 +118,13 @@ (define_constraint "T"
> (and (match_operand 0 "move_operand")
>  (match_test "CONSTANT_P (op)")))
>   
> +;; Zfa constraints.

> +
> +(define_constraint "Zf"
> +  "A floating point number that can be loaded using instruction `fli` in 
zfa."
> +  (and (match_code "const_double")
> +   (match_test "(riscv_float_const_rtx_index_for_fli (op) != -1)")))
> +
>   ;; Vector constraints.
>   
>   (define_register_constraint "vr" "TARGET_VECTOR ? V_REGS : NO_REGS"

> @@ -183,8 +190,8 @@ (define_memory_constraint "Wdm"
>   
>   ;; Vendor ISA extension constraints.
>   
> -(define_register_constraint "th_f_fmv" "TARGET_XTHEADFMV ? FP_REGS : NO_REGS"

> +(define_register_constraint "th_f_fmv" "(TARGET_XTHEADFMV || TARGET_ZFA) ? 
FP_REGS : NO_REGS"
> "A floating-point register for XTheadFmv.")
>   
> -(define_register_constraint "th_r_fmv" "TARGET_XTHEADFMV ? GR_REGS : NO_REGS"

> +(define_register_constraint "th_r_fmv" "(TARGET_XTHEADFMV || TARGET_ZFA) ? 
GR_REGS : NO_REGS"
> "An integer register for XTheadFmv.")
I think Christoph had good suggestions on the constraints.  So let's go 
with his suggestions.


You might consider a follow-up patch where you use negation of one of 
the predefined constants for synthesis.  I would not be surprised at all 
if that's as efficient on some cores as loading the negated constants 
out of the constant pool.  But I don't think it has to be a part of this 
patch.




I also think the Christoph is right, and I will revise it according to his 
suggestion.





> diff --git a/gcc/config/riscv/iterators.md b/gcc/config/riscv/iterators.md
> index 9b767038452..c81b08e3cc5 100644
> --- a/gcc/config/riscv/iterators.md
> +++ b/gcc/config/riscv/iterators.md
> @@ -288,3 +288,8 @@ (define_int_iterator QUIET_COMPARISON [UNSPEC_FLT_QUIET 
UNSPEC_FLE_QUIET])
>   (define_in

Re: [PATCH v8] RISC-V: Add the 'zfa' extension, version 0.2.

[Jeff Law via Gcc-patches]

On 4/19/23 03:57, Jin Ma wrote:

This patch adds the 'Zfa' extension for riscv, which is based on:
   https://github.com/riscv/riscv-isa-manual/commits/zfb
   
https://github.com/riscv/riscv-isa-manual/commit/1f038182810727f5feca311072e630d6baac51da

The binutils-gdb for 'Zfa' extension:
   https://github.com/a4lg/binutils-gdb/commits/riscv-zfa

What needs special explanation is:
1, The immediate number of the instructions FLI.H/S/D is represented in the 
assembly as a
   floating-point value, with scientific counting when rs1 is 1,2, and decimal 
numbers for
   the rest.

   Related llvm link:
 https://reviews.llvm.org/D145645
   Related discussion link:
 https://github.com/riscv/riscv-isa-manual/issues/980
Right.  I think the goal right now is to get the bulk of this reviewed 
now.  Ideally we'll get to the point where the only outstanding issue is 
the interface between the assembler & gcc.




2, According to riscv-spec, "The FCVTMO D.W.D instruction was added principally 
to
   accelerate the processing of JavaScript Numbers.", so it seems that no 
implementation
   is required.
Fair enough.  There's seems to be a general desire to wire up builtins 
for many things that aren't directly usable by the compiler.  So 
consider such a change as a follow-up.   I don't think something like 
this should hold up the blk of Zfa.




3, The instructions FMINM and FMAXM correspond to C23 library function fminimum 
and fmaximum.
   Therefore, this patch has simply implemented the pattern of fminm3 
and
   fmaxm3 to prepare for later.

Sounds good.




gcc/ChangeLog:

* common/config/riscv/riscv-common.cc: Add zfa extension version.
* config/riscv/constraints.md (Zf): Constrain the floating point number 
that the
instructions FLI.H/S/D can load.
((TARGET_XTHEADFMV || TARGET_ZFA) ? FP_REGS : NO_REGS): enable FMVP.D.X 
and FMVH.X.D.
* config/riscv/iterators.md (ceil): New.
* config/riscv/riscv-protos.h (riscv_float_const_rtx_index_for_fli): 
New.
* config/riscv/riscv.cc (find_index_in_array): New.
(riscv_float_const_rtx_index_for_fli): Get the index of the 
floating-point number that
the instructions FLI.H/S/D can mov.
(riscv_cannot_force_const_mem): If instruction FLI.H/S/D can be used, 
memory is not applicable.
(riscv_const_insns): The cost of FLI.H/S/D is 3.
(riscv_legitimize_const_move): Likewise.
(riscv_split_64bit_move_p): If instruction FLI.H/S/D can be used, no 
split is required.
(riscv_output_move): Output the mov instructions in zfa extension.
(riscv_print_operand): Output the floating-point value of the FLI.H/S/D 
immediate in assembly
(riscv_secondary_memory_needed): Likewise.
* config/riscv/riscv.h (GP_REG_RTX_P): New.
* config/riscv/riscv.md (fminm3): New.




index c448e6b37e9..62d9094f966 100644
--- a/gcc/config/riscv/constraints.md
+++ b/gcc/config/riscv/constraints.md
@@ -118,6 +118,13 @@ (define_constraint "T"
(and (match_operand 0 "move_operand")
 (match_test "CONSTANT_P (op)")))
  
+;; Zfa constraints.

+
+(define_constraint "Zf"
+  "A floating point number that can be loaded using instruction `fli` in zfa."
+  (and (match_code "const_double")
+   (match_test "(riscv_float_const_rtx_index_for_fli (op) != -1)")))
+
  ;; Vector constraints.
  
  (define_register_constraint "vr" "TARGET_VECTOR ? V_REGS : NO_REGS"

@@ -183,8 +190,8 @@ (define_memory_constraint "Wdm"
  
  ;; Vendor ISA extension constraints.
  
-(define_register_constraint "th_f_fmv" "TARGET_XTHEADFMV ? FP_REGS : NO_REGS"

+(define_register_constraint "th_f_fmv" "(TARGET_XTHEADFMV || TARGET_ZFA) ? FP_REGS 
: NO_REGS"
"A floating-point register for XTheadFmv.")
  
-(define_register_constraint "th_r_fmv" "TARGET_XTHEADFMV ? GR_REGS : NO_REGS"

+(define_register_constraint "th_r_fmv" "(TARGET_XTHEADFMV || TARGET_ZFA) ? GR_REGS 
: NO_REGS"
"An integer register for XTheadFmv.")
I think Christoph had good suggestions on the constraints.  So let's go 
with his suggestions.


You might consider a follow-up patch where you use negation of one of 
the predefined constants for synthesis.  I would not be surprised at all 
if that's as efficient on some cores as loading the negated constants 
out of the constant pool.  But I don't think it has to be a part of this 
patch.






diff --git a/gcc/config/riscv/iterators.md b/gcc/config/riscv/iterators.md
index 9b767038452..c81b08e3cc5 100644
--- a/gcc/config/riscv/iterators.md
+++ b/gcc/config/riscv/iterators.md
@@ -288,3 +288,8 @@ (define_int_iterator QUIET_COMPARISON [UNSPEC_FLT_QUIET 
UNSPEC_FLE_QUIET])
  (define_int_attr quiet_pattern [(UNSPEC_FLT_QUIET "lt") (UNSPEC_FLE_QUIET 
"le")])
  (define_int_attr QUIET_PATTERN [(UNSPEC_FLT_QUIET "LT") (UNSPEC_FLE_QUIET 
"LE")])
  
+(define_int_iterator ROUND [UNSPEC_ROUND UNSPEC_FLOOR UNSPEC_CEIL UNSPEC_BTRUNC UNSPEC_ROUNDEVEN UNSPEC_NEARBYINT])

+(define_int_attr round_patter

Re: [PATCH v8] RISC-V: Add the 'zfa' extension, version 0.2.

[Christoph Müllner]

On Fri, May 5, 2023 at 5:13 PM Palmer Dabbelt  wrote:
>
> On Fri, 05 May 2023 08:04:53 PDT (-0700), christoph.muell...@vrull.eu wrote:
> > What I forgot to mention:
> > Zfa is frozen and in public review:
> >   https://groups.google.com/a/groups.riscv.org/g/isa-dev/c/SED4ntBkabg
>
> Thanks, I'd also forgot to send that out ;).
>
> I think the only blocker here on the specification side is the assembly
> format for FLI?  It looks like the feedback on
>  has been
> pretty minor so far.  It'd be nice to have the docs lined up before
> we merge, but we could always just call it a GNU extension -- we've
> already got a lot of that in assembler land, so I don't think it's that
> big of a deal.

I also don't think that we need to wait for that PR to land.

Nelson already gave his ok on the Binutils v4 (but after ratification,
not freeze):
  https://sourceware.org/pipermail/binutils/2023-April/127027.html

FWIW, I have meanwhile sent out a v5 for Binutils as well (there were
few changes requested).
And the v5 has been rebased and retested as well.

>
> >
> > On Fri, May 5, 2023 at 5:03 PM Christoph Müllner
> >  wrote:
> >>
> >> On Wed, Apr 19, 2023 at 11:58 AM Jin Ma  wrote:
> >> >
> >> > This patch adds the 'Zfa' extension for riscv, which is based on:
> >> >   https://github.com/riscv/riscv-isa-manual/commits/zfb
> >> >   
> >> > https://github.com/riscv/riscv-isa-manual/commit/1f038182810727f5feca311072e630d6baac51da
> >> >
> >> > The binutils-gdb for 'Zfa' extension:
> >> >   https://github.com/a4lg/binutils-gdb/commits/riscv-zfa
> >> >
> >> > What needs special explanation is:
> >> > 1, The immediate number of the instructions FLI.H/S/D is represented in 
> >> > the assembly as a
> >> >   floating-point value, with scientific counting when rs1 is 1,2, and 
> >> > decimal numbers for
> >> >   the rest.
> >> >
> >> >   Related llvm link:
> >> > https://reviews.llvm.org/D145645
> >> >   Related discussion link:
> >> > https://github.com/riscv/riscv-isa-manual/issues/980
> >> >
> >> > 2, According to riscv-spec, "The FCVTMO D.W.D instruction was added 
> >> > principally to
> >> >   accelerate the processing of JavaScript Numbers.", so it seems that no 
> >> > implementation
> >> >   is required.
> >> >
> >> > 3, The instructions FMINM and FMAXM correspond to C23 library function 
> >> > fminimum and fmaximum.
> >> >   Therefore, this patch has simply implemented the pattern of 
> >> > fminm3 and
> >> >   fmaxm3 to prepare for later.
> >> >
> >> > gcc/ChangeLog:
> >> >
> >> > * common/config/riscv/riscv-common.cc: Add zfa extension version.
> >> > * config/riscv/constraints.md (Zf): Constrain the floating point 
> >> > number that the
> >> > instructions FLI.H/S/D can load.
> >> > ((TARGET_XTHEADFMV || TARGET_ZFA) ? FP_REGS : NO_REGS): enable 
> >> > FMVP.D.X and FMVH.X.D.
> >> > * config/riscv/iterators.md (ceil): New.
> >> > * config/riscv/riscv-protos.h 
> >> > (riscv_float_const_rtx_index_for_fli): New.
> >> > * config/riscv/riscv.cc (find_index_in_array): New.
> >> > (riscv_float_const_rtx_index_for_fli): Get the index of the 
> >> > floating-point number that
> >> > the instructions FLI.H/S/D can mov.
> >> > (riscv_cannot_force_const_mem): If instruction FLI.H/S/D can be 
> >> > used, memory is not applicable.
> >> > (riscv_const_insns): The cost of FLI.H/S/D is 3.
> >> > (riscv_legitimize_const_move): Likewise.
> >> > (riscv_split_64bit_move_p): If instruction FLI.H/S/D can be 
> >> > used, no split is required.
> >> > (riscv_output_move): Output the mov instructions in zfa 
> >> > extension.
> >> > (riscv_print_operand): Output the floating-point value of the 
> >> > FLI.H/S/D immediate in assembly
> >> > (riscv_secondary_memory_needed): Likewise.
> >> > * config/riscv/riscv.h (GP_REG_RTX_P): New.
> >> > * config/riscv/riscv.md (fminm3): New.
> >> >
> >> > gcc/testsuite/ChangeLog:
> >> >
> >> > * gcc.target/riscv/zfa-fleq-fltq-rv32.c: New test.
> >> > * gcc.target/riscv/zfa-fleq-fltq.c: New test.
> >> > * gcc.target/riscv/zfa-fli-rv32.c: New test.
> >> > * gcc.target/riscv/zfa-fli-zfh-rv32.c: New test.
> >> > * gcc.target/riscv/zfa-fli-zfh.c: New test.
> >> > * gcc.target/riscv/zfa-fli.c: New test.
> >> > * gcc.target/riscv/zfa-fmovh-fmovp-rv32.c: New test.
> >> > * gcc.target/riscv/zfa-fround-rv32.c: New test.
> >> > * gcc.target/riscv/zfa-fround.c: New test.
> >> > ---
> >> >  gcc/common/config/riscv/riscv-common.cc   |   4 +
> >> >  gcc/config/riscv/constraints.md   |  11 +-
> >> >  gcc/config/riscv/iterators.md |   5 +
> >> >  gcc/config/riscv/riscv-opts.h |   3 +
> >> >  gcc/config/riscv/riscv-protos.h   |   1 +
> >> >  gcc/config/riscv/riscv.cc   

Re: [PATCH v8] RISC-V: Add the 'zfa' extension, version 0.2.

[Palmer Dabbelt]

On Fri, 05 May 2023 08:04:53 PDT (-0700), christoph.muell...@vrull.eu wrote:

What I forgot to mention:
Zfa is frozen and in public review:
  https://groups.google.com/a/groups.riscv.org/g/isa-dev/c/SED4ntBkabg


Thanks, I'd also forgot to send that out ;).

I think the only blocker here on the specification side is the assembly 
format for FLI?  It looks like the feedback on 
 has been 
pretty minor so far.  It'd be nice to have the docs lined up before 
we merge, but we could always just call it a GNU extension -- we've 
already got a lot of that in assembler land, so I don't think it's that 
big of a deal.




On Fri, May 5, 2023 at 5:03 PM Christoph Müllner
 wrote:


On Wed, Apr 19, 2023 at 11:58 AM Jin Ma  wrote:
>
> This patch adds the 'Zfa' extension for riscv, which is based on:
>   https://github.com/riscv/riscv-isa-manual/commits/zfb
>   
https://github.com/riscv/riscv-isa-manual/commit/1f038182810727f5feca311072e630d6baac51da
>
> The binutils-gdb for 'Zfa' extension:
>   https://github.com/a4lg/binutils-gdb/commits/riscv-zfa
>
> What needs special explanation is:
> 1, The immediate number of the instructions FLI.H/S/D is represented in the 
assembly as a
>   floating-point value, with scientific counting when rs1 is 1,2, and decimal 
numbers for
>   the rest.
>
>   Related llvm link:
> https://reviews.llvm.org/D145645
>   Related discussion link:
> https://github.com/riscv/riscv-isa-manual/issues/980
>
> 2, According to riscv-spec, "The FCVTMO D.W.D instruction was added 
principally to
>   accelerate the processing of JavaScript Numbers.", so it seems that no 
implementation
>   is required.
>
> 3, The instructions FMINM and FMAXM correspond to C23 library function 
fminimum and fmaximum.
>   Therefore, this patch has simply implemented the pattern of 
fminm3 and
>   fmaxm3 to prepare for later.
>
> gcc/ChangeLog:
>
> * common/config/riscv/riscv-common.cc: Add zfa extension version.
> * config/riscv/constraints.md (Zf): Constrain the floating point 
number that the
> instructions FLI.H/S/D can load.
> ((TARGET_XTHEADFMV || TARGET_ZFA) ? FP_REGS : NO_REGS): enable 
FMVP.D.X and FMVH.X.D.
> * config/riscv/iterators.md (ceil): New.
> * config/riscv/riscv-protos.h (riscv_float_const_rtx_index_for_fli): 
New.
> * config/riscv/riscv.cc (find_index_in_array): New.
> (riscv_float_const_rtx_index_for_fli): Get the index of the 
floating-point number that
> the instructions FLI.H/S/D can mov.
> (riscv_cannot_force_const_mem): If instruction FLI.H/S/D can be used, 
memory is not applicable.
> (riscv_const_insns): The cost of FLI.H/S/D is 3.
> (riscv_legitimize_const_move): Likewise.
> (riscv_split_64bit_move_p): If instruction FLI.H/S/D can be used, no 
split is required.
> (riscv_output_move): Output the mov instructions in zfa extension.
> (riscv_print_operand): Output the floating-point value of the 
FLI.H/S/D immediate in assembly
> (riscv_secondary_memory_needed): Likewise.
> * config/riscv/riscv.h (GP_REG_RTX_P): New.
> * config/riscv/riscv.md (fminm3): New.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/zfa-fleq-fltq-rv32.c: New test.
> * gcc.target/riscv/zfa-fleq-fltq.c: New test.
> * gcc.target/riscv/zfa-fli-rv32.c: New test.
> * gcc.target/riscv/zfa-fli-zfh-rv32.c: New test.
> * gcc.target/riscv/zfa-fli-zfh.c: New test.
> * gcc.target/riscv/zfa-fli.c: New test.
> * gcc.target/riscv/zfa-fmovh-fmovp-rv32.c: New test.
> * gcc.target/riscv/zfa-fround-rv32.c: New test.
> * gcc.target/riscv/zfa-fround.c: New test.
> ---
>  gcc/common/config/riscv/riscv-common.cc   |   4 +
>  gcc/config/riscv/constraints.md   |  11 +-
>  gcc/config/riscv/iterators.md |   5 +
>  gcc/config/riscv/riscv-opts.h |   3 +
>  gcc/config/riscv/riscv-protos.h   |   1 +
>  gcc/config/riscv/riscv.cc | 168 +-
>  gcc/config/riscv/riscv.h  |   1 +
>  gcc/config/riscv/riscv.md | 112 +---
>  .../gcc.target/riscv/zfa-fleq-fltq-rv32.c |  19 ++
>  .../gcc.target/riscv/zfa-fleq-fltq.c  |  19 ++
>  gcc/testsuite/gcc.target/riscv/zfa-fli-rv32.c |  79 
>  .../gcc.target/riscv/zfa-fli-zfh-rv32.c   |  41 +
>  gcc/testsuite/gcc.target/riscv/zfa-fli-zfh.c  |  41 +
>  gcc/testsuite/gcc.target/riscv/zfa-fli.c  |  79 
>  .../gcc.target/riscv/zfa-fmovh-fmovp-rv32.c   |  10 ++
>  .../gcc.target/riscv/zfa-fround-rv32.c|  42 +
>  gcc/testsuite/gcc.target/riscv/zfa-fround.c   |  42 +
>  17 files changed, 652 insertions(+), 25 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fleq-fltq-rv32.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fle

Re: [PATCH v8] RISC-V: Add the 'zfa' extension, version 0.2.

[Christoph Müllner]

What I forgot to mention:
Zfa is frozen and in public review:
  https://groups.google.com/a/groups.riscv.org/g/isa-dev/c/SED4ntBkabg

On Fri, May 5, 2023 at 5:03 PM Christoph Müllner
 wrote:
>
> On Wed, Apr 19, 2023 at 11:58 AM Jin Ma  wrote:
> >
> > This patch adds the 'Zfa' extension for riscv, which is based on:
> >   https://github.com/riscv/riscv-isa-manual/commits/zfb
> >   
> > https://github.com/riscv/riscv-isa-manual/commit/1f038182810727f5feca311072e630d6baac51da
> >
> > The binutils-gdb for 'Zfa' extension:
> >   https://github.com/a4lg/binutils-gdb/commits/riscv-zfa
> >
> > What needs special explanation is:
> > 1, The immediate number of the instructions FLI.H/S/D is represented in the 
> > assembly as a
> >   floating-point value, with scientific counting when rs1 is 1,2, and 
> > decimal numbers for
> >   the rest.
> >
> >   Related llvm link:
> > https://reviews.llvm.org/D145645
> >   Related discussion link:
> > https://github.com/riscv/riscv-isa-manual/issues/980
> >
> > 2, According to riscv-spec, "The FCVTMO D.W.D instruction was added 
> > principally to
> >   accelerate the processing of JavaScript Numbers.", so it seems that no 
> > implementation
> >   is required.
> >
> > 3, The instructions FMINM and FMAXM correspond to C23 library function 
> > fminimum and fmaximum.
> >   Therefore, this patch has simply implemented the pattern of 
> > fminm3 and
> >   fmaxm3 to prepare for later.
> >
> > gcc/ChangeLog:
> >
> > * common/config/riscv/riscv-common.cc: Add zfa extension version.
> > * config/riscv/constraints.md (Zf): Constrain the floating point 
> > number that the
> > instructions FLI.H/S/D can load.
> > ((TARGET_XTHEADFMV || TARGET_ZFA) ? FP_REGS : NO_REGS): enable 
> > FMVP.D.X and FMVH.X.D.
> > * config/riscv/iterators.md (ceil): New.
> > * config/riscv/riscv-protos.h 
> > (riscv_float_const_rtx_index_for_fli): New.
> > * config/riscv/riscv.cc (find_index_in_array): New.
> > (riscv_float_const_rtx_index_for_fli): Get the index of the 
> > floating-point number that
> > the instructions FLI.H/S/D can mov.
> > (riscv_cannot_force_const_mem): If instruction FLI.H/S/D can be 
> > used, memory is not applicable.
> > (riscv_const_insns): The cost of FLI.H/S/D is 3.
> > (riscv_legitimize_const_move): Likewise.
> > (riscv_split_64bit_move_p): If instruction FLI.H/S/D can be used, 
> > no split is required.
> > (riscv_output_move): Output the mov instructions in zfa extension.
> > (riscv_print_operand): Output the floating-point value of the 
> > FLI.H/S/D immediate in assembly
> > (riscv_secondary_memory_needed): Likewise.
> > * config/riscv/riscv.h (GP_REG_RTX_P): New.
> > * config/riscv/riscv.md (fminm3): New.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/riscv/zfa-fleq-fltq-rv32.c: New test.
> > * gcc.target/riscv/zfa-fleq-fltq.c: New test.
> > * gcc.target/riscv/zfa-fli-rv32.c: New test.
> > * gcc.target/riscv/zfa-fli-zfh-rv32.c: New test.
> > * gcc.target/riscv/zfa-fli-zfh.c: New test.
> > * gcc.target/riscv/zfa-fli.c: New test.
> > * gcc.target/riscv/zfa-fmovh-fmovp-rv32.c: New test.
> > * gcc.target/riscv/zfa-fround-rv32.c: New test.
> > * gcc.target/riscv/zfa-fround.c: New test.
> > ---
> >  gcc/common/config/riscv/riscv-common.cc   |   4 +
> >  gcc/config/riscv/constraints.md   |  11 +-
> >  gcc/config/riscv/iterators.md |   5 +
> >  gcc/config/riscv/riscv-opts.h |   3 +
> >  gcc/config/riscv/riscv-protos.h   |   1 +
> >  gcc/config/riscv/riscv.cc | 168 +-
> >  gcc/config/riscv/riscv.h  |   1 +
> >  gcc/config/riscv/riscv.md | 112 +---
> >  .../gcc.target/riscv/zfa-fleq-fltq-rv32.c |  19 ++
> >  .../gcc.target/riscv/zfa-fleq-fltq.c  |  19 ++
> >  gcc/testsuite/gcc.target/riscv/zfa-fli-rv32.c |  79 
> >  .../gcc.target/riscv/zfa-fli-zfh-rv32.c   |  41 +
> >  gcc/testsuite/gcc.target/riscv/zfa-fli-zfh.c  |  41 +
> >  gcc/testsuite/gcc.target/riscv/zfa-fli.c  |  79 
> >  .../gcc.target/riscv/zfa-fmovh-fmovp-rv32.c   |  10 ++
> >  .../gcc.target/riscv/zfa-fround-rv32.c|  42 +
> >  gcc/testsuite/gcc.target/riscv/zfa-fround.c   |  42 +
> >  17 files changed, 652 insertions(+), 25 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fleq-fltq-rv32.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fleq-fltq.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli-rv32.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli-zfh-rv32.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli-zfh.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli.c
> >  create mode 100644 gcc/testsuite/gcc.ta

Re: [PATCH v8] RISC-V: Add the 'zfa' extension, version 0.2.

[Christoph Müllner]

On Wed, Apr 19, 2023 at 11:58 AM Jin Ma  wrote:
>
> This patch adds the 'Zfa' extension for riscv, which is based on:
>   https://github.com/riscv/riscv-isa-manual/commits/zfb
>   
> https://github.com/riscv/riscv-isa-manual/commit/1f038182810727f5feca311072e630d6baac51da
>
> The binutils-gdb for 'Zfa' extension:
>   https://github.com/a4lg/binutils-gdb/commits/riscv-zfa
>
> What needs special explanation is:
> 1, The immediate number of the instructions FLI.H/S/D is represented in the 
> assembly as a
>   floating-point value, with scientific counting when rs1 is 1,2, and decimal 
> numbers for
>   the rest.
>
>   Related llvm link:
> https://reviews.llvm.org/D145645
>   Related discussion link:
> https://github.com/riscv/riscv-isa-manual/issues/980
>
> 2, According to riscv-spec, "The FCVTMO D.W.D instruction was added 
> principally to
>   accelerate the processing of JavaScript Numbers.", so it seems that no 
> implementation
>   is required.
>
> 3, The instructions FMINM and FMAXM correspond to C23 library function 
> fminimum and fmaximum.
>   Therefore, this patch has simply implemented the pattern of 
> fminm3 and
>   fmaxm3 to prepare for later.
>
> gcc/ChangeLog:
>
> * common/config/riscv/riscv-common.cc: Add zfa extension version.
> * config/riscv/constraints.md (Zf): Constrain the floating point 
> number that the
> instructions FLI.H/S/D can load.
> ((TARGET_XTHEADFMV || TARGET_ZFA) ? FP_REGS : NO_REGS): enable 
> FMVP.D.X and FMVH.X.D.
> * config/riscv/iterators.md (ceil): New.
> * config/riscv/riscv-protos.h (riscv_float_const_rtx_index_for_fli): 
> New.
> * config/riscv/riscv.cc (find_index_in_array): New.
> (riscv_float_const_rtx_index_for_fli): Get the index of the 
> floating-point number that
> the instructions FLI.H/S/D can mov.
> (riscv_cannot_force_const_mem): If instruction FLI.H/S/D can be used, 
> memory is not applicable.
> (riscv_const_insns): The cost of FLI.H/S/D is 3.
> (riscv_legitimize_const_move): Likewise.
> (riscv_split_64bit_move_p): If instruction FLI.H/S/D can be used, no 
> split is required.
> (riscv_output_move): Output the mov instructions in zfa extension.
> (riscv_print_operand): Output the floating-point value of the 
> FLI.H/S/D immediate in assembly
> (riscv_secondary_memory_needed): Likewise.
> * config/riscv/riscv.h (GP_REG_RTX_P): New.
> * config/riscv/riscv.md (fminm3): New.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/zfa-fleq-fltq-rv32.c: New test.
> * gcc.target/riscv/zfa-fleq-fltq.c: New test.
> * gcc.target/riscv/zfa-fli-rv32.c: New test.
> * gcc.target/riscv/zfa-fli-zfh-rv32.c: New test.
> * gcc.target/riscv/zfa-fli-zfh.c: New test.
> * gcc.target/riscv/zfa-fli.c: New test.
> * gcc.target/riscv/zfa-fmovh-fmovp-rv32.c: New test.
> * gcc.target/riscv/zfa-fround-rv32.c: New test.
> * gcc.target/riscv/zfa-fround.c: New test.
> ---
>  gcc/common/config/riscv/riscv-common.cc   |   4 +
>  gcc/config/riscv/constraints.md   |  11 +-
>  gcc/config/riscv/iterators.md |   5 +
>  gcc/config/riscv/riscv-opts.h |   3 +
>  gcc/config/riscv/riscv-protos.h   |   1 +
>  gcc/config/riscv/riscv.cc | 168 +-
>  gcc/config/riscv/riscv.h  |   1 +
>  gcc/config/riscv/riscv.md | 112 +---
>  .../gcc.target/riscv/zfa-fleq-fltq-rv32.c |  19 ++
>  .../gcc.target/riscv/zfa-fleq-fltq.c  |  19 ++
>  gcc/testsuite/gcc.target/riscv/zfa-fli-rv32.c |  79 
>  .../gcc.target/riscv/zfa-fli-zfh-rv32.c   |  41 +
>  gcc/testsuite/gcc.target/riscv/zfa-fli-zfh.c  |  41 +
>  gcc/testsuite/gcc.target/riscv/zfa-fli.c  |  79 
>  .../gcc.target/riscv/zfa-fmovh-fmovp-rv32.c   |  10 ++
>  .../gcc.target/riscv/zfa-fround-rv32.c|  42 +
>  gcc/testsuite/gcc.target/riscv/zfa-fround.c   |  42 +
>  17 files changed, 652 insertions(+), 25 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fleq-fltq-rv32.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fleq-fltq.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli-rv32.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli-zfh-rv32.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli-zfh.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fmovh-fmovp-rv32.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fround-rv32.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fround.c
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc 
> b/gcc/common/config/riscv/riscv-common.cc
> index 309a52def75..f9fce6bcc38 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/risc

[PATCH v8] RISC-V: Add the 'zfa' extension, version 0.2.

[Jin Ma via Gcc-patches]

This patch adds the 'Zfa' extension for riscv, which is based on:
  https://github.com/riscv/riscv-isa-manual/commits/zfb
  
https://github.com/riscv/riscv-isa-manual/commit/1f038182810727f5feca311072e630d6baac51da

The binutils-gdb for 'Zfa' extension:
  https://github.com/a4lg/binutils-gdb/commits/riscv-zfa

What needs special explanation is:
1, The immediate number of the instructions FLI.H/S/D is represented in the 
assembly as a
  floating-point value, with scientific counting when rs1 is 1,2, and decimal 
numbers for
  the rest.

  Related llvm link:
https://reviews.llvm.org/D145645
  Related discussion link:
https://github.com/riscv/riscv-isa-manual/issues/980

2, According to riscv-spec, "The FCVTMO D.W.D instruction was added principally 
to
  accelerate the processing of JavaScript Numbers.", so it seems that no 
implementation
  is required.

3, The instructions FMINM and FMAXM correspond to C23 library function fminimum 
and fmaximum.
  Therefore, this patch has simply implemented the pattern of fminm3 
and
  fmaxm3 to prepare for later.

gcc/ChangeLog:

* common/config/riscv/riscv-common.cc: Add zfa extension version.
* config/riscv/constraints.md (Zf): Constrain the floating point number 
that the
instructions FLI.H/S/D can load.
((TARGET_XTHEADFMV || TARGET_ZFA) ? FP_REGS : NO_REGS): enable FMVP.D.X 
and FMVH.X.D.
* config/riscv/iterators.md (ceil): New.
* config/riscv/riscv-protos.h (riscv_float_const_rtx_index_for_fli): 
New.
* config/riscv/riscv.cc (find_index_in_array): New.
(riscv_float_const_rtx_index_for_fli): Get the index of the 
floating-point number that
the instructions FLI.H/S/D can mov.
(riscv_cannot_force_const_mem): If instruction FLI.H/S/D can be used, 
memory is not applicable.
(riscv_const_insns): The cost of FLI.H/S/D is 3.
(riscv_legitimize_const_move): Likewise.
(riscv_split_64bit_move_p): If instruction FLI.H/S/D can be used, no 
split is required.
(riscv_output_move): Output the mov instructions in zfa extension.
(riscv_print_operand): Output the floating-point value of the FLI.H/S/D 
immediate in assembly
(riscv_secondary_memory_needed): Likewise.
* config/riscv/riscv.h (GP_REG_RTX_P): New.
* config/riscv/riscv.md (fminm3): New.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/zfa-fleq-fltq-rv32.c: New test.
* gcc.target/riscv/zfa-fleq-fltq.c: New test.
* gcc.target/riscv/zfa-fli-rv32.c: New test.
* gcc.target/riscv/zfa-fli-zfh-rv32.c: New test.
* gcc.target/riscv/zfa-fli-zfh.c: New test.
* gcc.target/riscv/zfa-fli.c: New test.
* gcc.target/riscv/zfa-fmovh-fmovp-rv32.c: New test.
* gcc.target/riscv/zfa-fround-rv32.c: New test.
* gcc.target/riscv/zfa-fround.c: New test.
---
 gcc/common/config/riscv/riscv-common.cc   |   4 +
 gcc/config/riscv/constraints.md   |  11 +-
 gcc/config/riscv/iterators.md |   5 +
 gcc/config/riscv/riscv-opts.h |   3 +
 gcc/config/riscv/riscv-protos.h   |   1 +
 gcc/config/riscv/riscv.cc | 168 +-
 gcc/config/riscv/riscv.h  |   1 +
 gcc/config/riscv/riscv.md | 112 +---
 .../gcc.target/riscv/zfa-fleq-fltq-rv32.c |  19 ++
 .../gcc.target/riscv/zfa-fleq-fltq.c  |  19 ++
 gcc/testsuite/gcc.target/riscv/zfa-fli-rv32.c |  79 
 .../gcc.target/riscv/zfa-fli-zfh-rv32.c   |  41 +
 gcc/testsuite/gcc.target/riscv/zfa-fli-zfh.c  |  41 +
 gcc/testsuite/gcc.target/riscv/zfa-fli.c  |  79 
 .../gcc.target/riscv/zfa-fmovh-fmovp-rv32.c   |  10 ++
 .../gcc.target/riscv/zfa-fround-rv32.c|  42 +
 gcc/testsuite/gcc.target/riscv/zfa-fround.c   |  42 +
 17 files changed, 652 insertions(+), 25 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fleq-fltq-rv32.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fleq-fltq.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli-rv32.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli-zfh-rv32.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli-zfh.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fli.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fmovh-fmovp-rv32.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fround-rv32.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/zfa-fround.c

diff --git a/gcc/common/config/riscv/riscv-common.cc 
b/gcc/common/config/riscv/riscv-common.cc
index 309a52def75..f9fce6bcc38 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -217,6 +217,8 @@ static const struct riscv_ext_version 
riscv_ext_version_table[] =
   {"zfh",   ISA_SPEC_CLASS_NONE, 1, 0},
   {"zfhmin",ISA_SPEC_CLASS_NONE, 1, 0},
 
+  {"zfa", ISA_SPEC_CLASS_NONE, 0, 2},
+
   {"zmm