Re: [PATCH ver 2] rs6000, add overloaded DFP quantize support

Re: [PATCH ver 2] rs6000, add overloaded DFP quantize support

[Carl Love]

Kewen, Peter:

> on 2023/8/17 08:19, Carl Love wrote:
> > GCC maintainers:
> > 
> > Version 2, renamed the built-in instances.  Changed the name of the
> > overloaded built-in.  Added the missing documentation for the new
> > built-ins.  Fixed typos.  Changed name of the test.  Updated the
> > effective target for the test.  Retested the patch on Power 10LE
> > and
> > Power 8 and Power 9.
> > 
> > The following patch adds four built-ins for the decimal floating
> point
> > (DFP) quantize instructions on rs6000.  The built-ins are for 64-
> > bit
> > and 128-bit DFP operands.
> > 
> > The patch also adds a test case for the new builtins.
> > 
> > The Patch has been tested on Power 10LE and Power 9 LE/BE.
> > 
> > Please let me know if the patch is acceptable for
> > mainline.  Thanks.
> > 
> >                  Carl Love
> > 
> > 
> > 
> > --------------------------------------------------
> > [PATCH] rs6000, add overloaded DFP quantize support
> > 
> > Add decimal floating point (DFP) quantize built-ins for both 64-bit
> DFP
> > and 128-DFP operands.  In each case, there is an immediate version
> and a
> > variable version of the built-in.  The RM value is a 2-bit constant
> int
> > which specifies the rounding mode to use.  For the immediate
> > versions
> of
> > the built-in, the TE field is a 5-bit constant that specifies the
> value of
> > the ideal exponent for the result.  The built-in specifications
> > are:
> > 
> >   __Decimal64 builtin_dfp_quantize (_Decimal64, _Decimal64,
> > 				    const int RM)
> >   __Decimal64 builtin_dfp_quantize (const int TE, _Decimal64,
> > 				    const int)
> >   __Decimal128 builtin_dfp_quantize (_Decimal128, _Decimal128,
> > 				     const int RM)
> >   __Decimal128 builtin_dfp_quantize (const int TE, _Decimal128,
> > 				     const int)
> 
> Nit: Add the parameter name "RM" for all instances, otherwise the
> readers
> might feel confused what do the other two without RM mean. :)

Yes, they all should have the parameter name RM.  Fixed.

> 
> > A testcase is added for the new built-in definitions.
> 
> Nit: A PR marker line like:
> 
> 	PR target/93448
> 
> > gcc/ChangeLog:
> > 	* config/rs6000/dfp.md: New UNSPECDQUAN.
> > 	(dfp_quan_<mode>, dfp_quan_i<mode>): New define_insn.
> > 	* config/rs6000/rs6000-builtins.def (__builtin_dfp_quantize_64,
> > 	__builtin_dfp_quantize_64i, __builtin_dfp_quantize_128,
> > 	__builtin_dfp_quantize_128i): New buit-in definitions.
> > 	* config/rs6000/rs6000-overload.def (__builtin_dfp_quantize,
> > 	__builtin_dfpq_quantize): New overloaded definitions.
> 
> These entries need updates with this new revision, also miss one
> entry
Fixed with the new names, added the documentation entry.

> for documentation update.
> 
> > gcc/testsuite/
> > 	 * gcc.target/powerpc/builtin-dfp-quantize-runnable.c: New test
> > 	case.
> 
> Ditto, inconsistent name.

Fixed with the new name of the file, pr93448.c.

