Re: [patch] Fix PR20922

[Roger Sayle <roger at eyesopen dot com>]

On 17 Apr 2005, James A. Morrison wrote:
> 2005-04-16  James A. Morrison  <phython@gcc.gnu.org>
>
>	* fold-const.c (fold_binary): Fold X - c > X and X + c < X to false.
>       Fold X + c >= X and fold X - c <= X to true.

I'll warn you that I'm not a fan of C's undefined signed overflow
semantics, but your patch has done an excellent job of keeping track
of when these transformations are applicable, and which ones are safe.

The several minor tweaks that I'd recommend are to avoid a dependence
upon fold's canonicalization rules which have been known to change
(evolve) over time.

Firstly, all of the floating point transformations rely on the real
constant having been canonicalized to a non-negative form, which at
present they are.  It might be slightly safer to double check this
using "&& ! REAL_VALUE_NEGATIVE (TREE_REAL_CST (arg01))" so that
nothing bad happens if this ever changes in the future.

I also think these transformations require the additional check for
signaling NaNs, where we're required to preserve the addition/subtraction
to generate an exception if necessary.  I also notice there's a bug
in your "HONOR_NANS (TYPE_MODE (type))", because at this point type
is the result type of the comparison, i.e. a Boolean or integer type,
and not the floating point mode of the operands.


Just a suggestion and not a requirement, but it might be slight
simplification to refactor the code for these transformations as:

	if ((TREE_CODE (arg0) == PLUS_EXPR
	     || TREE_CODE (arg0) == MINUS_EXPR)
	    && operand_equal_p (TREE_OPERAND (arg0, 0), arg1)
	    && TREE_CODE (TREE_OPERAND (arg0, 1) == REAL_CST
	    && !HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg0))))
	  {
	    /* All the individual transformations go here.  */
	  }

That way the check for a negative REAL_CST can be used to
flip the sense of MINUS_EXPR to a PLUS_EXPR and vice versa.


Then lastly, for the integer transformations, it might make sense
to add support for the PLUS_EXPR of a negative constant.  Again
we may always be transforming "x + -c" into "x - c" at the moment,
but it's a simple thing to catch.  Given the large duplication
in these clauses it might make sense to also factor them as
described above.


My only concern is that these transformations are likely to
expose latent bugs in the compiler where we aren't being as
careful about -fwrapv vs. -fno-wrapv with other transformations.


Finally, I'm impressed by the huge amount of thought that
you've put into this, that might not be immediately obvious
to someone reading the patch.  For example, I'm impressed
by all of the unsafe transformations that you've intentionally
left out, such as the floating point "(x - c) >= x" -> false.

Perhaps someone can comment on whether other compilers perform
these transformations?

p.s. Don't forget to mention PR tree-optimization/20922 in
your ChangeLog entry.

Roger
--