Extend widening_mul pass to handle fixed-point types

This patch makes the widening_mul pass handle fixed-point types,
thus fixing gcc.target/dpaq_sa_l_w.c.  WIDEN_MULT_PLUS_EXPR and
WIDEN_MULT_MINUS_EXPR already handle the types correctly,
it's just that we never generate them.

The current multiply-accumulate code (quite reasonably) converts the
multiplication to a WIDEN_MULT first, then uses the operands of that
WIDEN_MULT in the WIDEN_MULT_PLUS_EXPR and WIDEN_MULT_MINUS_EXPR.
The main gotcha with fixed-point types is that WIDEN_MULT_EXPR _doesn't_
support fixed-point types.  Indeed MIPS -- the only fixed-point target
AFAIK -- doesn't have a pure fixed-point widening multiplication
instruction, just a multiply-accumulate one[*].  So there isn't any
use for a fixed-point WIDEN_MULT_EXPR right now.  Even if I tried to
implement it, it would just be dead code, and there's no easy way of
telling whether I got right.

This patch therefore separates out the processes of getting the
unwidened operands and of generating the WIDEN_MULT, so that we can
still use the operands in cases where converting to a WIDEN_MULT isn't
possible or worthwhile.  I realise this is a bit of a strange case,
but it does mean that the generation of WIDEN_MULT_{PLUS,MINUS}_EXPR
depends only on whether the associated multiply-accumulate optab exists,
not on whether other optabs (like the pure multiplication ones) exist too.

Note that expand_debug_expr doesn't handle fixed-point types at
the moment.  That goes for these codes and for much simpler ones.
I think adding that is a separate change.

Tested on mipsisa64-elfoabi, where it fixes the testsuite failure.
Also bootstrapped & regression-tested on x86_64-linux-gnu.  OK to instal?

Richard

  [*] It may well be that MIPS should use multiply-accumulate patterns
      to implement plain multiplication, but I don't have appropriate
      hardware to test that performance-wise.


gcc/
	* tree-ssa-math-opts.c (convert_mult_to_widen): Add an RHS_OUT
	parameter.  Handle fixed-point types as well as integer ones,
	but continue to only generate WIDEN_MULT_EXPR for the latter.
	Use RHS_OUT to return the unwidened operands to the caller.
	(convert_plusminus_to_widen): Handle fixed-point types as
	well as integer ones.  Update the call to convert_mult_to_widen.
	(execute_optimize_widening_mul):  Update the call to
	convert_mult_to_widen.

On 07/18/2010 02:03 PM, Richard Sandiford wrote:

> +  /* At present, WIDEN_MULT_EXPR only supports integer types,
> +     not fixed-point ones.  Processing fixed-point types is only
> +     useful if the caller wants the unextended operands.  */
> +  if (TREE_CODE (type) == FIXED_POINT_TYPE)
> +    use_widen_mult_p = false;

I don't like this bit.  I'd break up this function into one that just
extracts the unwidened operands, and another one that generates the
widening multiply.  The former can then be used for also generating
widening-macc.  Whether to generate anything for fixed-point should just
depend on the availability of the optabs.

Ok with that change.


Bernd

Bernd Schmidt <bernds@codesourcery.com> writes:
> On 07/18/2010 02:03 PM, Richard Sandiford wrote:
>
>> +  /* At present, WIDEN_MULT_EXPR only supports integer types,
>> +     not fixed-point ones.  Processing fixed-point types is only
>> +     useful if the caller wants the unextended operands.  */
>> +  if (TREE_CODE (type) == FIXED_POINT_TYPE)
>> +    use_widen_mult_p = false;
>
> I don't like this bit.  I'd break up this function into one that just
> extracts the unwidened operands, and another one that generates the
> widening multiply.  The former can then be used for also generating
> widening-macc.  Whether to generate anything for fixed-point should just
> depend on the availability of the optabs.

I'm not sure I follow the last sentence.  Whether we generate widening
macc for fixed-point types _does_ just depend on the optabs.  The code
you quote above is there to handle the fact that WIDEN_MULT_EXPR (as
opposed to WIDEN_MULT_PLUS_EXPR and WIDEN_MULT_MINUS_EXPR) don't handle
fixed-point types at present.  I'd rather not add that support because
nothing needs it, and it'd therefore be hard to test.  (See covering
message for details.)

I did wonder about splitting the stuff out into a separate function.
The advantage of not doing is that it avoids duplicate work in cases
where we visit the macc first, and where the WIDEN_MULT_EXPR _can_
be used.  I thought that would be quite a common case.