> 
> > ---
> >  gcc/config/rs6000/dfp.md                      |  25 ++-
> >  gcc/config/rs6000/rs6000-builtins.def         |  15 ++
> >  gcc/config/rs6000/rs6000-overload.def         |  10 +
> >  gcc/doc/extend.texi                           |  15 ++
> >  .../gcc.target/powerpc/pr93448-dfp-quantize.c | 199
> ++++++++++++++++++
> >  5 files changed, 263 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/pr93448-dfp-
> quantize.c
> > diff --git a/gcc/config/rs6000/dfp.md b/gcc/config/rs6000/dfp.md
> > index 5ed8a73ac51..abd21c5db75 100644
> > --- a/gcc/config/rs6000/dfp.md
> > +++ b/gcc/config/rs6000/dfp.md
> > @@ -271,7 +271,8 @@
> >     UNSPEC_DIEX
> >     UNSPEC_DSCLI
> >     UNSPEC_DTSTSFI
> > -   UNSPEC_DSCRI])
> > +   UNSPEC_DSCRI
> > +   UNSPEC_DQUAN])
> >  
> >  (define_code_iterator DFP_TEST [eq lt gt unordered])
> >  
> > @@ -395,3 +396,25 @@
> >    "dscri<q> %0,%1,%2"
> >    [(set_attr "type" "dfp")
> >     (set_attr "size" "<bits>")])
> > +
> > +(define_insn "dfp_dquan_<mode>"
> 
> I guess I mentioned this previously, I prefer "dfp_dqua_<mode>"
> which aligns with the most others ...

Yes, I missed that I had the extra "n" and didn't fix that part of the
name.  Sorry about that.  Updated both define_insn definitions.

> 
> > +  [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
> > +        (unspec:DDTD [(match_operand:DDTD 1 "gpc_reg_operand" "d")
> > +		      (match_operand:DDTD 2 "gpc_reg_operand" "d")
> > +		      (match_operand:QI 3 "immediate_operand" "i")]
> > +                     UNSPEC_DQUAN))]
> > +  "TARGET_DFP"
> > +  "dqua<q> %0,%1,%2,%3"
> > +  [(set_attr "type" "dfp")
> > +   (set_attr "size" "<bits>")])
> > +
> > +(define_insn "dfp_dquan_i<mode>"
> 
> ..., also prefer "dfp_dquai_<mode>" here.

Ditto on the name change fix.

> 
> Please also incorporate Peter's insightful comments on predicates
> and constraints on this part.

OK, changed to the stricter predicate constraints.

> 
> > +  [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
> > +        (unspec:DDTD [(match_operand:SI 1 "const_int_operand" "n")
> > +		      (match_operand:DDTD 2 "gpc_reg_operand" "d")
> > +		      (match_operand:SI 3 "immediate_operand" "i")]
> > +                     UNSPEC_DQUAN))]
> > +  "TARGET_DFP"
> > +  "dquai<q> %1,%0,%2,%3"
> > +  [(set_attr "type" "dfp")
> > +   (set_attr "size" "<bits>")])
> > diff --git a/gcc/config/rs6000/rs6000-builtins.def
> b/gcc/config/rs6000/rs6000-builtins.def
> > index 8a294d6c934..a7ab90771f9 100644
> > --- a/gcc/config/rs6000/rs6000-builtins.def
> > +++ b/gcc/config/rs6000/rs6000-builtins.def
> > @@ -2983,6 +2983,21 @@
> >    const unsigned long long __builtin_unpack_dec128 (_Decimal128,
> const int<1>);
> >      UNPACK_TD unpacktd {}
> >  
> > +  const _Decimal64 __builtin_dfp_dqua (_Decimal64, _Decimal64, \
> > +				       const int<2>);
> > +    DFPQUAN_64 dfp_dquan_dd {}
> > +
> > +  const _Decimal64 __builtin_dfp_dquai (const int<5>, _Decimal64,
> > \
> > +					const int<2>);
> > +    DFPQUAN_64i dfp_dquan_idd {}
> > +
> > +  const _Decimal128 __builtin_dfp_dquaq (_Decimal128, _Decimal128,
> > \
> > +					 const int<2>);
> > +    DFPQUAN_128 dfp_dquan_td {}
> > +
> > +  const _Decimal128 __builtin_dfp_dquaqi (const int<5>,
> > _Decimal128,
> \
> > +					  const int<2>);
> > +    DFPQUAN_128i dfp_dquan_itd {}
> >  
> >  [crypto]
> >    const vull __builtin_crypto_vcipher (vull, vull);
> > diff --git a/gcc/config/rs6000/rs6000-overload.def
> b/gcc/config/rs6000/rs6000-overload.def
> > index b83946f5ad8..38d92fcf1f0 100644
> > --- a/gcc/config/rs6000/rs6000-overload.def
> > +++ b/gcc/config/rs6000/rs6000-overload.def
> > @@ -195,6 +195,16 @@
> >    unsigned long long __builtin_cmpb (unsigned long long, unsigned
> long long);
> >      CMPB
> >  
> > +[DFPQUAN, dfp_quantize, __builtin_dfp_quantize]
> > +  _Decimal64 __builtin_dfp_quantize (_Decimal64, _Decimal64, const
> int);
> > +    DFPQUAN_64
> > +  _Decimal64 __builtin_dfp_quantize (const int, _Decimal64, const
> int);
> > +    DFPQUAN_64i
> > +  _Decimal128 __builtin_dfp_quantize (_Decimal128, _Decimal128,
> const int);
> > +    DFPQUAN_128
> > +  _Decimal128 __builtin_dfp_quantize (const int, _Decimal128,
> > const
> int);
> > +    DFPQUAN_128i
> > +
> >  [VEC_ABS, vec_abs, __builtin_vec_abs]
> >    vsc __builtin_vec_abs (vsc);
> >      ABS_V16QI
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > index 73a997276cb..b3b92bf0632 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -18566,6 +18566,21 @@ The builtin uses the ISA 3.0 instruction
> @code{mffscdrn} if available.
> >  Otherwise the builtin reads the FPSCR, masks the current decimal
> rounding
> >  mode bits out and OR's in the new value.
> >  
> > +_Decimal64 __builtin_dfp_quantize (_Decimal64, _Decimal64, const
> int);
> > +_Decimal64 __builtin_dfp_quantize (const int, _Decimal64, const
> int);
> > +_Decimal128 __builtin_dfp_quantize (_Decimal128, _Decimal128,
> > const
> int);
> > +_Decimal128 __builtin_dfp_quantize (const int, _Decimal128, const
> int);
> > +
> > +The @code{__builtin_dfp_quantize} built-in, converts and rounds
> > the
> second
> > +argument to the form with the same exponent as that of the first
> operand.
> 
> Nit: May be "... to the form with the exponent specified by the first
> argument
> based on the rounding mode specified by the third argument. If the
> first argument
> is a decimal floating point, its exponent is used for converting and
> rounding;
> otherwise the first argument should be a 5-bit constant int, which
> specifies
> the exponent to be used." ?

