Re: PATCH - [tree-ssa] regrouping of expression tree for single multiply add.

[Fariborz Jahanian <fjahanian at apple dot com>]

Sorry for being late to respond. Here is my comments and a new patch
(pending bootstrap, dejagnu test of mainline).

OK for mainline if all pass ?

- Thanks, Fariborz

On Mar 22, 2004, at 6:02 PM, Roger Sayle wrote:

> 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.

I left out tree10 and tree00 intentionally. In a more complicated case; 
such as:
( (a*b + c*d) + e*f) + r
tree00 is (a*b+c*d) and checking for MULT_EXPR prevents re-association 
to
take place (TREE_CODE(tree00) == PLUS_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].

Yes, good point.

> 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,

Here is minor revision of my last patch:

Attachment: tree-ssa-fmadd-patch.txt
Description: Text document

> Roger
> --