Re: Extend widening_mul pass to handle fixed-point types

[Richard Sandiford <rdsandiford@googlemail.com>]

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);