Fix PR64137

The following fixes invalid GENERIC (and also wrong-code?) generated
for various testcases involving min/maxloc intrinsics.  It seems
like the wrong expression is used to discriminate REAL vs. INTEGER
types and thus REAL -Huge is offsetted by integer -1.

To quote a little more context, here is the 'tmp' init code:

  limit = gfc_create_var (gfc_typenode_for_spec (&arrayexpr->ts), 
"limit");
  switch (arrayexpr->ts.type)
    {
    case BT_REAL:
      tmp = gfc_build_inf_or_huge (TREE_TYPE (limit), arrayexpr->ts.kind);
      break;

    case BT_INTEGER:
      n = gfc_validate_kind (arrayexpr->ts.type, arrayexpr->ts.kind, 
false);
      tmp = gfc_conv_mpz_to_tree (gfc_integer_kinds[n].huge,
                                  arrayexpr->ts.kind);
      break;

    default:
      gcc_unreachable ();
    }

note the type we switch on.

Testing in progress, ok if that passes (it fixes the testcases
I ran into ICEs with, including they still pass at runtime).

Thanks,
Richard.

2014-12-01  Richard Biener  <rguenther@suse.de>

	PR fortran/64137
	* trans-intrinsic.c (gfc_conv_intrinsic_minmaxloc): Check
	proper expressions type.

Your change is OK (we don’t want to use the type of the result, but the type of the argument indeed).


> Index: gcc/fortran/trans-intrinsic.c
> ===================================================================
> --- gcc/fortran/trans-intrinsic.c	(revision 218211)
> +++ gcc/fortran/trans-intrinsic.c	(working copy)
> @@ -3729,7 +3729,7 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * s
>      possible value is HUGE in both cases.  */
>   if (op == GT_EXPR)
>     tmp = fold_build1_loc (input_location, NEGATE_EXPR, TREE_TYPE (tmp), tmp);
> -  if (op == GT_EXPR && expr->ts.type == BT_INTEGER)
> +  if (op == GT_EXPR && arrayexpr->ts.type == BT_INTEGER)
>     tmp = fold_build2_loc (input_location, MINUS_EXPR, TREE_TYPE (tmp), tmp,
> 			   build_int_cst (type, 1));


Logic would dictate that it is "build_int_cst (TREE_TYPE (tmp), 1)” instead of "build_int_cst (type, 1)” in that last line. Probably doesn’t matter much, as it will be all folded into the same value anyway, but could you test and commit that change together, while you’re at it?


Thanks,
FX

On Mon, 1 Dec 2014, FX wrote:

> Your change is OK (we don’t want to use the type of the result, but the type of the argument indeed).
> 
> 
> > Index: gcc/fortran/trans-intrinsic.c
> > ===================================================================
> > --- gcc/fortran/trans-intrinsic.c	(revision 218211)
> > +++ gcc/fortran/trans-intrinsic.c	(working copy)
> > @@ -3729,7 +3729,7 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * s
> >      possible value is HUGE in both cases.  */
> >   if (op == GT_EXPR)
> >     tmp = fold_build1_loc (input_location, NEGATE_EXPR, TREE_TYPE (tmp), tmp);
> > -  if (op == GT_EXPR && expr->ts.type == BT_INTEGER)
> > +  if (op == GT_EXPR && arrayexpr->ts.type == BT_INTEGER)
> >     tmp = fold_build2_loc (input_location, MINUS_EXPR, TREE_TYPE (tmp), tmp,
> > 			   build_int_cst (type, 1));
> 
> 
> Logic would dictate that it is "build_int_cst (TREE_TYPE (tmp), 1)” 
> instead of "build_int_cst (type, 1)” in that last line. Probably doesn’t 
> matter much, as it will be all folded into the same value anyway, but 
> could you test and commit that change together, while you’re at it?

Sure, will re-test and commit tomorrow.

Thanks,
Richard.