[PATCH ver 2] rs6000, add overloaded DFP quantize support
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_, dfp_quan_i): 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 %0,%1,%2"
[(set_attr "type" "dfp")
(set_attr "size" "")])
+
+(define_insn "dfp_dquan_"
+ [(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 %0,%1,%2,%3"
+ [(set_attr "type" "dfp")
+ (set_attr "size" "")])
+
+(define_insn "dfp_dquan_i"
+ [(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 %1,%0,%2,%3"
+ [(set_attr "type" "dfp")
+ (set_attr "size" "")])
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]
cons
Re: [PATCH ver 2] rs6000, add overloaded DFP quantize support
On 8/16/23 7:19 PM, Carl Love wrote: > +(define_insn "dfp_dquan_" > + [(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 %0,%1,%2,%3" > + [(set_attr "type" "dfp") > + (set_attr "size" "")]) 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" > + [(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 %1,%0,%2,%3" > + [(set_attr "type" "dfp") > + (set_attr "size" "")]) 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
Re: [PATCH ver 2] rs6000, add overloaded DFP quantize support
on 2023/8/17 11:11, Peter Bergner wrote: > On 8/16/23 7:19 PM, Carl Love wrote: >> +(define_insn "dfp_dquan_" >> + [(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 %0,%1,%2,%3" >> + [(set_attr "type" "dfp") >> + (set_attr "size" "")]) > > 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
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_, dfp_quan_i): 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 %0,%1,%2" >[(set_attr "type" "dfp") > (set_attr "size" "")]) > + > +(define_insn "dfp_dquan_" I guess I mentioned this previously, I prefer "dfp_dqua_" 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 %0,%1,%2,%3" > + [(set_attr "type" "dfp") > + (set_attr "size" "")]) > + > +(define_insn "dfp_dquan_i" ..., also prefer "dfp_dquai_" 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 %1,%0,%2,%3" > + [(set_attr "type" "dfp") > + (set_attr "size" "")]) > 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 > @@ -
Re: [PATCH ver 2] rs6000, add overloaded DFP quantize support
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_, dfp_quan_i): 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 %0,%1,%2" > >[(set_attr "type" "dfp") > > (set_attr "size" "")]) > > + > > +(define_insn "dfp_dquan_" > > I guess I mentioned this previously, I prefer "dfp_dqua_" > 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 %0,%1,%2,%3" > > + [(set_attr "type" "dfp") > > + (set_attr "size" "")]) > > + > > +(define_insn "dfp_dquan_i" > > ..., also prefer "dfp_dquai_" 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
