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

[Richard Guenther]

On Thu, 30 Apr 2009, Richard Guenther wrote:

> 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
> 
> > -       && 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.
> 
> For this reason I dislike the substitute_ter_expr helper.  I'll
> defer to Matz for suggesting a correct helper (it needs to check
> we _can_ TER at least -- but some of these checks may also make
> sense if we can re-materialize the expression locally even if they
> are not singly used).
> 
> Is there a particular reason why combine doesn't catch whatever
> the expander does here (I didn't look too closely yet).

Actually the helper could look like

gimple
get_def_for_expr (tree name, enum tree_code code, bool single_use_p)
{
  gimple def_stmt;
  if (TREE_CODE (name) != SSA_NAME)
    return NULL;
  if (single_use_p
      && !has_single_use (name))
    return NULL;
  def_stmt = SSA_NAME_DEF_STMT (name);
  if (!is_gimple_assign (def_stmt)
      /* We can only unconditionally move unary and binary expressions.  
*/
      || (gimple_assign_rhs_class (def_stmt) != GIMPLE_UNARY_RHS
          && gimple_assign_rhs_class (def_stmt) != GIMPLE_BINARY_RHS)
      || gimple_assign_rhs_code (def_stmt) != code)
    return NULL;
  return def_stmt;
}

so you can check

    if ((TREE_CODE (type) == INTEGER_TYPE
          || TREE_CODE (type) == FIXED_POINT_TYPE)
        && (subexp0def = get_def_for_expr (TREE_OPERAND (exp, 0), 
MULT_EXPR, true)))

and then do

    subexp0 = gimple_assign_rhs1 (subexp0def);
    subexp1 = gimple_assign_rhs2 (subexp0def);

or use get_def_for_expr again.

Richard.

> 
> Thanks,
> Richard.
> 
> > Adam
> > 
> > 
> > 	* expr.c (substitute_ter_expr): New function.
> > 	(expand_expr_real_1) <PLUS_EXPR, MINUS_EXPR>: Use it when looking
> > 	at operands.
> > 
> > Index: expr.c
> > ===================================================================
> > --- expr.c	(revision 146915)
> > +++ expr.c	(working copy)
> > @@ -6992,6 +6992,23 @@ expand_constructor (tree exp, rtx target
> >    return target;
> >  }
> >  
> > +/* If EXP is an SSA_NAME and has a TER expression return that.
> > +   Otherwise return EXP.  */
> > +
> > +static tree
> > +substitute_ter_expr (tree exp)
> > +{
> > +  gimple g;
> > +
> > +  if (TREE_CODE (exp) != SSA_NAME)
> > +    return exp;
> > +  g = get_gimple_for_ssa_name (exp);
> > +  if (!g)
> > +    return exp;
> > +
> > +  return gimple_assign_rhs_to_tree (g);
> > +}
> > +
> >  
> >  /* expand_expr: generate code for computing expression EXP.
> >     An rtx for the computed value is returned.  The value is never null.
> > @@ -8322,14 +8339,14 @@ expand_expr_real_1 (tree exp, rtx target
> >        /* Check if this is a case for multiplication and addition.  */
> >        if ((TREE_CODE (type) == INTEGER_TYPE
> >  	   || TREE_CODE (type) == FIXED_POINT_TYPE)
> > -	  && TREE_CODE (TREE_OPERAND (exp, 0)) == MULT_EXPR)
> > +	  && (subexp0 = substitute_ter_expr (TREE_OPERAND (exp, 0)))
> > +	  && TREE_CODE (subexp0) == MULT_EXPR)
> >  	{
> >  	  tree subsubexp0, subsubexp1;
> >  	  enum tree_code code0, code1, this_code;
> >  
> > -	  subexp0 = TREE_OPERAND (exp, 0);
> > -	  subsubexp0 = TREE_OPERAND (subexp0, 0);
> > -	  subsubexp1 = TREE_OPERAND (subexp0, 1);
> > +	  subsubexp0 = substitute_ter_expr (TREE_OPERAND (subexp0, 0));
> > +	  subsubexp1 = substitute_ter_expr (TREE_OPERAND (subexp0, 1));
> >  	  code0 = TREE_CODE (subsubexp0);
> >  	  code1 = TREE_CODE (subsubexp1);
> >  	  this_code = TREE_CODE (type) == INTEGER_TYPE ? NOP_EXPR
> > @@ -8485,14 +8502,14 @@ expand_expr_real_1 (tree exp, rtx target
> >        /* Check if this is a case for multiplication and subtraction.  */
> >        if ((TREE_CODE (type) == INTEGER_TYPE
> >  	   || TREE_CODE (type) == FIXED_POINT_TYPE)
> > -	  && TREE_CODE (TREE_OPERAND (exp, 1)) == MULT_EXPR)
> > +	  && (subexp1 = substitute_ter_expr (TREE_OPERAND (exp, 1)))
> > +	  && TREE_CODE (subexp1) == MULT_EXPR)
> >  	{
> >  	  tree subsubexp0, subsubexp1;
> >  	  enum tree_code code0, code1, this_code;
> >  
> > -	  subexp1 = TREE_OPERAND (exp, 1);
> > -	  subsubexp0 = TREE_OPERAND (subexp1, 0);
> > -	  subsubexp1 = TREE_OPERAND (subexp1, 1);
> > +	  subsubexp0 = substitute_ter_expr (TREE_OPERAND (subexp1, 0));
> > +	  subsubexp1 = substitute_ter_expr (TREE_OPERAND (subexp1, 1));
> >  	  code0 = TREE_CODE (subsubexp0);
> >  	  code1 = TREE_CODE (subsubexp1);
> >  	  this_code = TREE_CODE (type) == INTEGER_TYPE ? NOP_EXPR
> > 
> > 
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex