This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC PATCH] Reassociate MINUS_EXPRs
On Wed, 14 Apr 2010, Michael Matz wrote:
> > 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.
Yes, I have gone the NEGATE_EXPR route for the sake of the simplicity of this
initial patch.
>
> > 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).
No, I think I should call fold_build1 here to allow constant coalescing by
existing code and canonicalize negative constants in fixup_negates.
Alexander