> > +If the first argument is a constant int, then the 5-bit value in
> > the
> second
> > +argument is converted and rounded to the form with the exponent
> specified by
> > +the 5-bit const int value in the first argument.  The third
> argument, constant
> > +int, is a two bit field that specifies the rounding mode.  The
> possible modes
> 
> Nit: Maybe "The third argument is a two bit constant int that
> specifies
> ... ".

OK, reworked the description for arguments. 

> 
> > +are: 00 Round to nearest, ties to even; 01 Round toward 0; 10
> > Round
> to nearest,
> > +ties away from 0; 11 Round according to DRN where DRN is the
> > Decimal
> Floating
> > +point field of the FPSCR.
> > +
> >  @end smallexample
> >  
> >  The following functions require @option{-mhard-float},
> > diff --git a/gcc/testsuite/gcc.target/powerpc/pr93448-dfp-
> > quantize.c
> b/gcc/testsuite/gcc.target/powerpc/pr93448-dfp-quantize.c
> > new file mode 100644
> > index 00000000000..9a6a1fdaea0
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/pr93448-dfp-quantize.c
> 
> Nit: Maybe just "pr93448.c" to align with the most existing?
> 
> IMHO people can easily search out this case with "grep -r
> dfp_quantize".

OK, I changed the name again.  I added a comment at the head of the
test case with the description of the test with the key words in
description that someone might use when grepping in addition to the
built-in name.

                              Carl 

Re: [PATCH ver 2] rs6000, add overloaded DFP quantize support

[Kewen.Lin]

Hi Carl,

on 2023/8/17 08:19, Carl Love wrote:
> 
> GCC maintainers:
> 
> Version 2, renamed the built-in instances.  Changed the name of the
> overloaded built-in.  Added the missing documentation for the new
> built-ins.  Fixed typos.  Changed name of the test.  Updated the
> effective target for the test.  Retested the patch on Power 10LE and
> Power 8 and Power 9.
> 
> The following patch adds four built-ins for the decimal floating point
> (DFP) quantize instructions on rs6000.  The built-ins are for 64-bit
> and 128-bit DFP operands.
> 
> The patch also adds a test case for the new builtins.
> 
> The Patch has been tested on Power 10LE and Power 9 LE/BE.
> 
> Please let me know if the patch is acceptable for mainline.  Thanks.
> 
>                  Carl Love
> 
> 
> 
> --------------------------------------------------
> [PATCH] rs6000, add overloaded DFP quantize support
> 
> Add decimal floating point (DFP) quantize built-ins for both 64-bit DFP
> and 128-DFP operands.  In each case, there is an immediate version and a
> variable version of the built-in.  The RM value is a 2-bit constant int
> which specifies the rounding mode to use.  For the immediate versions of
> the built-in, the TE field is a 5-bit constant that specifies the value of
> the ideal exponent for the result.  The built-in specifications are:
> 
>   __Decimal64 builtin_dfp_quantize (_Decimal64, _Decimal64,
> 				    const int RM)
>   __Decimal64 builtin_dfp_quantize (const int TE, _Decimal64,
> 				    const int)
>   __Decimal128 builtin_dfp_quantize (_Decimal128, _Decimal128,
> 				     const int RM)
>   __Decimal128 builtin_dfp_quantize (const int TE, _Decimal128,
> 				     const int)

Nit: Add the parameter name "RM" for all instances, otherwise the readers
might feel confused what do the other two without RM mean. :)

> 
> A testcase is added for the new built-in definitions.

Nit: A PR marker line like:

	PR target/93448

> 
> gcc/ChangeLog:
> 	* config/rs6000/dfp.md: New UNSPECDQUAN.
> 	(dfp_quan_<mode>, dfp_quan_i<mode>): New define_insn.
> 	* config/rs6000/rs6000-builtins.def (__builtin_dfp_quantize_64,
> 	__builtin_dfp_quantize_64i, __builtin_dfp_quantize_128,
> 	__builtin_dfp_quantize_128i): New buit-in definitions.
> 	* config/rs6000/rs6000-overload.def (__builtin_dfp_quantize,
> 	__builtin_dfpq_quantize): New overloaded definitions.

These entries need updates with this new revision, also miss one entry
for documentation update.

> 
> gcc/testsuite/
> 	 * gcc.target/powerpc/builtin-dfp-quantize-runnable.c: New test
> 	case.

Ditto, inconsistent name.

