On Tue, 26 Jul 2016, Richard Sandiford wrote:
> Richard Biener <rguent...@suse.de> writes:
> > On Mon, 25 Jul 2016, Prathamesh Kulkarni wrote:
> >
> >> Hi,
> >> The attached patch tries to fix PR71078.
> >> I am not sure if I have got the converts right.
> >> I put (convert? @0) and (convert1? (abs @1))
> >> to match for cases when operands's types may
> >> be different from outermost type like in pr71078-3.c
> >
> > Types of RDIV_EXPR have to be the same so as you have a
> > match on @0 the converts need to be either both present
> > or not present.
> >
> > + (if (FLOAT_TYPE_P (type)
> >
> > as you special-case several types below please use SCALAR_FLOAT_TYPE_P
> > here.
> >
> > + && ! HONOR_NANS (type)
> > + && ! HONOR_INFINITIES (type))
> > + (switch
> > + (if (type == float_type_node)
> > + (BUILT_IN_COPYSIGNF { build_one_cst (type); } (convert @0)))
> >
> > please use if (types_match (type, float_type_node)) instead of
> > pointer equality.
>
> IMO it'd be better to have some way of using mathfn_builtin from match.pd.
Yeah ... but that is not supported. I suppose we could "easily"
allow BUILT_IN_COPYSIGN for all types when the generator inserts
a mathfn_built_in_2 call unconditionally for fn-kind result ops.
Of course we'd need to deal with it failing and it would be
redundant work for most of the existing patterns. See patch below
which should allow
(BUILT_IN_COPYSIGN { build_cone_cst (type); } (conver @0))
for all types. Well. We probably should check for END_BUILTINS
as return value and fail pattern generation late here...
> > I _think_ you can do better here by using
> > IFN_COPYSIGN but possibly only so if the target supports it.
> > Richard - this seems to be the first pattern in need of
> > generating a builtin where no other was there to match the type
> > to - any idea how we can safely use the internal function here?
>
> I don't think there's any benefit to using the internal function here.
The advantage is solely that the internal function is "overloaded"
vs the builtin having variants for all the float types that are
supported.
> At the moment we only use internal functions before cfgexpand if they
> let us do something that we couldn't do otherwise, such as vectorising a
> function or avoiding unnecessary effects on errno. All other cases are
> handled by cfgexpand itself.
>
> > I see those do not have an expander that would fall back to
> > expanding the regular builtin, correct?
>
> Right. In:
>
> https://gcc.gnu.org/ml/gcc-patches/2015-08/msg01057.html
>
> you weren't keen on the idea of tree codes introducing new dependencies
> on libm, so I carried that priniple over to the internal functions
> (which are really just extended tree codes). I suppose copysign is
> a special case since we can always open code it, but in general we
> shouldn't fall back to something that could generate a call.
Richard.
Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c (revision 238743)
+++ gcc/builtins.c (working copy)
@@ -1767,7 +1767,7 @@ expand_builtin_classify_type (tree exp)
This is purely an operation on function codes; it does not guarantee
that the target actually has an implementation of the function. */
-static built_in_function
+built_in_function
mathfn_built_in_2 (tree type, combined_fn fn)
{
built_in_function fcode, fcodef, fcodel;
Index: gcc/builtins.h
===================================================================
--- gcc/builtins.h (revision 238743)
+++ gcc/builtins.h (working copy)
@@ -61,6 +61,7 @@ extern tree c_strlen (tree, int);
extern void expand_builtin_setjmp_setup (rtx, rtx);
extern void expand_builtin_setjmp_receiver (rtx);
extern void expand_builtin_update_setjmp_buf (rtx);
+extern built_in_function mathfn_built_in_2 (tree, combined_fn);
extern tree mathfn_built_in (tree, enum built_in_function fn);
extern tree mathfn_built_in (tree, combined_fn);
extern rtx builtin_strncpy_read_str (void *, HOST_WIDE_INT, machine_mode);
Index: gcc/genmatch.c
===================================================================
--- gcc/genmatch.c (revision 238743)
+++ gcc/genmatch.c (working copy)
@@ -2333,6 +2333,14 @@ expr::gen_transform (FILE *f, int indent
else
opr_name = operation->id;
+ if (operation->kind == id_base::FN)
+ {
+ fprintf_indent (f, indent,
+ "combined_fn tem_fcode = as_combined_fn ("
+ "mathfn_built_in_2 (%s, %s));\n", type, opr_name);
+ opr_name = "tem_fcode";
+ }
+
if (gimple)
{
if (*opr == CONVERT_EXPR)
