This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH]: PR29335 add code for exp, exp2, exp10/pow10
- From: Roger Sayle <roger at eyesopen dot com>
- To: "Kaveh R. GHAZI" <ghazi at caip dot rutgers dot edu>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 23 Oct 2006 20:31:08 -0600 (MDT)
- Subject: Re: [PATCH]: PR29335 add code for exp, exp2, exp10/pow10
On Mon, 23 Oct 2006, Kaveh R. GHAZI wrote:
> 2006-10-22 Kaveh R. Ghazi <email@example.com>
> * 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.
> * 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.