Re: [PATCH]: Add some folding of builtins fmin/fmax

[Roger Sayle <roger at eyesopen dot com>]

Hi Kaveh,

On Sat, 25 Nov 2006, Kaveh R. GHAZI wrote:
> 2006-11-23  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>
>
> 	* builtins.c (fold_builtin_fmin_fmax): Handle NaN arguments.
>
> 	* gcc.dg/torture/builtin-minmax-1.c: Test NaN in fmin/fmax.

This is OK for mainline.  Thanks.    You're prefectly correct that
you don't need to use omit_one_operand here, as a NaN REAL_CST doesn't
have any explicit side-effects, but we might consider disabling
this transformation when we honor flag_signaling_nans, i.e. HONOR_SNANS.
Not sure what the prescribed signaling NaN semantics for fmin/fmax are,
but it does no harm to skip this optimization if someone explicitly
passes -fsignaling-nans on the command line.


> +      /* If either argument is NaN, return the other one.  */
> +      if (TREE_CODE (arg0) == REAL_CST && real_isnan (&TREE_REAL_CST (arg0)))
> +	return fold_convert (type, arg1);
> +      else
> +	if (TREE_CODE (arg1) == REAL_CST && real_isnan (&TREE_REAL_CST (arg1)))
> +	  return fold_convert (type, arg0);

I'd also prefer removing the "else" here.  The prevailing style in
fold-const.c/builtins.c is to list the long sequence of possible
transformations omitting the "else" between independent transformations
that can be reordered, especially when the then-clause always returns.
This is easier to read than the deep else-if constructs in cse.c and
combine.c and also slightly easier on the compiler/parser, c.f. PR12454.


>        /* Transform fmin/fmax(x,x) -> x.  */
>        if (operand_equal_p (arg0, arg1, OEP_PURE_SAME))
>  	return omit_one_operand (type, arg0, arg1);

In fact, I think we don't strictly need omit_one_operand here as
operand_equal_p guarantees that that its arguments don't have
side-effects when it returns true.  But this is a pre-existing
condition, that I just noticed in reference to Andrew's comments.


Thanks for implementing this.  I'll also pre-approve either a revised
patch or a follow-up patch that addresses the above issues.  Thanks
in advance.

Roger
--