> ---
>  gcc/config/rs6000/dfp.md                      |  25 ++-
>  gcc/config/rs6000/rs6000-builtins.def         |  15 ++
>  gcc/config/rs6000/rs6000-overload.def         |  10 +
>  gcc/doc/extend.texi                           |  15 ++
>  .../gcc.target/powerpc/pr93448-dfp-quantize.c | 199 ++++++++++++++++++
>  5 files changed, 263 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/pr93448-dfp-quantize.c
> 
> diff --git a/gcc/config/rs6000/dfp.md b/gcc/config/rs6000/dfp.md
> index 5ed8a73ac51..abd21c5db75 100644
> --- a/gcc/config/rs6000/dfp.md
> +++ b/gcc/config/rs6000/dfp.md
> @@ -271,7 +271,8 @@
>     UNSPEC_DIEX
>     UNSPEC_DSCLI
>     UNSPEC_DTSTSFI
> -   UNSPEC_DSCRI])
> +   UNSPEC_DSCRI
> +   UNSPEC_DQUAN])
>  
>  (define_code_iterator DFP_TEST [eq lt gt unordered])
>  
> @@ -395,3 +396,25 @@
>    "dscri<q> %0,%1,%2"
>    [(set_attr "type" "dfp")
>     (set_attr "size" "<bits>")])
> +
> +(define_insn "dfp_dquan_<mode>"

I guess I mentioned this previously, I prefer "dfp_dqua_<mode>"
which aligns with the most others ...

> +  [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
> +        (unspec:DDTD [(match_operand:DDTD 1 "gpc_reg_operand" "d")
> +		      (match_operand:DDTD 2 "gpc_reg_operand" "d")
> +		      (match_operand:QI 3 "immediate_operand" "i")]
> +                     UNSPEC_DQUAN))]
> +  "TARGET_DFP"
> +  "dqua<q> %0,%1,%2,%3"
> +  [(set_attr "type" "dfp")
> +   (set_attr "size" "<bits>")])
> +
> +(define_insn "dfp_dquan_i<mode>"

..., also prefer "dfp_dquai_<mode>" here.

Please also incorporate Peter's insightful comments on predicates
and constraints on this part.

> +  [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
> +        (unspec:DDTD [(match_operand:SI 1 "const_int_operand" "n")
> +		      (match_operand:DDTD 2 "gpc_reg_operand" "d")
> +		      (match_operand:SI 3 "immediate_operand" "i")]
> +                     UNSPEC_DQUAN))]
> +  "TARGET_DFP"
> +  "dquai<q> %1,%0,%2,%3"
> +  [(set_attr "type" "dfp")
> +   (set_attr "size" "<bits>")])
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index 8a294d6c934..a7ab90771f9 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -2983,6 +2983,21 @@
>    const unsigned long long __builtin_unpack_dec128 (_Decimal128, const int<1>);
>      UNPACK_TD unpacktd {}
>  
> +  const _Decimal64 __builtin_dfp_dqua (_Decimal64, _Decimal64, \
> +				       const int<2>);
> +    DFPQUAN_64 dfp_dquan_dd {}
> +
> +  const _Decimal64 __builtin_dfp_dquai (const int<5>, _Decimal64, \
> +					const int<2>);
> +    DFPQUAN_64i dfp_dquan_idd {}
> +
> +  const _Decimal128 __builtin_dfp_dquaq (_Decimal128, _Decimal128, \
> +					 const int<2>);
> +    DFPQUAN_128 dfp_dquan_td {}
> +
> +  const _Decimal128 __builtin_dfp_dquaqi (const int<5>, _Decimal128, \
> +					  const int<2>);
> +    DFPQUAN_128i dfp_dquan_itd {}
>  
>  [crypto]
>    const vull __builtin_crypto_vcipher (vull, vull);
> diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
> index b83946f5ad8..38d92fcf1f0 100644
> --- a/gcc/config/rs6000/rs6000-overload.def
> +++ b/gcc/config/rs6000/rs6000-overload.def
> @@ -195,6 +195,16 @@
>    unsigned long long __builtin_cmpb (unsigned long long, unsigned long long);
>      CMPB
>  
> +[DFPQUAN, dfp_quantize, __builtin_dfp_quantize]
> +  _Decimal64 __builtin_dfp_quantize (_Decimal64, _Decimal64, const int);
> +    DFPQUAN_64
> +  _Decimal64 __builtin_dfp_quantize (const int, _Decimal64, const int);
> +    DFPQUAN_64i
> +  _Decimal128 __builtin_dfp_quantize (_Decimal128, _Decimal128, const int);
> +    DFPQUAN_128
> +  _Decimal128 __builtin_dfp_quantize (const int, _Decimal128, const int);
> +    DFPQUAN_128i
> +
>  [VEC_ABS, vec_abs, __builtin_vec_abs]
>    vsc __builtin_vec_abs (vsc);
>      ABS_V16QI
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 73a997276cb..b3b92bf0632 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -18566,6 +18566,21 @@ The builtin uses the ISA 3.0 instruction @code{mffscdrn} if available.
>  Otherwise the builtin reads the FPSCR, masks the current decimal rounding
>  mode bits out and OR's in the new value.
>  
> +_Decimal64 __builtin_dfp_quantize (_Decimal64, _Decimal64, const int);
> +_Decimal64 __builtin_dfp_quantize (const int, _Decimal64, const int);
> +_Decimal128 __builtin_dfp_quantize (_Decimal128, _Decimal128, const int);
> +_Decimal128 __builtin_dfp_quantize (const int, _Decimal128, const int);
> +
> +The @code{__builtin_dfp_quantize} built-in, converts and rounds the second
> +argument to the form with the same exponent as that of the first operand.

