This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC PATCH] Convert pow(10.0,x) -> exp10(x), etc...


On Fri, 15 Apr 2005, Uros Bizjak wrote:
> 1.) The definition of BUILT_IN_EXP10 is defined as DEF_EXT_LIB_BUILTIN,
> and because of this, mathfn_built_in() always returns a NULL_TREE that
> triggers gcc_assert:

Hmm.  The fact that exp10 is a GNU/glibc extension complicates things.
The middle and backends will need more infrastructure to indicate whether
this/these functions are available.

In the meantime, however, I think a compromise is to soften the failure
modes of your proposed patch.  There's no reason to assert/abort when
expanding "pow(10.0,x)" as "exp10(x)", instead we can simply fall
through and use the usual pow optabs/libm calls.

For example, your current implementation for the exp2 case

+      /* Optimize pow(2.0,y) = exp2(y).  */
+      else if (REAL_VALUES_EQUAL (c, dconst2))
+	{
+	  tree fndecl = get_callee_fndecl (exp);
+	  enum built_in_function fcode, expcode;
+	  tree fn;
+
+	  /* Only convert in ISO C99 mode.  */
+	  if (!TARGET_C99_FUNCTIONS)
+	    return NULL_RTX;
+
+	  fcode = DECL_FUNCTION_CODE (fndecl);
+
+	  if (fcode == BUILT_IN_POW
+	      || fcode == BUILT_IN_POWF
+	      || fcode == BUILT_IN_POWL)
+	    expcode = BUILT_IN_EXP2;
+	  else
+	    gcc_unreachable();
+
+	  fn = mathfn_built_in (TREE_TYPE (arg1), expcode);
+	  gcc_assert (fn != NULL_TREE);
+
+	  arglist = build_tree_list (NULL_TREE, arg1);
+	  exp = build_function_call_expr (fn, arglist);
+
+	  return expand_builtin_mathfn (exp, target, subtarget);
+	}

would become something like:

+      /* Optimize pow(2.0,y) = exp2(y).  */
+      else if (REAL_VALUES_EQUAL (c, dconst2))
+        {
+	  tree fn = mathfn_built_in (TREE_TYPE (arg1), BUILT_IN_EXP2);
+         if (fn != NULL_TREE)
+           {
+	      arglist = build_tree_list (NULL_TREE, arg1);
+	      exp = build_function_call_expr (fn, arglist);
+	      return expand_builtin_mathfn (exp, target, subtarget);
+           }
+	 }


We know that the fcode of the current fndecl must be BUILT_IN_POW* here
in expand_builtin_pow, so there's no need to check, which means there's
no reason to call get_callee_fndecl.  Similarly, testing fn != NULL_TREE
avoids the gcc_assert and more accurately reflects the conditions
under which exp2 is available than your original TARGET_C99_FUNCTIONS
test.

For example, your proposed implementation would fail to use the
pow_optab on platforms that didn't have TARGET_C99_FUNCTIONS.
Again, it's better to fall through to the expand_builtin_mathfn2
than to return NULL_RTX, which always results in a function call.

I suspect the your current use of gcc_assert stems from your
recent work of __builtin_lfloor where we couldn't fall back to
calling a library function (lfloor doesn't exist in libm).  For
converting pow to expN things are much easier.


Of course, now we need some way of allowing targets with exp10
in their libm (such as glibc platforms) to express this so
that mathfn_built_in returns a suitable decl in the above code.
Clearly, we can't/shouldn't convert exp10 -> pow unless we're
sure we can convert it back again during RTL expansion.


At the risk of repeating myself, we should really also change
expand_builtin_mathfn to take a "fndecl" and "arglist" instead
of an "exp" to reduce the number of temporary tree nodes we
require (i.e. to eliminate build_function_call_expr above).


Finally, for testing against dconste, I think with -ffast-math
you should real_convert the dconste to float/double/long double
prior to caling REAL_VALUES_EQUAL.  dconste is internally stored
at extremely high precision, so it won't compare equal to values
that can be represented in a machine mode.  There might be a
a subroutine to "compare against constant in mode" somewhere
already, but if not it might be a useful to add one.  Then
pow(exp(1.0),n) should be generated as exp(n).


I hope this helps.

Roger
--


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]