Minimum version of mpfr? (was Re: [PATCH] Fix up norm2 simplification (PR middle-end/88074))

On Wed, 2019-02-13 at 00:09 +0100, Jakub Jelinek wrote:
> Hi!
>
> As discussed recently on the mailing list, the norm2 simplification
> doesn't
> work if we limit mpfr emin/emax to some values derived from maximum
> floating
> exponents (and precision for denormals).
>
> The following patch adjusts the computation, so that it is scaled
> down if
> needed.  In particular, if any value in the array is so large that
> **2 will
> overflow on it or will be very close to it, the scale down is set to
> 2**(max_exponent/2+4), and if the result during computation gets
> close to
> overflowing, it is scaled down a little bit too.  The scaling is
> always done
> using powers of two, operands by that and the sum by **2 of that, and
> at the
> end it multiplies the sqrt back.  I had to change
> simplify_transformation_to_array, so that post_op is done immediately
> after
> finishing ops corresponding to that, so that there can be just one
> global
> variable for the scale.  From my understanding of e.g. the
> libgfortran norm2
> code where sqrt is done basically in this spot I hope it isn't
> possible that
> the same *dest is updated multiple times with dest
> increments/decrements in
> between.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux (together with
> Richard's patch), ok for trunk?
>
> 2019-02-12  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR middle-end/88074
> 	* simplify.c (simplify_transformation_to_array): Run post_op
> 	immediately after processing corresponding row, rather than at
> the
> 	end.
> 	(norm2_scale): New variable.
> 	(add_squared): Rename to ...
> 	(norm2_add_squared): ... this.  Scale down operand and/or
> result
> 	if needed.
> 	(do_sqrt): Rename to ...
> 	(norm2_do_sqrt): ... this.  Handle the result == e case.  Scale
> up
> 	result and clear norm2_scale.
> 	(gfc_simplify_norm2): Clear norm2_scale.  Change add_squared to
> 	norm2_add_squared and &do_sqrt to norm2_do_sqrt.  Scale up
> result
> 	and clear norm2_scale again.
>

[...]

>  static gfc_expr *
> -add_squared (gfc_expr *result, gfc_expr *e)
> +norm2_add_squared (gfc_expr *result, gfc_expr *e)
>  {
>    mpfr_t tmp;
>
> @@ -6059,8 +6060,45 @@ add_squared (gfc_expr *result, gfc_expr
>  	      && result->expr_type == EXPR_CONSTANT);
>
>    gfc_set_model_kind (result->ts.kind);
> +  int index = gfc_validate_kind (BT_REAL, result->ts.kind, false);
> +  mpfr_exp_t exp;
> +  if (mpfr_regular_p (result->value.real))
> +    {

I've started seeing build failures in my testing setup here, apparently
introduced with this patch:

../../../src/gcc/fortran/simplify.c: In function ‘gfc_expr* norm2_add_squared(gfc_expr*, gfc_expr*)’:
../../../src/gcc/fortran/simplify.c:6064:3: error: ‘mpfr_exp_t’ was not declared in this scope; did you mean ‘mpfr_expm1’?
 6064 |   mpfr_exp_t exp;
      |   ^~~~~~~~~~
      |   mpfr_expm1

I'm using mpfr-2.4.2.tar.bz2, which is the minimum version stated at:
  https://gcc.gnu.org/install/prerequisites.html
which says "MPFR Library version 2.4.2 (or later)"; that
version doesn't seem to have that type.

However, I see on:
  https://www.mpfr.org/mpfr-3.0.1/mpfr.html#API-Compatibility
that "The official type for exponent values changed from mp_exp_t to
mpfr_exp_t in MPFR 3.0."

So should the patch be tweaked to use the old type name, like in the
following patch (caveat: not tested yet), or do we need to update
the minimum version of mpfr, and if so, to what version?

[...]

Dave

gcc/fortran/ChangeLog:
	PR middle-end/88074
	* simplify.c (norm2_add_squared): Use mp_exp_t rather than
	mpfr_exp_t.
---
 gcc/fortran/simplify.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Thu, Feb 21, 2019 at 04:50:27PM -0500, David Malcolm wrote:
>  gcc/fortran/simplify.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
> index fa6396b..a1df735 100644
> --- a/gcc/fortran/simplify.c
> +++ b/gcc/fortran/simplify.c
> @@ -6061,7 +6061,7 @@ norm2_add_squared (gfc_expr *result, gfc_expr *e)
>  
>    gfc_set_model_kind (result->ts.kind);
>    int index = gfc_validate_kind (BT_REAL, result->ts.kind, false);
> -  mpfr_exp_t exp;
> +  mp_exp_t exp;
>    if (mpfr_regular_p (result->value.real))
>      {
>        exp = mpfr_get_exp (result->value.real);

Ah, sorry for that.
I guess either we can use something like:
#if MPFR_VERSION < MPFR_VERSION_NUM(3,0,0)
  mp_exp_t exp;
#else
  mpfr_exp_t exp;
#endif

or perhaps just
  long exp;
or
  int exp;
?  We don't have pointers to that passed somewhere, so all we care is that
we cover the range (but, because of the previous limiting of the exponents
in toplev.c, it must fit into integer, because that is the type of the
min_exp/max_exp passed to mpfr_set_e{min,max}.

	Jakub