[RFC PATCH] Reassociate MINUS_EXPRs

[Michael Matz]

Hi,

On Tue, 13 Apr 2010, Alexander Monakov wrote:

> This makes MINUS_EXPRs reassociable with PLUS_EXPRs in 
> tree-ssa-reassoc.c, en passant getting rid of somewhat expensive gimple 
> rewriting that happens in break_up_subtract and repropagate_negates.

Nice approach.  I do like getting rid of break_up_subtract.

> The patch now allows not only SSA_NAMEs and constants, but also 
> NEGATE_EXPRs as operands in struct operand_entry.  Thus, 
> linearize_expr_tree collects operands from trees of {PLUS,MINUS}_EXPRs 
> into additive pool (thus, walking A - (B - C) places A, -B, C into ops 
> vector).

Have you considered adding a flag to the op struct, instead of building 
explicit NEGATE_EXPR trees?

> Later, rewrite_expr initially produces statements with PLUS_EXPRs and 
> possibly negated operands; such statements are fixed by fixup_negates 
> that does some minimal surgery on gimple to restore correctness.

I guess this would be a bit more complicated with only a flag, but it 
would build less trees which always is a good thing.

> Bootstrapped and regtested on x86_64-linux.  There is a couple of test 
> failures where the patch negates a floating-point constant, producing x 
> + -5.0 instead of x - 5.0.  What should I do about those?

We canonicalize to non-negative constants, so you should do the same.  My 
guess it that this code:

+  if (negate)
+    op = fold_build1 (NEGATE_EXPR, TREE_TYPE (op), op);

actually produces the negative constant.  If you would just call build1, 
not fold_build1, the problem would probably go away (and the explicit 
negate would be propagated as a MINUX_EXPR by your fixup code).


Ciao,
Michael.