Here's a version that splits it out and drops the two-at-once thing.
Only lightly tested, but does it look better?

Richard


gcc/
	* tree-ssa-math-opts.c (is_widening_mult_rhs_p): New function.
	(is_widening_mult_p): Likewise.
	(convert_to_widen): Use them.
	(convert_plusminus_to_widen): Likewise.  Handle fixed-point types as
	well as integer ones.

Index: gcc/tree-ssa-math-opts.c
===================================================================
*** gcc/tree-ssa-math-opts.c	2010-07-18 13:16:54.000000000 +0100
--- gcc/tree-ssa-math-opts.c	2010-07-19 19:28:27.000000000 +0100
*************** struct gimple_opt_pass pass_optimize_bsw
*** 1260,1352 ****
   }
  };
  
! /* Process a single gimple statement STMT, which has a MULT_EXPR as
!    its rhs, and try to convert it into a WIDEN_MULT_EXPR.  The return
!    value is true iff we converted the statement.  */
  
! static bool
! convert_mult_to_widen (gimple stmt)
! {
!   gimple rhs1_stmt = NULL, rhs2_stmt = NULL;
!   tree type1 = NULL, type2 = NULL;
!   tree rhs1, rhs2, rhs1_convop = NULL, rhs2_convop = NULL;
!   enum tree_code rhs1_code, rhs2_code;
!   tree type;
  
!   type = TREE_TYPE (gimple_assign_lhs (stmt));
! 
!   if (TREE_CODE (type) != INTEGER_TYPE)
!     return false;
  
!   rhs1 = gimple_assign_rhs1 (stmt);
!   rhs2 = gimple_assign_rhs2 (stmt);
  
!   if (TREE_CODE (rhs1) == SSA_NAME)
      {
!       rhs1_stmt = SSA_NAME_DEF_STMT (rhs1);
!       if (!is_gimple_assign (rhs1_stmt))
  	return false;
!       rhs1_code = gimple_assign_rhs_code (rhs1_stmt);
!       if (!CONVERT_EXPR_CODE_P (rhs1_code))
  	return false;
!       rhs1_convop = gimple_assign_rhs1 (rhs1_stmt);
!       type1 = TREE_TYPE (rhs1_convop);
!       if (TYPE_PRECISION (type1) * 2 != TYPE_PRECISION (type))
  	return false;
      }
-   else if (TREE_CODE (rhs1) != INTEGER_CST)
-     return false;
  
!   if (TREE_CODE (rhs2) == SSA_NAME)
      {
!       rhs2_stmt = SSA_NAME_DEF_STMT (rhs2);
!       if (!is_gimple_assign (rhs2_stmt))
! 	return false;
!       rhs2_code = gimple_assign_rhs_code (rhs2_stmt);
!       if (!CONVERT_EXPR_CODE_P (rhs2_code))
! 	return false;
!       rhs2_convop = gimple_assign_rhs1 (rhs2_stmt);
!       type2 = TREE_TYPE (rhs2_convop);
!       if (TYPE_PRECISION (type2) * 2 != TYPE_PRECISION (type))
! 	return false;
      }
!   else if (TREE_CODE (rhs2) != INTEGER_CST)
      return false;
  
!   if (rhs1_stmt == NULL && rhs2_stmt == NULL)
      return false;
  
!   /* Verify that the machine can perform a widening multiply in this
!      mode/signedness combination, otherwise this transformation is
!      likely to pessimize code.  */
!   if ((rhs1_stmt == NULL || TYPE_UNSIGNED (type1))
!       && (rhs2_stmt == NULL || TYPE_UNSIGNED (type2))
!       && (optab_handler (umul_widen_optab, TYPE_MODE (type))
! 	  == CODE_FOR_nothing))
      return false;
!   else if ((rhs1_stmt == NULL || !TYPE_UNSIGNED (type1))
! 	   && (rhs2_stmt == NULL || !TYPE_UNSIGNED (type2))
! 	   && (optab_handler (smul_widen_optab, TYPE_MODE (type))
! 	       == CODE_FOR_nothing))
      return false;
!   else if (rhs1_stmt != NULL && rhs2_stmt != NULL
! 	   && (TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2))
! 	   && (optab_handler (usmul_widen_optab, TYPE_MODE (type))
! 	       == CODE_FOR_nothing))
      return false;
  
!   if ((rhs1_stmt == NULL && !int_fits_type_p (rhs1, type2))
!       || (rhs2_stmt == NULL && !int_fits_type_p (rhs2, type1)))
      return false;
  
