Re: [PATCH]: PR29335 add code for exp, exp2, exp10/pow10

[Roger Sayle <roger at eyesopen dot com>]

Hi Kaveh,

On Mon, 23 Oct 2006, Kaveh R. GHAZI wrote:
> 2006-10-22  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>
>
> 	* builtins.c (fold_builtin_exponent): Evaluate constant arguments
> 	at compile-time using MPFR.  Change parameter VALUE to FUNC,
> 	update all callers.
> 	(do_mpfr_arg1): Rename `exact' to `inexact'.  Carefully check
> 	for overflow and underflow at all times and avoid folding in
> 	those cases.
>
> testsuite:
> 	* gcc.dg/builtin-notdone-1.c: New test.

This is OK for mainline, provided that you change the name of the new
testcase.  I'd much prefer that builtin-foo-X.c referred to the names
of built-in functions for "foo".  Hence, builtin-exp-1.c or even
builtin-expN-1.c would be better, but "notdone", "unoptimized" or
"unfolded" don't really convey any useful information about the test.


>  /* A subroutine of fold_builtin to fold the various exponent
> -   functions.  EXP is the CALL_EXPR of a call to a builtin function.
> -   VALUE is the value which will be raised to a power.  */
> +   functions.  Return NULL_TREE if no simplification can me made.
> +   FUNC is the corresponding MPFR exponent function.  */
>
>  static tree
>  fold_builtin_exponent (tree fndecl, tree arglist,
> -		       const REAL_VALUE_TYPE *value)
> +		       int (*func)(mpfr_ptr, mpfr_srcptr, mp_rnd_t))
>  {

Though not really a problem, some part of my aesthetic conscience
isn't delighted with passing MPFR function pointers over this API.
Admittedly, we did the same previously with REAL_VALUE_TYPE*,
but the fact that we then test this pointer for equality with either
mpfr_exp, mpfr_exp2 or mpfr_exp10 hints that it might be slightly
cleaner to pass an enumeration (perhaps BUILT_IN_EXP, BUILT_IN_EXP2
and BUILT_IN_EXP10) and then hiding the mapping to mpfr functions
internally?  Not a blocker for this patch, and I can't really put
my finger on why I consider this style/idiom questionable.

Roger
--