This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PATCH - [tree-ssa] regrouping of expression tree for singlemultiply add.
- From: Roger Sayle <roger at eyesopen dot com>
- To: Fariborz Jahanian <fjahanian at apple dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 22 Mar 2004 19:02:24 -0700 (MST)
- Subject: Re: PATCH - [tree-ssa] regrouping of expression tree for singlemultiply add.
On Mon, 22 Mar 2004, Fariborz Jahanian wrote:
> This is the revised patch; incorporating Roger's comments. This patch
> has been bootstrapped, dejagnu tested on ppc-darwin. It has also been
> tested against SPEC2000 with IMA on ppc-darwin. I am proposing this
> patch for tree-ssa because it is an 'improvement' patch and I believe
> mainline is frozen for improvement patches.
>
> OK for tree-ssa?
As Diego has pointed out, tree-ssa is frozen for regression only fixes
prior its merge with mainline. mainline, however, is still in stage1
[accepting any suitable improvement] and is where this patch should go.
> 2004-03-17 Fariborz Jahanian <fjahanian@apple.com>
>
> * fold-const.c (fold): reassociate multiply expression
> with an adjacent non-multiply expression to use
> architecture's multiply-add instruction.
>
Almost. The patch looks much better, however, in the first "clause"
you need to check that *both* tree10 and tree11 are MULT_EXPR, and in
the second clause, you need to check both tree00 and tree01 are MULT_EXPR.
Finally, I also like the way your comments show the reassociation, which
also makes is clear that the left-to-right evaluation order is unchanged
by this transformation. So as its currently written, you don't need to
test reorder_operands_p! [Sorry for not noticing this the way these
expressions/transformations were originally written].
Hence all you need to do is replace reorder_operands_p with a TREE_CODE
test for the other operand, in each clause. Ok for *mainline* with those
changes [after the usual bootstrap and regression testing]. You'll also
need to capitalize "Reassociate" in your ChangeLog entry :>
Thanks,
Roger
--