| Message ID | 87egfznnxc.fsf@e105548-lin.cambridge.arm.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Nov 9, 2015 at 5:21 PM, Richard Sandiford <richard.sandiford@arm.com> wrote: > In practice all targets that can vectorise sqrt define the appropriate > sqrt<mode>2 optab. The only case where this isn't immediately obvious > is the libmass support in rs6000.c, but Mike Meissner said that it shouldn't > be exercised for sqrt. > > This patch therefore uses the internal function interface instead of > going via the target hook. > > > gcc/ > * tree-vect-patterns.c: Include internal-fn.h. > (vect_recog_pow_pattern): Use IFN_SQRT instead of BUILT_IN_SQRT*. > > diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c > index bab9a4f..a803e8c 100644 > --- a/gcc/tree-vect-patterns.c > +++ b/gcc/tree-vect-patterns.c > @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see > #include "tree-vectorizer.h" > #include "dumpfile.h" > #include "builtins.h" > +#include "internal-fn.h" > #include "case-cfn-macros.h" > > /* Pattern recognition functions */ > @@ -1052,18 +1053,13 @@ vect_recog_pow_pattern (vec<gimple *> *stmts, tree *type_in, > if (TREE_CODE (exp) == REAL_CST > && real_equal (&TREE_REAL_CST (exp), &dconsthalf)) > { > - tree newfn = mathfn_built_in (TREE_TYPE (base), BUILT_IN_SQRT); > *type_in = get_vectype_for_scalar_type (TREE_TYPE (base)); > - if (*type_in) > + if (*type_in && direct_internal_fn_supported_p (IFN_SQRT, *type_in)) > { > - gcall *stmt = gimple_build_call (newfn, 1, base); > - if (vectorizable_function (stmt, *type_in, *type_in) > - != NULL_TREE) > - { > - var = vect_recog_temp_ssa_var (TREE_TYPE (base), stmt); > - gimple_call_set_lhs (stmt, var); > - return stmt; > - } > + gcall *stmt = gimple_build_call_internal (IFN_SQRT, 1, base); > + var = vect_recog_temp_ssa_var (TREE_TYPE (base), stmt); > + gimple_call_set_lhs (stmt, var); > + return stmt; Looks ok but I wonder if this is dead code with (for pows (POW) sqrts (SQRT) cbrts (CBRT) (simplify (pows @0 REAL_CST@1) (with { const REAL_VALUE_TYPE *value = TREE_REAL_CST_PTR (@1); REAL_VALUE_TYPE tmp; } (switch ... /* pow(x,0.5) -> sqrt(x). */ (if (flag_unsafe_math_optimizations && canonicalize_math_p () && real_equal (value, &dconsthalf)) (sqrts @0)) also wondering here about canonicalize_math_p (), I'd expected the reverse transform as canonicalization. Also wondering about flag_unsafe_math_optimizations (missing from the vectorizer pattern). Anyway, patch is ok. Thanks, Richard. > } > } > >
Richard Biener <richard.guenther@gmail.com> writes: > On Mon, Nov 9, 2015 at 5:21 PM, Richard Sandiford > <richard.sandiford@arm.com> wrote: >> In practice all targets that can vectorise sqrt define the appropriate >> sqrt<mode>2 optab. The only case where this isn't immediately obvious >> is the libmass support in rs6000.c, but Mike Meissner said that it shouldn't >> be exercised for sqrt. >> >> This patch therefore uses the internal function interface instead of >> going via the target hook. >> >> >> gcc/ >> * tree-vect-patterns.c: Include internal-fn.h. >> (vect_recog_pow_pattern): Use IFN_SQRT instead of BUILT_IN_SQRT*. >> >> diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c >> index bab9a4f..a803e8c 100644 >> --- a/gcc/tree-vect-patterns.c >> +++ b/gcc/tree-vect-patterns.c >> @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see >> #include "tree-vectorizer.h" >> #include "dumpfile.h" >> #include "builtins.h" >> +#include "internal-fn.h" >> #include "case-cfn-macros.h" >> >> /* Pattern recognition functions */ >> @@ -1052,18 +1053,13 @@ vect_recog_pow_pattern (vec<gimple *> *stmts, tree *type_in, >> if (TREE_CODE (exp) == REAL_CST >> && real_equal (&TREE_REAL_CST (exp), &dconsthalf)) >> { >> - tree newfn = mathfn_built_in (TREE_TYPE (base), BUILT_IN_SQRT); >> *type_in = get_vectype_for_scalar_type (TREE_TYPE (base)); >> - if (*type_in) >> + if (*type_in && direct_internal_fn_supported_p (IFN_SQRT, *type_in)) >> { >> - gcall *stmt = gimple_build_call (newfn, 1, base); >> - if (vectorizable_function (stmt, *type_in, *type_in) >> - != NULL_TREE) >> - { >> - var = vect_recog_temp_ssa_var (TREE_TYPE (base), stmt); >> - gimple_call_set_lhs (stmt, var); >> - return stmt; >> - } >> + gcall *stmt = gimple_build_call_internal (IFN_SQRT, 1, base); >> + var = vect_recog_temp_ssa_var (TREE_TYPE (base), stmt); >> + gimple_call_set_lhs (stmt, var); >> + return stmt; > > Looks ok but I wonder if this is dead code with > > (for pows (POW) > sqrts (SQRT) > cbrts (CBRT) > (simplify > (pows @0 REAL_CST@1) > (with { > const REAL_VALUE_TYPE *value = TREE_REAL_CST_PTR (@1); > REAL_VALUE_TYPE tmp; > } > (switch > ... > /* pow(x,0.5) -> sqrt(x). */ > (if (flag_unsafe_math_optimizations > && canonicalize_math_p () > && real_equal (value, &dconsthalf)) > (sqrts @0)) Yeah, I wondered that too, although I think it's more likely to be dead because of sincos. In the end it just seemed like a rabiit hole too far though. > Anyway, patch is ok. Thanks, Richard
On Tue, Nov 10, 2015 at 11:57 AM, Richard Sandiford <richard.sandiford@arm.com> wrote: > Richard Biener <richard.guenther@gmail.com> writes: >> On Mon, Nov 9, 2015 at 5:21 PM, Richard Sandiford >> <richard.sandiford@arm.com> wrote: >>> In practice all targets that can vectorise sqrt define the appropriate >>> sqrt<mode>2 optab. The only case where this isn't immediately obvious >>> is the libmass support in rs6000.c, but Mike Meissner said that it shouldn't >>> be exercised for sqrt. >>> >>> This patch therefore uses the internal function interface instead of >>> going via the target hook. >>> >>> >>> gcc/ >>> * tree-vect-patterns.c: Include internal-fn.h. >>> (vect_recog_pow_pattern): Use IFN_SQRT instead of BUILT_IN_SQRT*. >>> >>> diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c >>> index bab9a4f..a803e8c 100644 >>> --- a/gcc/tree-vect-patterns.c >>> +++ b/gcc/tree-vect-patterns.c >>> @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see >>> #include "tree-vectorizer.h" >>> #include "dumpfile.h" >>> #include "builtins.h" >>> +#include "internal-fn.h" >>> #include "case-cfn-macros.h" >>> >>> /* Pattern recognition functions */ >>> @@ -1052,18 +1053,13 @@ vect_recog_pow_pattern (vec<gimple *> *stmts, tree *type_in, >>> if (TREE_CODE (exp) == REAL_CST >>> && real_equal (&TREE_REAL_CST (exp), &dconsthalf)) >>> { >>> - tree newfn = mathfn_built_in (TREE_TYPE (base), BUILT_IN_SQRT); >>> *type_in = get_vectype_for_scalar_type (TREE_TYPE (base)); >>> - if (*type_in) >>> + if (*type_in && direct_internal_fn_supported_p (IFN_SQRT, *type_in)) >>> { >>> - gcall *stmt = gimple_build_call (newfn, 1, base); >>> - if (vectorizable_function (stmt, *type_in, *type_in) >>> - != NULL_TREE) >>> - { >>> - var = vect_recog_temp_ssa_var (TREE_TYPE (base), stmt); >>> - gimple_call_set_lhs (stmt, var); >>> - return stmt; >>> - } >>> + gcall *stmt = gimple_build_call_internal (IFN_SQRT, 1, base); >>> + var = vect_recog_temp_ssa_var (TREE_TYPE (base), stmt); >>> + gimple_call_set_lhs (stmt, var); >>> + return stmt; >> >> Looks ok but I wonder if this is dead code with >> >> (for pows (POW) >> sqrts (SQRT) >> cbrts (CBRT) >> (simplify >> (pows @0 REAL_CST@1) >> (with { >> const REAL_VALUE_TYPE *value = TREE_REAL_CST_PTR (@1); >> REAL_VALUE_TYPE tmp; >> } >> (switch >> ... >> /* pow(x,0.5) -> sqrt(x). */ >> (if (flag_unsafe_math_optimizations >> && canonicalize_math_p () >> && real_equal (value, &dconsthalf)) >> (sqrts @0)) > > Yeah, I wondered that too, although I think it's more likely to be dead > because of sincos. In the end it just seemed like a rabiit hole too far > though. Indeed. sincos should also fix the pow(x, 2.) case handled. Richard. >> Anyway, patch is ok. > > Thanks, > Richard >
On Tue, 10 Nov 2015, Richard Biener wrote: > Looks ok but I wonder if this is dead code with > > (for pows (POW) > sqrts (SQRT) > cbrts (CBRT) > (simplify > (pows @0 REAL_CST@1) > (with { > const REAL_VALUE_TYPE *value = TREE_REAL_CST_PTR (@1); > REAL_VALUE_TYPE tmp; > } > (switch > ... > /* pow(x,0.5) -> sqrt(x). */ > (if (flag_unsafe_math_optimizations > && canonicalize_math_p () > && real_equal (value, &dconsthalf)) > (sqrts @0)) > > also wondering here about canonicalize_math_p (), I'd expected the > reverse transform as canonicalization. Also wondering about > flag_unsafe_math_optimizations (missing from the vectorizer pattern). pow(x,0.5) -> sqrt(x) is unsafe because: pow (-0, 0.5) is specified in Annex F to be +0 but sqrt (-0) is -0; pow (-Inf, 0.5) is specified in Annex F to be +Inf, but sqrt (-Inf) is NaN with "invalid" exception raised. I think it's safe in other cases (the reverse of course is not safe, sqrt is a fully-specified correctly-rounded IEEE operation and pow isn't).
On November 10, 2015 6:29:36 PM GMT+01:00, Joseph Myers <joseph@codesourcery.com> wrote: >On Tue, 10 Nov 2015, Richard Biener wrote: > >> Looks ok but I wonder if this is dead code with >> >> (for pows (POW) >> sqrts (SQRT) >> cbrts (CBRT) >> (simplify >> (pows @0 REAL_CST@1) >> (with { >> const REAL_VALUE_TYPE *value = TREE_REAL_CST_PTR (@1); >> REAL_VALUE_TYPE tmp; >> } >> (switch >> ... >> /* pow(x,0.5) -> sqrt(x). */ >> (if (flag_unsafe_math_optimizations >> && canonicalize_math_p () >> && real_equal (value, &dconsthalf)) >> (sqrts @0)) >> >> also wondering here about canonicalize_math_p (), I'd expected the >> reverse transform as canonicalization. Also wondering about >> flag_unsafe_math_optimizations (missing from the vectorizer pattern). > >pow(x,0.5) -> sqrt(x) is unsafe because: pow (-0, 0.5) is specified in >Annex F to be +0 but sqrt (-0) is -0; pow (-Inf, 0.5) is specified in >Annex F to be +Inf, but sqrt (-Inf) is NaN with "invalid" exception >raised. I think it's safe in other cases So it's safe with no signed zeros and finite math rather than unsafe. The reverse would be unsafe in addition (not fully specified and rounded). (the reverse of course is not > >safe, sqrt is a fully-specified correctly-rounded IEEE operation and >pow >isn't).
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c index bab9a4f..a803e8c 100644 --- a/gcc/tree-vect-patterns.c +++ b/gcc/tree-vect-patterns.c @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see #include "tree-vectorizer.h" #include "dumpfile.h" #include "builtins.h" +#include "internal-fn.h" #include "case-cfn-macros.h" /* Pattern recognition functions */ @@ -1052,18 +1053,13 @@ vect_recog_pow_pattern (vec<gimple *> *stmts, tree *type_in, if (TREE_CODE (exp) == REAL_CST && real_equal (&TREE_REAL_CST (exp), &dconsthalf)) { - tree newfn = mathfn_built_in (TREE_TYPE (base), BUILT_IN_SQRT); *type_in = get_vectype_for_scalar_type (TREE_TYPE (base)); - if (*type_in) + if (*type_in && direct_internal_fn_supported_p (IFN_SQRT, *type_in)) { - gcall *stmt = gimple_build_call (newfn, 1, base); - if (vectorizable_function (stmt, *type_in, *type_in) - != NULL_TREE) - { - var = vect_recog_temp_ssa_var (TREE_TYPE (base), stmt); - gimple_call_set_lhs (stmt, var); - return stmt; - } + gcall *stmt = gimple_build_call_internal (IFN_SQRT, 1, base); + var = vect_recog_temp_ssa_var (TREE_TYPE (base), stmt); + gimple_call_set_lhs (stmt, var); + return stmt; } }