| Message ID | or5z7oxii1.fsf@livre.home |
|---|---|
| State | New |
| Headers | show |
| Series | make sincos take type from intrinsic formal, not from result assignment | expand |
On October 6, 2020 3:15:02 AM GMT+02:00, Alexandre Oliva <oliva@adacore.com> wrote: > >This is a first step towards enabling the sincos optimization in Ada. > >The issue this patch solves is that sincos takes the type to be looked >up with mathfn_built_in from variables or temporaries in which results >of sin and cos are stored. In Ada, sin and cos are declared in an >internal aux package, with uses thereof in a standard generic package, >which ensures that the types are not what mathfn_built_in expects. But are they not compatible? Richard. >Taking the type from the intrinsic's formal parameter, as in the >patch, ensures we get the type associated with the intrinsics, >regardless of the types used to declare and import them, so the lookup >of the CEXPI intrinsic for the same type finds it. > > >for gcc/ChangeLog > > * tree-ssa-math-opts.c (execute_cse_sincos_1): Take the type > for the cexpi/sincos intrinsic interface from formals of other > intrinsics. >--- > gcc/tree-ssa-math-opts.c | 26 ++++++++++++++++++++++---- > 1 file changed, 22 insertions(+), 4 deletions(-) > >diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c >index 8423caa..31fd241 100644 >--- a/gcc/tree-ssa-math-opts.c >+++ b/gcc/tree-ssa-math-opts.c >@@ -1139,7 +1139,7 @@ execute_cse_sincos_1 (tree name) > { > gimple_stmt_iterator gsi; > imm_use_iterator use_iter; >- tree fndecl, res, type; >+ tree fndecl = NULL_TREE, res, type = NULL_TREE; > gimple *def_stmt, *use_stmt, *stmt; > int seen_cos = 0, seen_sin = 0, seen_cexpi = 0; > auto_vec<gimple *> stmts; >@@ -1147,7 +1147,6 @@ execute_cse_sincos_1 (tree name) > int i; > bool cfg_changed = false; > >- type = TREE_TYPE (name); > FOR_EACH_IMM_USE_STMT (use_stmt, use_iter, name) > { > if (gimple_code (use_stmt) != GIMPLE_CALL >@@ -1169,15 +1168,34 @@ execute_cse_sincos_1 (tree name) > break; > > default:; >+ continue; > } >- } > >+ tree t = TREE_VALUE (TYPE_ARG_TYPES (gimple_call_fntype >(use_stmt))); >+ if (!type) >+ type = t; >+ else if (t != type) >+ { >+ if (!tree_nop_conversion_p (type, t)) >+ return false; >+ /* If there is more than one type to choose from, prefer one >+ that has a CEXPI builtin. */ >+ else if (!fndecl >+ && (fndecl = mathfn_built_in (t, BUILT_IN_CEXPI))) >+ type = t; >+ } >+ } > if (seen_cos + seen_sin + seen_cexpi <= 1) > return false; > >+ if (type != TREE_TYPE (name) >+ && !tree_nop_conversion_p (type, TREE_TYPE (name))) >+ return false; >+ > /* Simply insert cexpi at the beginning of top_bb but not earlier than > the name def statement. */ >- fndecl = mathfn_built_in (type, BUILT_IN_CEXPI); >+ if (!fndecl) >+ fndecl = mathfn_built_in (type, BUILT_IN_CEXPI); > if (!fndecl) > return false; > stmt = gimple_build_call (fndecl, 1, name);
On Oct 6, 2020, Richard Biener <richard.guenther@gmail.com> wrote: > On October 6, 2020 3:15:02 AM GMT+02:00, Alexandre Oliva <oliva@adacore.com> wrote: >> >> This is a first step towards enabling the sincos optimization in Ada. >> >> The issue this patch solves is that sincos takes the type to be looked >> up with mathfn_built_in from variables or temporaries in which results >> of sin and cos are stored. In Ada, sin and cos are declared in an >> internal aux package, with uses thereof in a standard generic package, >> which ensures that the types are not what mathfn_built_in expects. > But are they not compatible? They are, in that they use the same underlying representation, but they're distinct types, not associated with the same TYPE_MAIN_VARIANT. In Ada it's not unusual to have multiple floating-point types unrelated to each other, even if they share identical underlying representation. Each such type is a distinct type, in a similar sense that in C++ each struct type holding a single double field is a distinct type. Each such distinct FP type gets a different instantiation of Ada.Numerics.Generic_Elementary_Functions, just as a C++ template taking a parameter type would get different instantiations for such different struct types. Overall, it's a very confusing situation. We use these alternate types to declare the Sin and Cos functions imported from libm as intrinsics (separate patch I've written very recently, yet to be contributed), and they get matched to the libm intrinsics despite the distinct types, we issue calls to them, passing variables of the alternate types without explicit conversions, but when the sincos pass looks up the sincos/cexpi intrinsic, it uses the alternate type taken from the variable and fails, rather than the types declared as taken by the builtins.
On Tue, Oct 6, 2020 at 9:21 AM Alexandre Oliva <oliva@adacore.com> wrote: > > On Oct 6, 2020, Richard Biener <richard.guenther@gmail.com> wrote: > > > On October 6, 2020 3:15:02 AM GMT+02:00, Alexandre Oliva <oliva@adacore.com> wrote: > >> > >> This is a first step towards enabling the sincos optimization in Ada. > >> > >> The issue this patch solves is that sincos takes the type to be looked > >> up with mathfn_built_in from variables or temporaries in which results > >> of sin and cos are stored. In Ada, sin and cos are declared in an > >> internal aux package, with uses thereof in a standard generic package, > >> which ensures that the types are not what mathfn_built_in expects. > > > But are they not compatible? > > They are, in that they use the same underlying representation, but > they're distinct types, not associated with the same TYPE_MAIN_VARIANT. > > In Ada it's not unusual to have multiple floating-point types unrelated > to each other, even if they share identical underlying representation. > Each such type is a distinct type, in a similar sense that in C++ each > struct type holding a single double field is a distinct type. > > Each such distinct FP type gets a different instantiation of > Ada.Numerics.Generic_Elementary_Functions, just as a C++ template taking > a parameter type would get different instantiations for such different > struct types. > > > Overall, it's a very confusing situation. We use these alternate types > to declare the Sin and Cos functions imported from libm as intrinsics > (separate patch I've written very recently, yet to be contributed), and > they get matched to the libm intrinsics despite the distinct types, we > issue calls to them, passing variables of the alternate types without > explicit conversions, but when the sincos pass looks up the sincos/cexpi > intrinsic, it uses the alternate type taken from the variable and fails, > rather than the types declared as taken by the builtins. OK, I see. mathfn_built_in expects a type inter-operating with the C ABI types (float_type_node, double_type_node, etc.) where "inter-operating" means having the same main variant. Now, I guess for the sincos pass we want to combine sinl + cosl to sincosl, independent on the case where the result would be assigned to a 'double' when 'double == long double'? Now what about sinl + cos when 'double == long double'? What I'm after is rather than going with sth like your patch use the original builtin code to derive the canonical C ABI type, sort-of a "reverse" mathfn_built_in. In execute_cse_sincos_1 we have the switch (gimple_call_combined_fn (use_stmt)) { CASE_CFN_COS: seen_cos |= maybe_record_sincos (&stmts, &top_bb, use_stmt) ? 1 : 0; break; CASE_CFN_SIN: seen_sin |= maybe_record_sincos (&stmts, &top_bb, use_stmt) ? 1 : 0; break; CASE_CFN_CEXPI: seen_cexpi |= maybe_record_sincos (&stmts, &top_bb, use_stmt) ? 1 : 0; break; default:; } switch so we _do_ have an idea what builtins we call and thus we could add a tree mathfn_type (combined_fn fn) { switch (fn) { case all-double-typed-fns: return double_type_node; case ...: ... } function that might prove useful in other places we're trying to find a "matching" alternate builtin function? Maybe this function can also simply look at the implicit/explicit defined decls but at least for the case of lrint using the return value will be misleading. Richard. > -- > Alexandre Oliva, happy hacker > https://FSFLA.org/blogs/lxo/ > Free Software Activist > GNU Toolchain Engineer
On Oct 6, 2020, Richard Biener <richard.guenther@gmail.com> wrote: > OK, I see. mathfn_built_in expects a type inter-operating with > the C ABI types (float_type_node, double_type_node, etc.) where > "inter-operating" means having the same main variant. Yup. > Now, I guess for the sincos pass we want to combine sinl + cosl > to sincosl, independent on the case where the result would be > assigned to a 'double' when 'double == long double'? Sorry, I goofed in the patch description and misled you. When looking at _d = sin (_s); the sincos didn't take the type of _d, but that of _s. I changed it so it takes the not from the actual passed to the intrinsic, but from the formal in the intrinsic declaration. If we had conversions of _s to different precisions, the optimization wouldn't kick in: we'd have different actuals passed to sin and cos. I'm not sure it makes much sense to try to turn e.g. _d1 = sin (_s); _t = (float) _s; _d2 = cosf (_t); into: sincos (_s, &D1, &T); _d1 = D1; _td2 = T; _d2 = (float) _td2; If someone goes through the trouble of computing sin and cos for the same angle at different precisions, you might as well leave it alone. > Now what about sinl + cos when 'double == long double'? Now that might make more sense to optimize, but if we're going to do such transformations, we might as well canonicalize the *l intrinsics to the equivalent double versions (assuming long double and double have the same precision), and then sincos will get it right.
On Tue, Oct 6, 2020 at 11:34 AM Alexandre Oliva <oliva@adacore.com> wrote: > > On Oct 6, 2020, Richard Biener <richard.guenther@gmail.com> wrote: > > > OK, I see. mathfn_built_in expects a type inter-operating with > > the C ABI types (float_type_node, double_type_node, etc.) where > > "inter-operating" means having the same main variant. > > Yup. > > > Now, I guess for the sincos pass we want to combine sinl + cosl > > to sincosl, independent on the case where the result would be > > assigned to a 'double' when 'double == long double'? > > Sorry, I goofed in the patch description and misled you. > > When looking at > > _d = sin (_s); > > the sincos didn't take the type of _d, but that of _s. > > I changed it so it takes the not from the actual passed to the > intrinsic, but from the formal in the intrinsic declaration. Yes, I understand. > If we had conversions of _s to different precisions, the optimization > wouldn't kick in: we'd have different actuals passed to sin and cos. > I'm not sure it makes much sense to try to turn e.g. > > _d1 = sin (_s); > _t = (float) _s; > _d2 = cosf (_t); > > into: > > sincos (_s, &D1, &T); > _d1 = D1; > _td2 = T; > _d2 = (float) _td2; > > If someone goes through the trouble of computing sin and cos for the > same angle at different precisions, you might as well leave it alone. > > > Now what about sinl + cos when 'double == long double'? > > Now that might make more sense to optimize, but if we're going to do > such transformations, we might as well canonicalize the *l intrinsics to > the equivalent double versions (assuming long double and double have the > same precision), and then sincos will get it right. Ah, we eventually already do that. So how about that mathfn_type helper instead of hard-wring this logic in sincos()? Richard. > > -- > Alexandre Oliva, happy hacker > https://FSFLA.org/blogs/lxo/ > Free Software Activist > GNU Toolchain Engineer
On Oct 6, 2020, Richard Biener <richard.guenther@gmail.com> wrote: > So how about that mathfn_type helper instead of hard-wring this logic > in sincos()? Like this? Regstrapped on x86_64-linux-gnu. Ok to install? I'm a little unhappy with the duplication of the CASE_MATHFN* sequence, that ought to be kept in sync, , and considered turning that whole sequence into a #define used in both places, but that would bloat the patch further and make it less readable, so I figured I'd propose this one first. Please let me know if you agree this additional change would make it better. take type from intrinsic in sincos pass From: Alexandre Oliva <oliva@adacore.com> This is a first step towards enabling the sincos optimization in Ada. The issue this patch solves is that sincos takes the type to be looked up with mathfn_built_in from variables or temporaries passed as arguments to SIN and COS intrinsics. In Ada, different float types may be used but, despite their representation equivalence, their distinctness causes the optimization to be skipped, because they are not the types that mathfn_built_in expects. This patch introduces a function that maps intrinsics to the type they're associated with, and uses that type, obtained from the intrinsics used in calls to be optimized, to look up the correspoding CEXPI intrinsic. For the sake of defensive programming, when using the type obtained from the intrinsic, it now checks that, if different types are found for the used argument, or for other calls that use it, that the types are interchangeable. for gcc/ChangeLog * builtins.c (mathfn_built_in_type): New. * builtins.h (mathfn_built_in_type): Declare. * tree-ssa-math-opts.c (execute_cse_sincos_1): Use it to obtain the type expected by the intrinsic. --- gcc/builtins.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++ gcc/builtins.h | 1 gcc/tree-ssa-math-opts.c | 17 ++++- 3 files changed, 162 insertions(+), 3 deletions(-) diff --git a/gcc/builtins.c b/gcc/builtins.c index f91266e4..5649242 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -2160,6 +2160,7 @@ mathfn_built_in_2 (tree type, combined_fn fn) switch (fn) { + /* Copied to mathfn_built_in_type, please keep in sync. */ CASE_MATHFN (ACOS) CASE_MATHFN (ACOSH) CASE_MATHFN (ASIN) @@ -2278,6 +2279,10 @@ mathfn_built_in_2 (tree type, combined_fn fn) return END_BUILTINS; } +#undef CASE_MATHFN +#undef CASE_MATHFN_FLOATN +#undef CASE_MATHFN_REENT + /* Return mathematic function equivalent to FN but operating directly on TYPE, if available. If IMPLICIT_P is true use the implicit builtin declaration, otherwise use the explicit declaration. If we can't do the conversion, @@ -2313,6 +2318,148 @@ mathfn_built_in (tree type, enum built_in_function fn) return mathfn_built_in_1 (type, as_combined_fn (fn), /*implicit=*/ 1); } +/* Return the type associated with a built in function, i.e., the one + to be passed to mathfn_built_in to get the type-specific + function. */ + +tree +mathfn_built_in_type (combined_fn fn) +{ +#define CASE_MATHFN(MATHFN) \ + case BUILT_IN_##MATHFN: \ + return double_type_node; \ + case BUILT_IN_##MATHFN##F: \ + return float_type_node; \ + case BUILT_IN_##MATHFN##L: \ + return long_double_type_node; + +#define CASE_MATHFN_FLOATN(MATHFN) \ + CASE_MATHFN(MATHFN) \ + case BUILT_IN_##MATHFN##F16: \ + return float16_type_node; \ + case BUILT_IN_##MATHFN##F32: \ + return float32_type_node; \ + case BUILT_IN_##MATHFN##F64: \ + return float64_type_node; \ + case BUILT_IN_##MATHFN##F128: \ + return float128_type_node; \ + case BUILT_IN_##MATHFN##F32X: \ + return float32x_type_node; \ + case BUILT_IN_##MATHFN##F64X: \ + return float64x_type_node; \ + case BUILT_IN_##MATHFN##F128X: \ + return float128x_type_node; + +/* Similar to above, but appends _R after any F/L suffix. */ +#define CASE_MATHFN_REENT(MATHFN) \ + case BUILT_IN_##MATHFN##_R: \ + return double_type_node; \ + case BUILT_IN_##MATHFN##F_R: \ + return float_type_node; \ + case BUILT_IN_##MATHFN##L_R: \ + return long_double_type_node; + + switch (fn) + { + /* Copied from mathfn_built_in_2, please keep in sync. */ + CASE_MATHFN (ACOS) + CASE_MATHFN (ACOSH) + CASE_MATHFN (ASIN) + CASE_MATHFN (ASINH) + CASE_MATHFN (ATAN) + CASE_MATHFN (ATAN2) + CASE_MATHFN (ATANH) + CASE_MATHFN (CBRT) + CASE_MATHFN_FLOATN (CEIL) + CASE_MATHFN (CEXPI) + CASE_MATHFN_FLOATN (COPYSIGN) + CASE_MATHFN (COS) + CASE_MATHFN (COSH) + CASE_MATHFN (DREM) + CASE_MATHFN (ERF) + CASE_MATHFN (ERFC) + CASE_MATHFN (EXP) + CASE_MATHFN (EXP10) + CASE_MATHFN (EXP2) + CASE_MATHFN (EXPM1) + CASE_MATHFN (FABS) + CASE_MATHFN (FDIM) + CASE_MATHFN_FLOATN (FLOOR) + CASE_MATHFN_FLOATN (FMA) + CASE_MATHFN_FLOATN (FMAX) + CASE_MATHFN_FLOATN (FMIN) + CASE_MATHFN (FMOD) + CASE_MATHFN (FREXP) + CASE_MATHFN (GAMMA) + CASE_MATHFN_REENT (GAMMA) /* GAMMA_R */ + CASE_MATHFN (HUGE_VAL) + CASE_MATHFN (HYPOT) + CASE_MATHFN (ILOGB) + CASE_MATHFN (ICEIL) + CASE_MATHFN (IFLOOR) + CASE_MATHFN (INF) + CASE_MATHFN (IRINT) + CASE_MATHFN (IROUND) + CASE_MATHFN (ISINF) + CASE_MATHFN (J0) + CASE_MATHFN (J1) + CASE_MATHFN (JN) + CASE_MATHFN (LCEIL) + CASE_MATHFN (LDEXP) + CASE_MATHFN (LFLOOR) + CASE_MATHFN (LGAMMA) + CASE_MATHFN_REENT (LGAMMA) /* LGAMMA_R */ + CASE_MATHFN (LLCEIL) + CASE_MATHFN (LLFLOOR) + CASE_MATHFN (LLRINT) + CASE_MATHFN (LLROUND) + CASE_MATHFN (LOG) + CASE_MATHFN (LOG10) + CASE_MATHFN (LOG1P) + CASE_MATHFN (LOG2) + CASE_MATHFN (LOGB) + CASE_MATHFN (LRINT) + CASE_MATHFN (LROUND) + CASE_MATHFN (MODF) + CASE_MATHFN (NAN) + CASE_MATHFN (NANS) + CASE_MATHFN_FLOATN (NEARBYINT) + CASE_MATHFN (NEXTAFTER) + CASE_MATHFN (NEXTTOWARD) + CASE_MATHFN (POW) + CASE_MATHFN (POWI) + CASE_MATHFN (POW10) + CASE_MATHFN (REMAINDER) + CASE_MATHFN (REMQUO) + CASE_MATHFN_FLOATN (RINT) + CASE_MATHFN_FLOATN (ROUND) + CASE_MATHFN_FLOATN (ROUNDEVEN) + CASE_MATHFN (SCALB) + CASE_MATHFN (SCALBLN) + CASE_MATHFN (SCALBN) + CASE_MATHFN (SIGNBIT) + CASE_MATHFN (SIGNIFICAND) + CASE_MATHFN (SIN) + CASE_MATHFN (SINCOS) + CASE_MATHFN (SINH) + CASE_MATHFN_FLOATN (SQRT) + CASE_MATHFN (TAN) + CASE_MATHFN (TANH) + CASE_MATHFN (TGAMMA) + CASE_MATHFN_FLOATN (TRUNC) + CASE_MATHFN (Y0) + CASE_MATHFN (Y1) + CASE_MATHFN (YN) + + default: + return NULL_TREE; + } + +#undef CASE_MATHFN +#undef CASE_MATHFN_FLOATN +#undef CASE_MATHFN_REENT +} + /* If BUILT_IN_NORMAL function FNDECL has an associated internal function, return its code, otherwise return IFN_LAST. Note that this function only tests whether the function is defined in internals.def, not whether diff --git a/gcc/builtins.h b/gcc/builtins.h index 504c618..72012a2 100644 --- a/gcc/builtins.h +++ b/gcc/builtins.h @@ -109,6 +109,7 @@ extern void expand_builtin_setjmp_receiver (rtx); extern void expand_builtin_update_setjmp_buf (rtx); extern tree mathfn_built_in (tree, enum built_in_function fn); extern tree mathfn_built_in (tree, combined_fn); +extern tree mathfn_built_in_type (combined_fn); extern rtx builtin_strncpy_read_str (void *, HOST_WIDE_INT, scalar_int_mode); extern rtx builtin_memset_read_str (void *, HOST_WIDE_INT, scalar_int_mode); extern rtx expand_builtin_saveregs (void); diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c index 4927255..7504569 100644 --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -1139,7 +1139,7 @@ execute_cse_sincos_1 (tree name) { gimple_stmt_iterator gsi; imm_use_iterator use_iter; - tree fndecl, res, type; + tree fndecl, res, type = NULL_TREE; gimple *def_stmt, *use_stmt, *stmt; int seen_cos = 0, seen_sin = 0, seen_cexpi = 0; auto_vec<gimple *> stmts; @@ -1147,7 +1147,6 @@ execute_cse_sincos_1 (tree name) int i; bool cfg_changed = false; - type = TREE_TYPE (name); FOR_EACH_IMM_USE_STMT (use_stmt, use_iter, name) { if (gimple_code (use_stmt) != GIMPLE_CALL @@ -1169,9 +1168,21 @@ execute_cse_sincos_1 (tree name) break; default:; + continue; } - } + tree t = mathfn_built_in_type (gimple_call_combined_fn (use_stmt)); + if (!type) + { + type = t; + t = TREE_TYPE (name); + } + /* This checks that NAME has the right type in the first round, + and, in subsequent rounds, that the built_in type is the same + type. */ + if (type != t && !tree_nop_conversion_p (type, t)) + return false; + } if (seen_cos + seen_sin + seen_cexpi <= 1) return false;
On Wed, Oct 7, 2020 at 7:15 PM Alexandre Oliva <oliva@adacore.com> wrote: > > On Oct 6, 2020, Richard Biener <richard.guenther@gmail.com> wrote: > > > So how about that mathfn_type helper instead of hard-wring this logic > > in sincos()? > > Like this? > > Regstrapped on x86_64-linux-gnu. Ok to install? OK with a minor nit, see below > I'm a little unhappy with the duplication of the CASE_MATHFN* sequence, > that ought to be kept in sync, , and considered turning that whole > sequence into a #define used in both places, but that would bloat the > patch further and make it less readable, so I figured I'd propose this > one first. Please let me know if you agree this additional change would > make it better. Yeah, I guess so. > take type from intrinsic in sincos pass > > From: Alexandre Oliva <oliva@adacore.com> > > This is a first step towards enabling the sincos optimization in Ada. > > The issue this patch solves is that sincos takes the type to be looked > up with mathfn_built_in from variables or temporaries passed as > arguments to SIN and COS intrinsics. In Ada, different float types > may be used but, despite their representation equivalence, their > distinctness causes the optimization to be skipped, because they are > not the types that mathfn_built_in expects. > > This patch introduces a function that maps intrinsics to the type > they're associated with, and uses that type, obtained from the > intrinsics used in calls to be optimized, to look up the correspoding > CEXPI intrinsic. > > For the sake of defensive programming, when using the type obtained > from the intrinsic, it now checks that, if different types are found > for the used argument, or for other calls that use it, that the types > are interchangeable. > > > for gcc/ChangeLog > > * builtins.c (mathfn_built_in_type): New. > * builtins.h (mathfn_built_in_type): Declare. > * tree-ssa-math-opts.c (execute_cse_sincos_1): Use it to > obtain the type expected by the intrinsic. > --- > gcc/builtins.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++ > gcc/builtins.h | 1 > gcc/tree-ssa-math-opts.c | 17 ++++- > 3 files changed, 162 insertions(+), 3 deletions(-) > > diff --git a/gcc/builtins.c b/gcc/builtins.c > index f91266e4..5649242 100644 > --- a/gcc/builtins.c > +++ b/gcc/builtins.c > @@ -2160,6 +2160,7 @@ mathfn_built_in_2 (tree type, combined_fn fn) > > switch (fn) > { > + /* Copied to mathfn_built_in_type, please keep in sync. */ > CASE_MATHFN (ACOS) > CASE_MATHFN (ACOSH) > CASE_MATHFN (ASIN) > @@ -2278,6 +2279,10 @@ mathfn_built_in_2 (tree type, combined_fn fn) > return END_BUILTINS; > } > > +#undef CASE_MATHFN > +#undef CASE_MATHFN_FLOATN > +#undef CASE_MATHFN_REENT > + > /* Return mathematic function equivalent to FN but operating directly on TYPE, > if available. If IMPLICIT_P is true use the implicit builtin declaration, > otherwise use the explicit declaration. If we can't do the conversion, > @@ -2313,6 +2318,148 @@ mathfn_built_in (tree type, enum built_in_function fn) > return mathfn_built_in_1 (type, as_combined_fn (fn), /*implicit=*/ 1); > } > > +/* Return the type associated with a built in function, i.e., the one > + to be passed to mathfn_built_in to get the type-specific > + function. */ > + > +tree > +mathfn_built_in_type (combined_fn fn) > +{ > +#define CASE_MATHFN(MATHFN) \ > + case BUILT_IN_##MATHFN: \ > + return double_type_node; \ > + case BUILT_IN_##MATHFN##F: \ > + return float_type_node; \ > + case BUILT_IN_##MATHFN##L: \ > + return long_double_type_node; > + > +#define CASE_MATHFN_FLOATN(MATHFN) \ > + CASE_MATHFN(MATHFN) \ > + case BUILT_IN_##MATHFN##F16: \ > + return float16_type_node; \ > + case BUILT_IN_##MATHFN##F32: \ > + return float32_type_node; \ > + case BUILT_IN_##MATHFN##F64: \ > + return float64_type_node; \ > + case BUILT_IN_##MATHFN##F128: \ > + return float128_type_node; \ > + case BUILT_IN_##MATHFN##F32X: \ > + return float32x_type_node; \ > + case BUILT_IN_##MATHFN##F64X: \ > + return float64x_type_node; \ > + case BUILT_IN_##MATHFN##F128X: \ > + return float128x_type_node; > + > +/* Similar to above, but appends _R after any F/L suffix. */ > +#define CASE_MATHFN_REENT(MATHFN) \ > + case BUILT_IN_##MATHFN##_R: \ > + return double_type_node; \ > + case BUILT_IN_##MATHFN##F_R: \ > + return float_type_node; \ > + case BUILT_IN_##MATHFN##L_R: \ > + return long_double_type_node; > + > + switch (fn) > + { > + /* Copied from mathfn_built_in_2, please keep in sync. */ > + CASE_MATHFN (ACOS) > + CASE_MATHFN (ACOSH) > + CASE_MATHFN (ASIN) > + CASE_MATHFN (ASINH) > + CASE_MATHFN (ATAN) > + CASE_MATHFN (ATAN2) > + CASE_MATHFN (ATANH) > + CASE_MATHFN (CBRT) > + CASE_MATHFN_FLOATN (CEIL) > + CASE_MATHFN (CEXPI) > + CASE_MATHFN_FLOATN (COPYSIGN) > + CASE_MATHFN (COS) > + CASE_MATHFN (COSH) > + CASE_MATHFN (DREM) > + CASE_MATHFN (ERF) > + CASE_MATHFN (ERFC) > + CASE_MATHFN (EXP) > + CASE_MATHFN (EXP10) > + CASE_MATHFN (EXP2) > + CASE_MATHFN (EXPM1) > + CASE_MATHFN (FABS) > + CASE_MATHFN (FDIM) > + CASE_MATHFN_FLOATN (FLOOR) > + CASE_MATHFN_FLOATN (FMA) > + CASE_MATHFN_FLOATN (FMAX) > + CASE_MATHFN_FLOATN (FMIN) > + CASE_MATHFN (FMOD) > + CASE_MATHFN (FREXP) > + CASE_MATHFN (GAMMA) > + CASE_MATHFN_REENT (GAMMA) /* GAMMA_R */ > + CASE_MATHFN (HUGE_VAL) > + CASE_MATHFN (HYPOT) > + CASE_MATHFN (ILOGB) > + CASE_MATHFN (ICEIL) > + CASE_MATHFN (IFLOOR) > + CASE_MATHFN (INF) > + CASE_MATHFN (IRINT) > + CASE_MATHFN (IROUND) > + CASE_MATHFN (ISINF) > + CASE_MATHFN (J0) > + CASE_MATHFN (J1) > + CASE_MATHFN (JN) > + CASE_MATHFN (LCEIL) > + CASE_MATHFN (LDEXP) > + CASE_MATHFN (LFLOOR) > + CASE_MATHFN (LGAMMA) > + CASE_MATHFN_REENT (LGAMMA) /* LGAMMA_R */ > + CASE_MATHFN (LLCEIL) > + CASE_MATHFN (LLFLOOR) > + CASE_MATHFN (LLRINT) > + CASE_MATHFN (LLROUND) > + CASE_MATHFN (LOG) > + CASE_MATHFN (LOG10) > + CASE_MATHFN (LOG1P) > + CASE_MATHFN (LOG2) > + CASE_MATHFN (LOGB) > + CASE_MATHFN (LRINT) > + CASE_MATHFN (LROUND) > + CASE_MATHFN (MODF) > + CASE_MATHFN (NAN) > + CASE_MATHFN (NANS) > + CASE_MATHFN_FLOATN (NEARBYINT) > + CASE_MATHFN (NEXTAFTER) > + CASE_MATHFN (NEXTTOWARD) > + CASE_MATHFN (POW) > + CASE_MATHFN (POWI) > + CASE_MATHFN (POW10) > + CASE_MATHFN (REMAINDER) > + CASE_MATHFN (REMQUO) > + CASE_MATHFN_FLOATN (RINT) > + CASE_MATHFN_FLOATN (ROUND) > + CASE_MATHFN_FLOATN (ROUNDEVEN) > + CASE_MATHFN (SCALB) > + CASE_MATHFN (SCALBLN) > + CASE_MATHFN (SCALBN) > + CASE_MATHFN (SIGNBIT) > + CASE_MATHFN (SIGNIFICAND) > + CASE_MATHFN (SIN) > + CASE_MATHFN (SINCOS) > + CASE_MATHFN (SINH) > + CASE_MATHFN_FLOATN (SQRT) > + CASE_MATHFN (TAN) > + CASE_MATHFN (TANH) > + CASE_MATHFN (TGAMMA) > + CASE_MATHFN_FLOATN (TRUNC) > + CASE_MATHFN (Y0) > + CASE_MATHFN (Y1) > + CASE_MATHFN (YN) > + > + default: > + return NULL_TREE; > + } > + > +#undef CASE_MATHFN > +#undef CASE_MATHFN_FLOATN > +#undef CASE_MATHFN_REENT > +} > + > /* If BUILT_IN_NORMAL function FNDECL has an associated internal function, > return its code, otherwise return IFN_LAST. Note that this function > only tests whether the function is defined in internals.def, not whether > diff --git a/gcc/builtins.h b/gcc/builtins.h > index 504c618..72012a2 100644 > --- a/gcc/builtins.h > +++ b/gcc/builtins.h > @@ -109,6 +109,7 @@ extern void expand_builtin_setjmp_receiver (rtx); > extern void expand_builtin_update_setjmp_buf (rtx); > extern tree mathfn_built_in (tree, enum built_in_function fn); > extern tree mathfn_built_in (tree, combined_fn); > +extern tree mathfn_built_in_type (combined_fn); > extern rtx builtin_strncpy_read_str (void *, HOST_WIDE_INT, scalar_int_mode); > extern rtx builtin_memset_read_str (void *, HOST_WIDE_INT, scalar_int_mode); > extern rtx expand_builtin_saveregs (void); > diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c > index 4927255..7504569 100644 > --- a/gcc/tree-ssa-math-opts.c > +++ b/gcc/tree-ssa-math-opts.c > @@ -1139,7 +1139,7 @@ execute_cse_sincos_1 (tree name) > { > gimple_stmt_iterator gsi; > imm_use_iterator use_iter; > - tree fndecl, res, type; > + tree fndecl, res, type = NULL_TREE; > gimple *def_stmt, *use_stmt, *stmt; > int seen_cos = 0, seen_sin = 0, seen_cexpi = 0; > auto_vec<gimple *> stmts; > @@ -1147,7 +1147,6 @@ execute_cse_sincos_1 (tree name) > int i; > bool cfg_changed = false; > > - type = TREE_TYPE (name); > FOR_EACH_IMM_USE_STMT (use_stmt, use_iter, name) > { > if (gimple_code (use_stmt) != GIMPLE_CALL > @@ -1169,9 +1168,21 @@ execute_cse_sincos_1 (tree name) > break; > > default:; > + continue; > } > - } > > + tree t = mathfn_built_in_type (gimple_call_combined_fn (use_stmt)); > + if (!type) > + { > + type = t; > + t = TREE_TYPE (name); > + } > + /* This checks that NAME has the right type in the first round, > + and, in subsequent rounds, that the built_in type is the same > + type. */ > + if (type != t && !tree_nop_conversion_p (type, t)) use !types_compatible_p (type, t) here if we don't want to go with the stricter type == t (both yours and types_compatible_p would treat long double and double the same in case they are mapped to the same FP mode). types_compatible_p is the appropriate GIMPLE predicate here. Thanks, Richard. > + return false; > + } > if (seen_cos + seen_sin + seen_cexpi <= 1) > return false; > > > > > -- > Alexandre Oliva, happy hacker > https://FSFLA.org/blogs/lxo/ > Free Software Activist > GNU Toolchain Engineer
On Oct 8, 2020, Richard Biener <richard.guenther@gmail.com> wrote: > OK with a minor nit, see below >> I'm a little unhappy with the duplication of the CASE_MATHFN* sequence, >> that ought to be kept in sync, , and considered turning that whole >> sequence into a #define used in both places, but that would bloat the >> patch further and make it less readable, so I figured I'd propose this >> one first. Please let me know if you agree this additional change would >> make it better. > Yeah, I guess so. Thanks, I folded that change into the patch. Bloated version below :-) > use !types_compatible_p (type, t) here if we don't want to go > with the stricter type == t (both yours and types_compatible_p > would treat long double and double the same in case they > are mapped to the same FP mode). types_compatible_p > is the appropriate GIMPLE predicate here. Thanks, fixed, regstrapped. Here's what I'm installing. take type from intrinsic in sincos pass From: Alexandre Oliva <oliva@adacore.com> This is a first step towards enabling the sincos optimization in Ada. The issue this patch solves is that sincos takes the type to be looked up with mathfn_built_in from variables or temporaries passed as arguments to SIN and COS intrinsics. In Ada, different float types may be used but, despite their representation equivalence, their distinctness causes the optimization to be skipped, because they are not the types that mathfn_built_in expects. This patch introduces a function that maps intrinsics to the type they're associated with, and uses that type, obtained from the intrinsics used in calls to be optimized, to look up the correspoding CEXPI intrinsic. For the sake of defensive programming, when using the type obtained from the intrinsic, it now checks that, if different types are found for the used argument, or for other calls that use it, that the types are interchangeable. for gcc/ChangeLog * builtins.c (mathfn_built_in_type): New. * builtins.h (mathfn_built_in_type): Declare. * tree-ssa-math-opts.c (execute_cse_sincos_1): Use it to obtain the type expected by the intrinsic. --- gcc/builtins.c | 236 +++++++++++++++++++++++++++++----------------- gcc/builtins.h | 1 gcc/tree-ssa-math-opts.c | 17 +++ 3 files changed, 164 insertions(+), 90 deletions(-) diff --git a/gcc/builtins.c b/gcc/builtins.c index f91266e4..284926f 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -2160,95 +2160,98 @@ mathfn_built_in_2 (tree type, combined_fn fn) switch (fn) { - CASE_MATHFN (ACOS) - CASE_MATHFN (ACOSH) - CASE_MATHFN (ASIN) - CASE_MATHFN (ASINH) - CASE_MATHFN (ATAN) - CASE_MATHFN (ATAN2) - CASE_MATHFN (ATANH) - CASE_MATHFN (CBRT) - CASE_MATHFN_FLOATN (CEIL) - CASE_MATHFN (CEXPI) - CASE_MATHFN_FLOATN (COPYSIGN) - CASE_MATHFN (COS) - CASE_MATHFN (COSH) - CASE_MATHFN (DREM) - CASE_MATHFN (ERF) - CASE_MATHFN (ERFC) - CASE_MATHFN (EXP) - CASE_MATHFN (EXP10) - CASE_MATHFN (EXP2) - CASE_MATHFN (EXPM1) - CASE_MATHFN (FABS) - CASE_MATHFN (FDIM) - CASE_MATHFN_FLOATN (FLOOR) - CASE_MATHFN_FLOATN (FMA) - CASE_MATHFN_FLOATN (FMAX) - CASE_MATHFN_FLOATN (FMIN) - CASE_MATHFN (FMOD) - CASE_MATHFN (FREXP) - CASE_MATHFN (GAMMA) - CASE_MATHFN_REENT (GAMMA) /* GAMMA_R */ - CASE_MATHFN (HUGE_VAL) - CASE_MATHFN (HYPOT) - CASE_MATHFN (ILOGB) - CASE_MATHFN (ICEIL) - CASE_MATHFN (IFLOOR) - CASE_MATHFN (INF) - CASE_MATHFN (IRINT) - CASE_MATHFN (IROUND) - CASE_MATHFN (ISINF) - CASE_MATHFN (J0) - CASE_MATHFN (J1) - CASE_MATHFN (JN) - CASE_MATHFN (LCEIL) - CASE_MATHFN (LDEXP) - CASE_MATHFN (LFLOOR) - CASE_MATHFN (LGAMMA) - CASE_MATHFN_REENT (LGAMMA) /* LGAMMA_R */ - CASE_MATHFN (LLCEIL) - CASE_MATHFN (LLFLOOR) - CASE_MATHFN (LLRINT) - CASE_MATHFN (LLROUND) - CASE_MATHFN (LOG) - CASE_MATHFN (LOG10) - CASE_MATHFN (LOG1P) - CASE_MATHFN (LOG2) - CASE_MATHFN (LOGB) - CASE_MATHFN (LRINT) - CASE_MATHFN (LROUND) - CASE_MATHFN (MODF) - CASE_MATHFN (NAN) - CASE_MATHFN (NANS) - CASE_MATHFN_FLOATN (NEARBYINT) - CASE_MATHFN (NEXTAFTER) - CASE_MATHFN (NEXTTOWARD) - CASE_MATHFN (POW) - CASE_MATHFN (POWI) - CASE_MATHFN (POW10) - CASE_MATHFN (REMAINDER) - CASE_MATHFN (REMQUO) - CASE_MATHFN_FLOATN (RINT) - CASE_MATHFN_FLOATN (ROUND) - CASE_MATHFN_FLOATN (ROUNDEVEN) - CASE_MATHFN (SCALB) - CASE_MATHFN (SCALBLN) - CASE_MATHFN (SCALBN) - CASE_MATHFN (SIGNBIT) - CASE_MATHFN (SIGNIFICAND) - CASE_MATHFN (SIN) - CASE_MATHFN (SINCOS) - CASE_MATHFN (SINH) - CASE_MATHFN_FLOATN (SQRT) - CASE_MATHFN (TAN) - CASE_MATHFN (TANH) - CASE_MATHFN (TGAMMA) - CASE_MATHFN_FLOATN (TRUNC) - CASE_MATHFN (Y0) - CASE_MATHFN (Y1) +#define SEQ_OF_CASE_MATHFN \ + CASE_MATHFN (ACOS) \ + CASE_MATHFN (ACOSH) \ + CASE_MATHFN (ASIN) \ + CASE_MATHFN (ASINH) \ + CASE_MATHFN (ATAN) \ + CASE_MATHFN (ATAN2) \ + CASE_MATHFN (ATANH) \ + CASE_MATHFN (CBRT) \ + CASE_MATHFN_FLOATN (CEIL) \ + CASE_MATHFN (CEXPI) \ + CASE_MATHFN_FLOATN (COPYSIGN) \ + CASE_MATHFN (COS) \ + CASE_MATHFN (COSH) \ + CASE_MATHFN (DREM) \ + CASE_MATHFN (ERF) \ + CASE_MATHFN (ERFC) \ + CASE_MATHFN (EXP) \ + CASE_MATHFN (EXP10) \ + CASE_MATHFN (EXP2) \ + CASE_MATHFN (EXPM1) \ + CASE_MATHFN (FABS) \ + CASE_MATHFN (FDIM) \ + CASE_MATHFN_FLOATN (FLOOR) \ + CASE_MATHFN_FLOATN (FMA) \ + CASE_MATHFN_FLOATN (FMAX) \ + CASE_MATHFN_FLOATN (FMIN) \ + CASE_MATHFN (FMOD) \ + CASE_MATHFN (FREXP) \ + CASE_MATHFN (GAMMA) \ + CASE_MATHFN_REENT (GAMMA) /* GAMMA_R */ \ + CASE_MATHFN (HUGE_VAL) \ + CASE_MATHFN (HYPOT) \ + CASE_MATHFN (ILOGB) \ + CASE_MATHFN (ICEIL) \ + CASE_MATHFN (IFLOOR) \ + CASE_MATHFN (INF) \ + CASE_MATHFN (IRINT) \ + CASE_MATHFN (IROUND) \ + CASE_MATHFN (ISINF) \ + CASE_MATHFN (J0) \ + CASE_MATHFN (J1) \ + CASE_MATHFN (JN) \ + CASE_MATHFN (LCEIL) \ + CASE_MATHFN (LDEXP) \ + CASE_MATHFN (LFLOOR) \ + CASE_MATHFN (LGAMMA) \ + CASE_MATHFN_REENT (LGAMMA) /* LGAMMA_R */ \ + CASE_MATHFN (LLCEIL) \ + CASE_MATHFN (LLFLOOR) \ + CASE_MATHFN (LLRINT) \ + CASE_MATHFN (LLROUND) \ + CASE_MATHFN (LOG) \ + CASE_MATHFN (LOG10) \ + CASE_MATHFN (LOG1P) \ + CASE_MATHFN (LOG2) \ + CASE_MATHFN (LOGB) \ + CASE_MATHFN (LRINT) \ + CASE_MATHFN (LROUND) \ + CASE_MATHFN (MODF) \ + CASE_MATHFN (NAN) \ + CASE_MATHFN (NANS) \ + CASE_MATHFN_FLOATN (NEARBYINT) \ + CASE_MATHFN (NEXTAFTER) \ + CASE_MATHFN (NEXTTOWARD) \ + CASE_MATHFN (POW) \ + CASE_MATHFN (POWI) \ + CASE_MATHFN (POW10) \ + CASE_MATHFN (REMAINDER) \ + CASE_MATHFN (REMQUO) \ + CASE_MATHFN_FLOATN (RINT) \ + CASE_MATHFN_FLOATN (ROUND) \ + CASE_MATHFN_FLOATN (ROUNDEVEN) \ + CASE_MATHFN (SCALB) \ + CASE_MATHFN (SCALBLN) \ + CASE_MATHFN (SCALBN) \ + CASE_MATHFN (SIGNBIT) \ + CASE_MATHFN (SIGNIFICAND) \ + CASE_MATHFN (SIN) \ + CASE_MATHFN (SINCOS) \ + CASE_MATHFN (SINH) \ + CASE_MATHFN_FLOATN (SQRT) \ + CASE_MATHFN (TAN) \ + CASE_MATHFN (TANH) \ + CASE_MATHFN (TGAMMA) \ + CASE_MATHFN_FLOATN (TRUNC) \ + CASE_MATHFN (Y0) \ + CASE_MATHFN (Y1) \ CASE_MATHFN (YN) + SEQ_OF_CASE_MATHFN + default: return END_BUILTINS; } @@ -2278,6 +2281,10 @@ mathfn_built_in_2 (tree type, combined_fn fn) return END_BUILTINS; } +#undef CASE_MATHFN +#undef CASE_MATHFN_FLOATN +#undef CASE_MATHFN_REENT + /* Return mathematic function equivalent to FN but operating directly on TYPE, if available. If IMPLICIT_P is true use the implicit builtin declaration, otherwise use the explicit declaration. If we can't do the conversion, @@ -2313,6 +2320,61 @@ mathfn_built_in (tree type, enum built_in_function fn) return mathfn_built_in_1 (type, as_combined_fn (fn), /*implicit=*/ 1); } +/* Return the type associated with a built in function, i.e., the one + to be passed to mathfn_built_in to get the type-specific + function. */ + +tree +mathfn_built_in_type (combined_fn fn) +{ +#define CASE_MATHFN(MATHFN) \ + case BUILT_IN_##MATHFN: \ + return double_type_node; \ + case BUILT_IN_##MATHFN##F: \ + return float_type_node; \ + case BUILT_IN_##MATHFN##L: \ + return long_double_type_node; + +#define CASE_MATHFN_FLOATN(MATHFN) \ + CASE_MATHFN(MATHFN) \ + case BUILT_IN_##MATHFN##F16: \ + return float16_type_node; \ + case BUILT_IN_##MATHFN##F32: \ + return float32_type_node; \ + case BUILT_IN_##MATHFN##F64: \ + return float64_type_node; \ + case BUILT_IN_##MATHFN##F128: \ + return float128_type_node; \ + case BUILT_IN_##MATHFN##F32X: \ + return float32x_type_node; \ + case BUILT_IN_##MATHFN##F64X: \ + return float64x_type_node; \ + case BUILT_IN_##MATHFN##F128X: \ + return float128x_type_node; + +/* Similar to above, but appends _R after any F/L suffix. */ +#define CASE_MATHFN_REENT(MATHFN) \ + case BUILT_IN_##MATHFN##_R: \ + return double_type_node; \ + case BUILT_IN_##MATHFN##F_R: \ + return float_type_node; \ + case BUILT_IN_##MATHFN##L_R: \ + return long_double_type_node; + + switch (fn) + { + SEQ_OF_CASE_MATHFN + + default: + return NULL_TREE; + } + +#undef CASE_MATHFN +#undef CASE_MATHFN_FLOATN +#undef CASE_MATHFN_REENT +#undef SEQ_OF_CASE_MATHFN +} + /* If BUILT_IN_NORMAL function FNDECL has an associated internal function, return its code, otherwise return IFN_LAST. Note that this function only tests whether the function is defined in internals.def, not whether diff --git a/gcc/builtins.h b/gcc/builtins.h index 504c618..72012a2 100644 --- a/gcc/builtins.h +++ b/gcc/builtins.h @@ -109,6 +109,7 @@ extern void expand_builtin_setjmp_receiver (rtx); extern void expand_builtin_update_setjmp_buf (rtx); extern tree mathfn_built_in (tree, enum built_in_function fn); extern tree mathfn_built_in (tree, combined_fn); +extern tree mathfn_built_in_type (combined_fn); extern rtx builtin_strncpy_read_str (void *, HOST_WIDE_INT, scalar_int_mode); extern rtx builtin_memset_read_str (void *, HOST_WIDE_INT, scalar_int_mode); extern rtx expand_builtin_saveregs (void); diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c index 4927255..90dfb98 100644 --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -1139,7 +1139,7 @@ execute_cse_sincos_1 (tree name) { gimple_stmt_iterator gsi; imm_use_iterator use_iter; - tree fndecl, res, type; + tree fndecl, res, type = NULL_TREE; gimple *def_stmt, *use_stmt, *stmt; int seen_cos = 0, seen_sin = 0, seen_cexpi = 0; auto_vec<gimple *> stmts; @@ -1147,7 +1147,6 @@ execute_cse_sincos_1 (tree name) int i; bool cfg_changed = false; - type = TREE_TYPE (name); FOR_EACH_IMM_USE_STMT (use_stmt, use_iter, name) { if (gimple_code (use_stmt) != GIMPLE_CALL @@ -1169,9 +1168,21 @@ execute_cse_sincos_1 (tree name) break; default:; + continue; } - } + tree t = mathfn_built_in_type (gimple_call_combined_fn (use_stmt)); + if (!type) + { + type = t; + t = TREE_TYPE (name); + } + /* This checks that NAME has the right type in the first round, + and, in subsequent rounds, that the built_in type is the same + type, or a compatible type. */ + if (type != t && !types_compatible_p (type, t)) + return false; + } if (seen_cos + seen_sin + seen_cexpi <= 1) return false;
Hello Alexander.
It seems the patch caused quite some clang warnings:
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-clang/build/gcc/builtins.c:2366:5: warning: comparison of different enumeration types in switch statement ('combined_fn' and 'built_in_function') [-Wenum-compare-switch]
...
Can we please fix them?
Thanks,
Martin
Hello, Martin, On Oct 12, 2020, Martin Liška <mliska@suse.cz> wrote: > It seems the patch caused quite some clang warnings: > /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-clang/build/gcc/builtins.c:2366:5: > warning: comparison of different enumeration types in switch statement > ('combined_fn' and 'built_in_function') [-Wenum-compare-switch] > ... Thanks for the report. > Can we please fix them? Here's the patch I'm just about to begin regstrapping. I'll check it in, as obvious, once I'm done. Thanks again, mathfn_built_in_type case type fix Martin Liška reported warnings about type mismatches in the cases in the recently-introduced mathfn_built_in_type. This patch adjusts the macros to use the combined_fn enumerators rather than the (currently same-numbered) built_in_function ones. for gcc/ChangeLog * builtins.c (mathfn_built_in_type): Use CFN_ enumerators. --- gcc/builtins.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/gcc/builtins.c b/gcc/builtins.c index 3a77da2..3f799e5 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -2451,37 +2451,37 @@ tree mathfn_built_in_type (combined_fn fn) { #define CASE_MATHFN(MATHFN) \ - case BUILT_IN_##MATHFN: \ + case CFN_BUILT_IN_##MATHFN: \ return double_type_node; \ - case BUILT_IN_##MATHFN##F: \ + case CFN_BUILT_IN_##MATHFN##F: \ return float_type_node; \ - case BUILT_IN_##MATHFN##L: \ + case CFN_BUILT_IN_##MATHFN##L: \ return long_double_type_node; #define CASE_MATHFN_FLOATN(MATHFN) \ CASE_MATHFN(MATHFN) \ - case BUILT_IN_##MATHFN##F16: \ + case CFN_BUILT_IN_##MATHFN##F16: \ return float16_type_node; \ - case BUILT_IN_##MATHFN##F32: \ + case CFN_BUILT_IN_##MATHFN##F32: \ return float32_type_node; \ - case BUILT_IN_##MATHFN##F64: \ + case CFN_BUILT_IN_##MATHFN##F64: \ return float64_type_node; \ - case BUILT_IN_##MATHFN##F128: \ + case CFN_BUILT_IN_##MATHFN##F128: \ return float128_type_node; \ - case BUILT_IN_##MATHFN##F32X: \ + case CFN_BUILT_IN_##MATHFN##F32X: \ return float32x_type_node; \ - case BUILT_IN_##MATHFN##F64X: \ + case CFN_BUILT_IN_##MATHFN##F64X: \ return float64x_type_node; \ - case BUILT_IN_##MATHFN##F128X: \ + case CFN_BUILT_IN_##MATHFN##F128X: \ return float128x_type_node; /* Similar to above, but appends _R after any F/L suffix. */ #define CASE_MATHFN_REENT(MATHFN) \ - case BUILT_IN_##MATHFN##_R: \ + case CFN_BUILT_IN_##MATHFN##_R: \ return double_type_node; \ - case BUILT_IN_##MATHFN##F_R: \ + case CFN_BUILT_IN_##MATHFN##F_R: \ return float_type_node; \ - case BUILT_IN_##MATHFN##L_R: \ + case CFN_BUILT_IN_##MATHFN##L_R: \ return long_double_type_node; switch (fn)
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c index 8423caa..31fd241 100644 --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -1139,7 +1139,7 @@ execute_cse_sincos_1 (tree name) { gimple_stmt_iterator gsi; imm_use_iterator use_iter; - tree fndecl, res, type; + tree fndecl = NULL_TREE, res, type = NULL_TREE; gimple *def_stmt, *use_stmt, *stmt; int seen_cos = 0, seen_sin = 0, seen_cexpi = 0; auto_vec<gimple *> stmts; @@ -1147,7 +1147,6 @@ execute_cse_sincos_1 (tree name) int i; bool cfg_changed = false; - type = TREE_TYPE (name); FOR_EACH_IMM_USE_STMT (use_stmt, use_iter, name) { if (gimple_code (use_stmt) != GIMPLE_CALL @@ -1169,15 +1168,34 @@ execute_cse_sincos_1 (tree name) break; default:; + continue; } - } + tree t = TREE_VALUE (TYPE_ARG_TYPES (gimple_call_fntype (use_stmt))); + if (!type) + type = t; + else if (t != type) + { + if (!tree_nop_conversion_p (type, t)) + return false; + /* If there is more than one type to choose from, prefer one + that has a CEXPI builtin. */ + else if (!fndecl + && (fndecl = mathfn_built_in (t, BUILT_IN_CEXPI))) + type = t; + } + } if (seen_cos + seen_sin + seen_cexpi <= 1) return false; + if (type != TREE_TYPE (name) + && !tree_nop_conversion_p (type, TREE_TYPE (name))) + return false; + /* Simply insert cexpi at the beginning of top_bb but not earlier than the name def statement. */ - fndecl = mathfn_built_in (type, BUILT_IN_CEXPI); + if (!fndecl) + fndecl = mathfn_built_in (type, BUILT_IN_CEXPI); if (!fndecl) return false; stmt = gimple_build_call (fndecl, 1, name);