Re: [PATCH] Fix multiply-add regressions after expand-from-SSA

[Richard Guenther <rguenther at suse dot de>]

On Thu, 30 Apr 2009, Adam Nemet wrote:

> Richard Guenther writes:
> > On Thu, 30 Apr 2009, Adam Nemet wrote:
> > 
> > > This fixes the MIPS madd* and msub* regressions from:
> > > 
> > >   http://gcc.gnu.org/ml/gcc-testresults/2009-04/msg03063.html
> > > 
> > > We now need to look at the TERed expressions for the operands to find the
> > > underlying multiplications and casts.
> > > 
> > > OK if regtest/bootstrap passes on mips64octeon-linux?
> > 
> > I think we should start to _not_ re-build trees here, but instead of
> 
> I am not rebuilding any tree here, just as operands are looked at I look at
> the TER expression in available.  Maybe the name of the helper is wrong.  I

Well, gimple_assign_rhs_to_tree re-builds trees for your operands.

> don't mean subsitute as in walking the tree and substituting subtrees but as
> in return the original expr or subsitute it with something "better".

But the better is trees ;)

> > > -       && TREE_CODE (TREE_OPERAND (exp, 0)) == MULT_EXPR)
> > > +       && (subexp0 = substitute_ter_expr (TREE_OPERAND (exp, 0)))
> > > +       && TREE_CODE (subexp0) == MULT_EXPR)
> > 
> > do
> >           && TREE_CODE (TREE_OPERAND (exp, 0)) == SSA_NAME
> >           && is_gimple_assign (SSA_NAME_DEF_STMT (TREE_OPERAND (exp, 0)))
> >           && gimple_assign_rhs_code (SSA_NAME_DEF_STMT (TREE_OPERAND (exp, 
> > 0)) == MULT_EXPR)
> > 
> > etc.
> 
> Isn't that exactly what substitute_ter_expr does (maybe the name is
> misleading).  I just only do the SSA_NAME_DEF_STMT if TER decided so.
>
> > Is there a particular reason why combine doesn't catch whatever
> > the expander does here (I didn't look too closely yet).
> 
> It's hard to say because at the moment it seems that even widening
> multiplication is broken.  I.e. we of course need to look at the definition of
> the operand to find the cast.

widening multiply needs to many insns to look at for combine at least.

> What's has been the design in expand-from-SSA to handle these cases?

The design is to either handle these before expansion or to look up
definitions like you/me try to do.

Richard.