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