Nit: May be "... to the form with the exponent specified by the first argument
based on the rounding mode specified by the third argument. If the first argument
is a decimal floating point, its exponent is used for converting and rounding;
otherwise the first argument should be a 5-bit constant int, which specifies
the exponent to be used." ?

> +If the first argument is a constant int, then the 5-bit value in the second
> +argument is converted and rounded to the form with the exponent specified by
> +the 5-bit const int value in the first argument.  The third argument, constant
> +int, is a two bit field that specifies the rounding mode.  The possible modes

Nit: Maybe "The third argument is a two bit constant int that specifies ... ".

> +are: 00 Round to nearest, ties to even; 01 Round toward 0; 10 Round to nearest,
> +ties away from 0; 11 Round according to DRN where DRN is the Decimal Floating
> +point field of the FPSCR.
> +
>  @end smallexample
>  
>  The following functions require @option{-mhard-float},
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr93448-dfp-quantize.c b/gcc/testsuite/gcc.target/powerpc/pr93448-dfp-quantize.c
> new file mode 100644
> index 00000000000..9a6a1fdaea0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr93448-dfp-quantize.c

Nit: Maybe just "pr93448.c" to align with the most existing?

IMHO people can easily search out this case with "grep -r dfp_quantize".

BR,
Kewen

Re: [PATCH ver 2] rs6000, add overloaded DFP quantize support

[Kewen.Lin]

on 2023/8/17 11:11, Peter Bergner wrote:
> On 8/16/23 7:19 PM, Carl Love wrote:
>> +(define_insn "dfp_dquan_<mode>"
>> +  [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
>> +        (unspec:DDTD [(match_operand:DDTD 1 "gpc_reg_operand" "d")
>> +		      (match_operand:DDTD 2 "gpc_reg_operand" "d")
>> +		      (match_operand:QI 3 "immediate_operand" "i")]
>> +                     UNSPEC_DQUAN))]
>> +  "TARGET_DFP"
>> +  "dqua<q> %0,%1,%2,%3"
>> +  [(set_attr "type" "dfp")
>> +   (set_attr "size" "<bits>")])
> 
> operand 3 refers to the RMC operand field of the insn we are emitting.
> RMC is a two bit unsigned operand, so I think the predicate should be
> const_0_to_3_operand rather than immediate_operand.  It's always best
> to use a tighter predicate if we have one. Ditto for the other patterns
> with an RMC operand.

Good point!  I agree it's better to use a suitable tighter predicate here,
even if for now it's only used for bif expanding and the bif prototype
already restricts it.

> 
> I don't think we allow anything other than an integer for that operand
> value, so I _think_ that "n" is probably a better constraint than "i"?
> Ke Wen/Segher???

Yeah, I agree "n" is better for this context, it better matches your
proposed const_0_to_3_operand/s5bit_cint_operand (const_int).

BR,
Kewen

Re: [PATCH ver 2] rs6000, add overloaded DFP quantize support

[Peter Bergner]

On 8/16/23 7:19 PM, Carl Love wrote:
> +(define_insn "dfp_dquan_<mode>"
> +  [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
> +        (unspec:DDTD [(match_operand:DDTD 1 "gpc_reg_operand" "d")
> +		      (match_operand:DDTD 2 "gpc_reg_operand" "d")
> +		      (match_operand:QI 3 "immediate_operand" "i")]
> +                     UNSPEC_DQUAN))]
> +  "TARGET_DFP"
> +  "dqua<q> %0,%1,%2,%3"
> +  [(set_attr "type" "dfp")
> +   (set_attr "size" "<bits>")])

operand 3 refers to the RMC operand field of the insn we are emitting.
RMC is a two bit unsigned operand, so I think the predicate should be
const_0_to_3_operand rather than immediate_operand.  It's always best
to use a tighter predicate if we have one. Ditto for the other patterns
with an RMC operand.

I don't think we allow anything other than an integer for that operand
value, so I _think_ that "n" is probably a better constraint than "i"?
Ke Wen/Segher???


> +(define_insn "dfp_dquan_i<mode>"
> +  [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
> +        (unspec:DDTD [(match_operand:SI 1 "const_int_operand" "n")
> +		      (match_operand:DDTD 2 "gpc_reg_operand" "d")
> +		      (match_operand:SI 3 "immediate_operand" "i")]
> +                     UNSPEC_DQUAN))]
> +  "TARGET_DFP"
> +  "dquai<q> %1,%0,%2,%3"
> +  [(set_attr "type" "dfp")
> +   (set_attr "size" "<bits>")])

operand 1 refers to the TE operand field and that is a 5-bit signed operand.
For that, I think we should be using the s5bit_cint_operand predicate,
rather than const_int_operand.



Peter

[PATCH ver 2] rs6000, add overloaded DFP quantize support

[Carl Love]

GCC maintainers:

Version 2, renamed the built-in instances.  Changed the name of the
overloaded built-in.  Added the missing documentation for the new
built-ins.  Fixed typos.  Changed name of the test.  Updated the
effective target for the test.  Retested the patch on Power 10LE and
Power 8 and Power 9.

The following patch adds four built-ins for the decimal floating point
(DFP) quantize instructions on rs6000.  The built-ins are for 64-bit
and 128-bit DFP operands.

The patch also adds a test case for the new builtins.

The Patch has been tested on Power 10LE and Power 9 LE/BE.

Please let me know if the patch is acceptable for mainline.  Thanks.

                 Carl Love



--------------------------------------------------
[PATCH] rs6000, add overloaded DFP quantize support

Add decimal floating point (DFP) quantize built-ins for both 64-bit DFP
and 128-DFP operands.  In each case, there is an immediate version and a
variable version of the built-in.  The RM value is a 2-bit constant int
which specifies the rounding mode to use.  For the immediate versions of
the built-in, the TE field is a 5-bit constant that specifies the value of
the ideal exponent for the result.  The built-in specifications are:

  __Decimal64 builtin_dfp_quantize (_Decimal64, _Decimal64,
				    const int RM)
  __Decimal64 builtin_dfp_quantize (const int TE, _Decimal64,
				    const int)
  __Decimal128 builtin_dfp_quantize (_Decimal128, _Decimal128,
				     const int RM)
  __Decimal128 builtin_dfp_quantize (const int TE, _Decimal128,
				     const int)

A testcase is added for the new built-in definitions.

gcc/ChangeLog:
	* config/rs6000/dfp.md: New UNSPECDQUAN.
	(dfp_quan_<mode>, dfp_quan_i<mode>): New define_insn.
	* config/rs6000/rs6000-builtins.def (__builtin_dfp_quantize_64,
	__builtin_dfp_quantize_64i, __builtin_dfp_quantize_128,
	__builtin_dfp_quantize_128i): New buit-in definitions.
	* config/rs6000/rs6000-overload.def (__builtin_dfp_quantize,
	__builtin_dfpq_quantize): New overloaded definitions.

gcc/testsuite/
	 * gcc.target/powerpc/builtin-dfp-quantize-runnable.c: New test
	case.
---
 gcc/config/rs6000/dfp.md                      |  25 ++-
 gcc/config/rs6000/rs6000-builtins.def         |  15 ++
 gcc/config/rs6000/rs6000-overload.def         |  10 +
 gcc/doc/extend.texi                           |  15 ++
 .../gcc.target/powerpc/pr93448-dfp-quantize.c | 199 ++++++++++++++++++
 5 files changed, 263 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pr93448-dfp-quantize.c

diff --git a/gcc/config/rs6000/dfp.md b/gcc/config/rs6000/dfp.md
index 5ed8a73ac51..abd21c5db75 100644
--- a/gcc/config/rs6000/dfp.md
+++ b/gcc/config/rs6000/dfp.md
@@ -271,7 +271,8 @@
    UNSPEC_DIEX
    UNSPEC_DSCLI
    UNSPEC_DTSTSFI
-   UNSPEC_DSCRI])
+   UNSPEC_DSCRI
+   UNSPEC_DQUAN])
 
 (define_code_iterator DFP_TEST [eq lt gt unordered])
 
@@ -395,3 +396,25 @@
   "dscri<q> %0,%1,%2"
   [(set_attr "type" "dfp")
    (set_attr "size" "<bits>")])
+
+(define_insn "dfp_dquan_<mode>"
+  [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
+        (unspec:DDTD [(match_operand:DDTD 1 "gpc_reg_operand" "d")
+		      (match_operand:DDTD 2 "gpc_reg_operand" "d")
+		      (match_operand:QI 3 "immediate_operand" "i")]
+                     UNSPEC_DQUAN))]
+  "TARGET_DFP"
+  "dqua<q> %0,%1,%2,%3"
+  [(set_attr "type" "dfp")
+   (set_attr "size" "<bits>")])
+
+(define_insn "dfp_dquan_i<mode>"
+  [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
+        (unspec:DDTD [(match_operand:SI 1 "const_int_operand" "n")
+		      (match_operand:DDTD 2 "gpc_reg_operand" "d")
+		      (match_operand:SI 3 "immediate_operand" "i")]
+                     UNSPEC_DQUAN))]
+  "TARGET_DFP"
+  "dquai<q> %1,%0,%2,%3"
+  [(set_attr "type" "dfp")
+   (set_attr "size" "<bits>")])
diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index 8a294d6c934..a7ab90771f9 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -2983,6 +2983,21 @@
   const unsigned long long __builtin_unpack_dec128 (_Decimal128, const int<1>);
     UNPACK_TD unpacktd {}
 
+  const _Decimal64 __builtin_dfp_dqua (_Decimal64, _Decimal64, \
+				       const int<2>);
+    DFPQUAN_64 dfp_dquan_dd {}
+
+  const _Decimal64 __builtin_dfp_dquai (const int<5>, _Decimal64, \
+					const int<2>);
+    DFPQUAN_64i dfp_dquan_idd {}
+
+  const _Decimal128 __builtin_dfp_dquaq (_Decimal128, _Decimal128, \
+					 const int<2>);
+    DFPQUAN_128 dfp_dquan_td {}
+
+  const _Decimal128 __builtin_dfp_dquaqi (const int<5>, _Decimal128, \
+					  const int<2>);
+    DFPQUAN_128i dfp_dquan_itd {}
 
 [crypto]
   const vull __builtin_crypto_vcipher (vull, vull);
diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
index b83946f5ad8..38d92fcf1f0 100644
--- a/gcc/config/rs6000/rs6000-overload.def
+++ b/gcc/config/rs6000/rs6000-overload.def
@@ -195,6 +195,16 @@
   unsigned long long __builtin_cmpb (unsigned long long, unsigned long long);
     CMPB
 
+[DFPQUAN, dfp_quantize, __builtin_dfp_quantize]
+  _Decimal64 __builtin_dfp_quantize (_Decimal64, _Decimal64, const int);
+    DFPQUAN_64
+  _Decimal64 __builtin_dfp_quantize (const int, _Decimal64, const int);
+    DFPQUAN_64i
+  _Decimal128 __builtin_dfp_quantize (_Decimal128, _Decimal128, const int);
+    DFPQUAN_128
+  _Decimal128 __builtin_dfp_quantize (const int, _Decimal128, const int);
+    DFPQUAN_128i
+
 [VEC_ABS, vec_abs, __builtin_vec_abs]
   vsc __builtin_vec_abs (vsc);
     ABS_V16QI
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 73a997276cb..b3b92bf0632 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -18566,6 +18566,21 @@ The builtin uses the ISA 3.0 instruction @code{mffscdrn} if available.
 Otherwise the builtin reads the FPSCR, masks the current decimal rounding
 mode bits out and OR's in the new value.
 
+_Decimal64 __builtin_dfp_quantize (_Decimal64, _Decimal64, const int);
+_Decimal64 __builtin_dfp_quantize (const int, _Decimal64, const int);
+_Decimal128 __builtin_dfp_quantize (_Decimal128, _Decimal128, const int);
+_Decimal128 __builtin_dfp_quantize (const int, _Decimal128, const int);
+
+The @code{__builtin_dfp_quantize} built-in, converts and rounds the second
+argument to the form with the same exponent as that of the first operand.
+If the first argument is a constant int, then the 5-bit value in the second
+argument is converted and rounded to the form with the exponent specified by
+the 5-bit const int value in the first argument.  The third argument, constant
+int, is a two bit field that specifies the rounding mode.  The possible modes
+are: 00 Round to nearest, ties to even; 01 Round toward 0; 10 Round to nearest,
+ties away from 0; 11 Round according to DRN where DRN is the Decimal Floating
+point field of the FPSCR.
+
 @end smallexample
 
 The following functions require @option{-mhard-float},