!   if (rhs1_stmt == NULL)
!     gimple_assign_set_rhs1 (stmt, fold_convert (type2, rhs1));
!   else
!     gimple_assign_set_rhs1 (stmt, rhs1_convop);
!   if (rhs2_stmt == NULL)
!     gimple_assign_set_rhs2 (stmt, fold_convert (type1, rhs2));
    else
!     gimple_assign_set_rhs2 (stmt, rhs2_convop);
    gimple_assign_set_rhs_code (stmt, WIDEN_MULT_EXPR);
    update_stmt (stmt);
    return true;
--- 1260,1379 ----
   }
  };
  
! /* Return true if RHS is a suitable operand for a widening multiplication.
!    There are two cases:
  
!      - RHS makes some value twice as wide.  Store that value in *NEW_RHS_OUT
!        if so, and store its type in *TYPE_OUT.
  
!      - RHS is an integer constant.  Store that value in *NEW_RHS_OUT if so,
!        but leave *TYPE_OUT untouched.  */
  
! static bool
! is_widening_mult_rhs_p (tree rhs, tree *type_out, tree *new_rhs_out)
! {
!   gimple stmt;
!   tree type, type1, rhs1;
!   enum tree_code rhs_code;
  
!   if (TREE_CODE (rhs) == SSA_NAME)
      {
!       type = TREE_TYPE (rhs);
!       stmt = SSA_NAME_DEF_STMT (rhs);
!       if (!is_gimple_assign (stmt))
  	return false;
! 
!       rhs_code = gimple_assign_rhs_code (stmt);
!       if (TREE_CODE (type) == INTEGER_TYPE
! 	  ? !CONVERT_EXPR_CODE_P (rhs_code)
! 	  : rhs_code != FIXED_CONVERT_EXPR)
  	return false;
! 
!       rhs1 = gimple_assign_rhs1 (stmt);
!       type1 = TREE_TYPE (rhs1);
!       if (TREE_CODE (type1) != TREE_CODE (type)
! 	  || TYPE_PRECISION (type1) * 2 != TYPE_PRECISION (type))
  	return false;
+ 
+       *new_rhs_out = rhs1;
+       *type_out = type1;
+       return true;
      }
  
!   if (TREE_CODE (rhs) == INTEGER_CST)
      {
!       *new_rhs_out = rhs;
!       *type_out = NULL;
!       return true;
      }
! 
!   return false;
! }
! 
! /* Return true if STMT performs a widening multiplication.  If so,
!    store the unwidened types of the operands in *TYPE1_OUT and *TYPE2_OUT
!    respectively.  Also fill *RHS1_OUT and *RHS2_OUT such that converting
!    those operands to types *TYPE1_OUT and *TYPE2_OUT would give the
!    operands of the multiplication.  */
! 
! static bool
! is_widening_mult_p (gimple stmt,
! 		    tree *type1_out, tree *rhs1_out,
! 		    tree *type2_out, tree *rhs2_out)
! {
!   tree type;
! 
!   type = TREE_TYPE (gimple_assign_lhs (stmt));
!   if (TREE_CODE (type) != INTEGER_TYPE
!       && TREE_CODE (type) != FIXED_POINT_TYPE)
      return false;
  
!   if (!is_widening_mult_rhs_p (gimple_assign_rhs1 (stmt), type1_out, rhs1_out))
      return false;
  
!   if (!is_widening_mult_rhs_p (gimple_assign_rhs2 (stmt), type2_out, rhs2_out))
      return false;
! 
!   if (*type1_out == NULL
!       && (*type2_out == NULL || !int_fits_type_p (*rhs1_out, *type2_out)))
      return false;
! 
!   if (*type2_out == NULL && !int_fits_type_p (*rhs2_out, *type1_out))
!     return false;
! 
!   return true;
! }
! 
! /* Process a single gimple statement STMT, which has a MULT_EXPR as
!    its rhs, and try to convert it into a WIDEN_MULT_EXPR.  The return
!    value is true iff we converted the statement.  */
! 
! static bool
! convert_mult_to_widen (gimple stmt)
! {
!   tree lhs, rhs1, rhs2, type, type1, type2;
!   enum insn_code handler;
! 
!   lhs = gimple_assign_lhs (stmt);
!   type = TREE_TYPE (lhs);
!   if (TREE_CODE (type) != INTEGER_TYPE)
      return false;
  
!   if (!is_widening_mult_p (stmt, &type1, &rhs1, &type2, &rhs2))
      return false;
  
!   if (TYPE_UNSIGNED (type1) && TYPE_UNSIGNED (type2))
!     handler = optab_handler (umul_widen_optab, TYPE_MODE (type));
!   else if (!TYPE_UNSIGNED (type1) && !TYPE_UNSIGNED (type2))
!     handler = optab_handler (smul_widen_optab, TYPE_MODE (type));
    else
!     handler = optab_handler (usmul_widen_optab, TYPE_MODE (type));
! 
!   if (handler == CODE_FOR_nothing)
!     return false;
! 
!   gimple_assign_set_rhs1 (stmt, fold_convert (type1, rhs1));
!   gimple_assign_set_rhs2 (stmt, fold_convert (type2, rhs2));
    gimple_assign_set_rhs_code (stmt, WIDEN_MULT_EXPR);
    update_stmt (stmt);
    return true;
*************** convert_plusminus_to_widen (gimple_stmt_
*** 1363,1369 ****
  			    enum tree_code code)
  {
    gimple rhs1_stmt = NULL, rhs2_stmt = NULL;
!   tree type;
    tree lhs, rhs1, rhs2, mult_rhs1, mult_rhs2, add_rhs;
    enum tree_code rhs1_code = ERROR_MARK, rhs2_code = ERROR_MARK;
    optab this_optab;
--- 1390,1396 ----
  			    enum tree_code code)
  {
    gimple rhs1_stmt = NULL, rhs2_stmt = NULL;
!   tree type, type1, type2;
    tree lhs, rhs1, rhs2, mult_rhs1, mult_rhs2, add_rhs;
    enum tree_code rhs1_code = ERROR_MARK, rhs2_code = ERROR_MARK;
    optab this_optab;
*************** convert_plusminus_to_widen (gimple_stmt_
*** 1371,1377 ****
  
    lhs = gimple_assign_lhs (stmt);
    type = TREE_TYPE (lhs);
!   if (TREE_CODE (type) != INTEGER_TYPE)
      return false;
  
    if (code == MINUS_EXPR)
--- 1398,1405 ----
  
    lhs = gimple_assign_lhs (stmt);
    type = TREE_TYPE (lhs);
!   if (TREE_CODE (type) != INTEGER_TYPE
!       && TREE_CODE (type) != FIXED_POINT_TYPE)
      return false;
  
    if (code == MINUS_EXPR)
*************** convert_plusminus_to_widen (gimple_stmt_
*** 1407,1426 ****
    else
      return false;
  
!   if (rhs1_code == MULT_EXPR)
      {
!       if (!convert_mult_to_widen (rhs1_stmt))
  	return false;
!       rhs1_code = gimple_assign_rhs_code (rhs1_stmt);
      }
!   if (rhs2_code == MULT_EXPR)
      {
!       if (!convert_mult_to_widen (rhs2_stmt))
  	return false;
!       rhs2_code = gimple_assign_rhs_code (rhs2_stmt);
      }
!   
!   if (code == PLUS_EXPR && rhs1_code == WIDEN_MULT_EXPR)
      {
        mult_rhs1 = gimple_assign_rhs1 (rhs1_stmt);
        mult_rhs2 = gimple_assign_rhs2 (rhs1_stmt);
--- 1435,1459 ----
    else
      return false;
  
!   if (code == PLUS_EXPR && rhs1_code == MULT_EXPR)
      {
!       if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1,
! 			       &type2, &mult_rhs2))
  	return false;
!       mult_rhs1 = fold_convert (type1, mult_rhs1);
!       mult_rhs2 = fold_convert (type2, mult_rhs2);
!       add_rhs = rhs2;
      }
!   else if (rhs2_code == MULT_EXPR)
      {
!       if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1,
! 			       &type2, &mult_rhs2))
  	return false;
!       mult_rhs1 = fold_convert (type1, mult_rhs1);
!       mult_rhs2 = fold_convert (type2, mult_rhs2);
!       add_rhs = rhs1;
      }
!   else if (code == PLUS_EXPR && rhs1_code == WIDEN_MULT_EXPR)
      {
        mult_rhs1 = gimple_assign_rhs1 (rhs1_stmt);
        mult_rhs2 = gimple_assign_rhs2 (rhs1_stmt);

On 07/19/2010 08:34 PM, Richard Sandiford wrote:
> I did wonder about splitting the stuff out into a separate function.
> The advantage of not doing is that it avoids duplicate work in cases
> where we visit the macc first, and where the WIDEN_MULT_EXPR _can_
> be used.  I thought that would be quite a common case.

Not sure about that - normally I'd expect the multiply and add to appear
roughly in the order they're executed.

> Here's a version that splits it out and drops the two-at-once thing.
> Only lightly tested, but does it look better?

That's pretty much what I had in mind.  Ok if it passes testing.


Bernd