diff --git a/gcc/testsuite/gcc.target/powerpc/pr93448-dfp-quantize.c b/gcc/testsuite/gcc.target/powerpc/pr93448-dfp-quantize.c
new file mode 100644
index 00000000000..9a6a1fdaea0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr93448-dfp-quantize.c
@@ -0,0 +1,199 @@
+/* { dg-do run } */
+/* { dg-require-effective-target  dfp_hw} */
+/* { dg-require-effective-target  has_arch_pwr6} */
+/* { dg-options "-mhard-float -O2 -save-temps" } */
+
+
+#define DEBUG 0
+
+#ifdef DEBUG
+#include <stdio.h>
+#endif
+#include <float.h>
+
+void abort (void);
+
+int main()
+{
+#define IMM2  2
+#define IMM3  3
+#define IMM4  4
+
+  _Decimal64 srcA_dfp64, srcB_dfp64;
+  _Decimal64 result_dfp64;
+  _Decimal64 expected_result_dfp64;
+  _Decimal128 srcA_dfp128, srcB_dfp128;
+  _Decimal128 result_dfp128;
+  _Decimal128 expected_result_dfp128;
+
+  /* Third argument of quantize built-ins is the rounding mode value (RMC).
+     
+     RMC    Rounding Mode
+     00     Round to nearest, ties to even
+     01     Round toward 0
+     10     Round to nearest, ties toward 0
+     11     Round according to DRN      */
+
+
+  /* Tests for quantize with 64-bit DFP variable.  */
+  srcA_dfp64 = 100.0df;
+  srcB_dfp64 = 300.456789df;
+  expected_result_dfp64 = 300.5df;
+
+  result_dfp64 = __builtin_dfp_quantize (srcA_dfp64, srcB_dfp64, 0x0);
+
+  if (result_dfp64 != expected_result_dfp64)
+#if DEBUG
+    printf("DFP 64-bit quantize of variable, RMC = 0 result does not match expected result\n");
+#else
+    abort();
+#endif
+
+  srcA_dfp64 = 100.00df;
+  srcB_dfp64 = 300.456789df;
+  expected_result_dfp64 = 300.45df;
+
+  result_dfp64 = __builtin_dfp_quantize (srcA_dfp64, srcB_dfp64, 0x1);
+
+  if (result_dfp64 != expected_result_dfp64)
+#if DEBUG
+    printf("DFP 64-bit quantize of variable, RMC = 1 result does not match expected result\n");
+#else
+    abort();
+#endif
+
+  srcA_dfp64 = 100.001df;
+  srcB_dfp64 = 3001.456789df;
+  expected_result_dfp64 = 3001.457df;
+
+  result_dfp64 = __builtin_dfp_quantize (srcA_dfp64, srcB_dfp64, 0x2);
+
+  if (result_dfp64 != expected_result_dfp64)
+#if DEBUG
+    printf("DFP 64-bit quantize of variable, RMC = 2 result does not match expected result\n");
+#else
+    abort();
+#endif
+
+  /* Tests for 64-bit quantize with immediate value.  */
+
+  srcB_dfp64 = 10.4567df;
+  expected_result_dfp64 = 000.0df;
+
+  result_dfp64 = __builtin_dfp_quantize (IMM2, srcB_dfp64, 0x0);
+
+  if (result_dfp64 != expected_result_dfp64)
+#if DEBUG
+    printf("DFP 64-bit quantize immediate, RMC = 0 result does not match expected result\n");
+#else
+    abort();
+#endif
+
+  srcB_dfp64 = 104567.891df;
+  expected_result_dfp64 = 100000.0df;
+
+  result_dfp64 = __builtin_dfp_quantize (IMM4, srcB_dfp64, 0x1);
+
+  if (result_dfp64 != expected_result_dfp64)
+#if DEBUG
+    printf("DFP 64-bit quantize immediate, RMC = 1 result does not match expected result\n");
+#else
+    abort();
+#endif
+
+  srcB_dfp64 = 109876.54321df;
+  expected_result_dfp64 = 109900.0df;
+
+  result_dfp64 = __builtin_dfp_quantize (IMM2, srcB_dfp64, 0x2);
+
+  if (result_dfp64 != expected_result_dfp64)
+#if DEBUG
+    printf("DFP 64-bit quantize immediate, RMC = 2 result does not match expected result\n");
+#else
+    abort();
+#endif
+
+  /* Tests for quantize 128-bit DFP variable.  */
+  srcA_dfp128 = 0.018df;
+  srcB_dfp128 = 50000.18345df;
+  expected_result_dfp128 = 50000.180df;
+
+  result_dfp128 = __builtin_dfp_quantize (srcA_dfp128, srcB_dfp128, 0x0);
+  
+  if (result_dfp128 != expected_result_dfp128)
+#if DEBUG
+    printf("DFP 128-bit quantize variable, RMC = 0 result does not match expected result\n");
+#else
+    abort();
+#endif
+
+  srcA_dfp128 = 8.01df;
+  srcB_dfp128 = 50000.18345df;
+  expected_result_dfp128 = 50000.18df;
+
+  result_dfp128 = __builtin_dfp_quantize (srcA_dfp128, srcB_dfp128, 0x1);
+  
+  if (result_dfp128 != expected_result_dfp128)
+#if DEBUG
+    printf("DFP 128-bit quantize variable, RMC = 1 result does not match expected result\n");
+#else
+    abort();
+#endif
+
+  srcA_dfp128 = 0.1234df;
+  srcB_dfp128 = 50000.18346789df;
+  expected_result_dfp128 = 50000.1800df;
+
+  result_dfp128 = __builtin_dfp_quantize (srcA_dfp128, srcB_dfp128, 0x2);
+  
+  if (result_dfp128 != expected_result_dfp128)
+#if DEBUG
+    printf("DFP 128-bit quantize variable, RMC = 2 result does not match expected result\n");
+#else
+    abort();
+#endif
+
+  /* Tests for 128-bit quantize with immediate value.  */
+  srcB_dfp128 = 1234.18345df;
+  expected_result_dfp128 = 1200.0df;
+
+  result_dfp128 = __builtin_dfp_quantize (IMM2, srcB_dfp128, 0x0);
+
+  if (result_dfp128 != expected_result_dfp128)
+#if DEBUG
+    printf("DFP 128-bit quantize immediate, RMC = 0 result does not match expected result\n");
+#else
+    abort();
+#endif
+
+  srcB_dfp128 = 123456.18345df;
+  expected_result_dfp128 = 120000.0df;
+
+  result_dfp128 = __builtin_dfp_quantize (IMM4, srcB_dfp128, 0x1);
+
+  if (result_dfp128 != expected_result_dfp128)
+#if DEBUG
+    printf("DFP 128-bit quantize immediate, RMC = 1 result does not match expected result\n");
+#else
+    abort();
+#endif
+
+  srcB_dfp128 = 12361834.5df;
+  expected_result_dfp128 = 12362000.0df;
+
+  result_dfp128 = __builtin_dfp_quantize (IMM3, srcB_dfp128, 0x2);
+
+  if (result_dfp128 != expected_result_dfp128)
+#if DEBUG
+    printf("DFP 128-bit quantize immediate, RMC = 2 result does not match expected result\n");
+#else
+    abort();
+#endif
+
+    return 0;
+}
+
+/* { dg-final { scan-assembler-times {\mdqua\M}   3 } } */
+/* { dg-final { scan-assembler-times {\mdquai\M}  3 } } */
+/* { dg-final { scan-assembler-times {\mdquaq\M}  3 } } */
+/* { dg-final { scan-assembler-times {\mdquaiq\M} 3 } } */
-- 
2.37.2