Extend widening_mul pass to handle fixed-point types

Extend widening_mul pass to handle fixed-point types

[Richard Sandiford]

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.

Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c	2010-07-18 08:36:48.000000000 +0100
+++ gcc/tree-ssa-math-opts.c	2010-07-18 12:35:42.000000000 +0100
@@ -1260,22 +1260,30 @@ struct gimple_opt_pass pass_optimize_bsw
  }
 };
 
-/* 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.  */
+/* Process a single gimple statement STMT, which has a MULT_EXPR as its
+   rhs, and see if it is a widening multiplication.  If so:
+
+     - store the two unextended operands in RHS_OUT[0] and RHS_OUT[1],
+       if RHS_OUT is nonnull; and
+     - try to convert the statement into a WIDEN_MULT_EXPR.
+
+   Return true if the statement could be converted or if operands
+   were stored in RHS_OUT.  */
 
 static bool
-convert_mult_to_widen (gimple stmt)
+convert_mult_to_widen (gimple stmt, tree *rhs_out)
 {
   gimple rhs1_stmt = NULL, rhs2_stmt = NULL;
   tree type1 = NULL, type2 = NULL;
-  tree rhs1, rhs2, rhs1_convop = NULL, rhs2_convop = NULL;
+  tree rhs1, rhs2, unwidened_rhs1 = NULL, unwidened_rhs2 = NULL;
   enum tree_code rhs1_code, rhs2_code;
   tree type;
+  bool use_widen_mult_p;
 
   type = TREE_TYPE (gimple_assign_lhs (stmt));
 
-  if (TREE_CODE (type) != INTEGER_TYPE)
+  if (TREE_CODE (type) != INTEGER_TYPE
+      && TREE_CODE (type) != FIXED_POINT_TYPE)
     return false;
 
   rhs1 = gimple_assign_rhs1 (stmt);
@@ -1287,14 +1295,17 @@ convert_mult_to_widen (gimple stmt)
       if (!is_gimple_assign (rhs1_stmt))
 	return false;
       rhs1_code = gimple_assign_rhs_code (rhs1_stmt);
-      if (!CONVERT_EXPR_CODE_P (rhs1_code))
+      if (TREE_CODE (type) == INTEGER_TYPE
+	  ? !CONVERT_EXPR_CODE_P (rhs1_code)
+	  : rhs1_code != FIXED_CONVERT_EXPR)
 	return false;
-      rhs1_convop = gimple_assign_rhs1 (rhs1_stmt);
-      type1 = TREE_TYPE (rhs1_convop);
+      unwidened_rhs1 = gimple_assign_rhs1 (rhs1_stmt);
+      type1 = TREE_TYPE (unwidened_rhs1);
       if (TYPE_PRECISION (type1) * 2 != TYPE_PRECISION (type))
 	return false;
     }
-  else if (TREE_CODE (rhs1) != INTEGER_CST)
+  else if (TREE_CODE (rhs1) != INTEGER_CST
+	   && TREE_CODE (rhs1) != FIXED_CST)
     return false;
 
   if (TREE_CODE (rhs2) == SSA_NAME)
@@ -1303,52 +1314,73 @@ convert_mult_to_widen (gimple stmt)
       if (!is_gimple_assign (rhs2_stmt))
 	return false;
       rhs2_code = gimple_assign_rhs_code (rhs2_stmt);
-      if (!CONVERT_EXPR_CODE_P (rhs2_code))
+      if (TREE_CODE (type) == INTEGER_TYPE
+	  ? !CONVERT_EXPR_CODE_P (rhs2_code)
+	  : rhs2_code != FIXED_CONVERT_EXPR)
 	return false;
-      rhs2_convop = gimple_assign_rhs1 (rhs2_stmt);
-      type2 = TREE_TYPE (rhs2_convop);
+      unwidened_rhs2 = gimple_assign_rhs1 (rhs2_stmt);
+      type2 = TREE_TYPE (unwidened_rhs2);
       if (TYPE_PRECISION (type2) * 2 != TYPE_PRECISION (type))
 	return false;
     }
-  else if (TREE_CODE (rhs2) != INTEGER_CST)
+  else if (TREE_CODE (rhs2) != INTEGER_CST
+	   && TREE_CODE (rhs2) != FIXED_CST)
     return false;
 
   if (rhs1_stmt == NULL && rhs2_stmt == NULL)
     return false;
 
+  /* 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;
   /* 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 (umul_widen_optab, TYPE_MODE (type))
+	       == CODE_FOR_nothing))
+    use_widen_mult_p = 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;
+    use_widen_mult_p = 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))
+    use_widen_mult_p = false;
+  else
+    use_widen_mult_p = true;
+
+  if (!use_widen_mult_p && rhs_out == NULL)
     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);
+  if (unwidened_rhs1 == NULL)
+    unwidened_rhs1 = fold_convert (type2, rhs1);
+  if (unwidened_rhs2 == NULL)
+    unwidened_rhs2 = fold_convert (type1, rhs2);
+
+  if (rhs_out)
+    {
+      rhs_out[0] = unwidened_rhs1;
+      rhs_out[1] = unwidened_rhs2;
+    }
+
+  if (use_widen_mult_p)
+    {
+      gimple_assign_set_rhs1 (stmt, unwidened_rhs1);
+      gimple_assign_set_rhs2 (stmt, unwidened_rhs2);
+      gimple_assign_set_rhs_code (stmt, WIDEN_MULT_EXPR);
+      update_stmt (stmt);
+    }
   return true;
 }
 
@@ -1364,14 +1396,15 @@ convert_plusminus_to_widen (gimple_stmt_
 {
   gimple rhs1_stmt = NULL, rhs2_stmt = NULL;
   tree type;
-  tree lhs, rhs1, rhs2, mult_rhs1, mult_rhs2, add_rhs;
+  tree lhs, rhs1, rhs2, mult_rhs[2], add_rhs;
   enum tree_code rhs1_code = ERROR_MARK, rhs2_code = ERROR_MARK;
   optab this_optab;
   enum tree_code wmult_code;
 
   lhs = gimple_assign_lhs (stmt);
   type = TREE_TYPE (lhs);
-  if (TREE_CODE (type) != INTEGER_TYPE)
+  if (TREE_CODE (type) != INTEGER_TYPE
+      && TREE_CODE (type) != FIXED_POINT_TYPE)
     return false;
 
   if (code == MINUS_EXPR)
@@ -1407,29 +1440,28 @@ convert_plusminus_to_widen (gimple_stmt_
   else
     return false;
 
-  if (rhs1_code == MULT_EXPR)
+  if (code == PLUS_EXPR && rhs1_code == MULT_EXPR)
     {
-      if (!convert_mult_to_widen (rhs1_stmt))
+      if (!convert_mult_to_widen (rhs1_stmt, mult_rhs))
 	return false;
-      rhs1_code = gimple_assign_rhs_code (rhs1_stmt);
+      add_rhs = rhs2;
     }
-  if (rhs2_code == MULT_EXPR)
+  else if (rhs2_code == MULT_EXPR)
     {
-      if (!convert_mult_to_widen (rhs2_stmt))
+      if (!convert_mult_to_widen (rhs2_stmt, mult_rhs))
 	return false;
-      rhs2_code = gimple_assign_rhs_code (rhs2_stmt);
+      add_rhs = rhs1;
     }
-  
-  if (code == PLUS_EXPR && rhs1_code == WIDEN_MULT_EXPR)
+  else if (code == PLUS_EXPR && rhs1_code == WIDEN_MULT_EXPR)
     {
-      mult_rhs1 = gimple_assign_rhs1 (rhs1_stmt);
-      mult_rhs2 = gimple_assign_rhs2 (rhs1_stmt);
+      mult_rhs[0] = gimple_assign_rhs1 (rhs1_stmt);
+      mult_rhs[1] = gimple_assign_rhs2 (rhs1_stmt);
       add_rhs = rhs2;
     }
   else if (rhs2_code == WIDEN_MULT_EXPR)
     {
-      mult_rhs1 = gimple_assign_rhs1 (rhs2_stmt);
-      mult_rhs2 = gimple_assign_rhs2 (rhs2_stmt);
+      mult_rhs[0] = gimple_assign_rhs1 (rhs2_stmt);
+      mult_rhs[1] = gimple_assign_rhs2 (rhs2_stmt);
       add_rhs = rhs1;
     }
   else
@@ -1437,7 +1469,7 @@ convert_plusminus_to_widen (gimple_stmt_
 
   /* ??? May need some type verification here?  */
 
-  gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs1, mult_rhs2,
+  gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs[0], mult_rhs[1],
 				    add_rhs);
   update_stmt (gsi_stmt (*gsi));
   return true;
@@ -1467,7 +1499,7 @@ execute_optimize_widening_mul (void)
 
 	  code = gimple_assign_rhs_code (stmt);
 	  if (code == MULT_EXPR)
-	    changed |= convert_mult_to_widen (stmt);
+	    changed |= convert_mult_to_widen (stmt, NULL);
 	  else if (code == PLUS_EXPR || code == MINUS_EXPR)
 	    changed |= convert_plusminus_to_widen (&gsi, stmt, code);
 	}

Re: Extend widening_mul pass to handle fixed-point types

[Bernd Schmidt]

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

Re: Extend widening_mul pass to handle fixed-point types

[Richard Sandiford]

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

Re: Extend widening_mul pass to handle fixed-point types

[Bernd Schmidt]

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

Re: Extend widening_mul pass to handle fixed-point types

[Ramana Radhakrishnan]

On Mon, 2010-07-19 at 23:53 +0200, Bernd Schmidt wrote:
> That's pretty much what I had in mind.  Ok if it passes testing.

This broke bootstraps on arm-linux-gnueabi and caused
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45067 . 

Taking a brief look at it this morning, I think the problem here is that
the call to optab_for_tree_code is made with the wrong type of the
expression. The presence of a widening optab for a WIDEN_MULT_PLUS_EXPR
should be based on the type of ops->op2 rather than opnd0 because you
have something like intval0 = shortval1 * shortval2 + intval1 whereas
the expander is testing for a widening mult and plus where the results
are into a HImode value and thus effectively for a widening operation
into an HImode value.

The assert that fails is because 
gcc_assert (icode != CODE_FOR_nothing);

Something like this fixes the problem for the test-cases under question
or would you prefer something else. A full bootstrap and test is
underway.

Ok to commit if no regressions ? 

cheers
Ramana

2010-07-27  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

        PR bootstrap/45067
        * optabs.c (expand_widen_pattern_expr): Initialize
widen_pattern_optab correctly for WIDEN_MULT_PLUS_EXPR and
WIDEN_MULT_MINUS_EXPR.

Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c        (revision 162526)
+++ gcc/optabs.c        (working copy)
@@ -511,14 +511,20 @@
 
   oprnd0 = ops->op0;
   tmode0 = TYPE_MODE (TREE_TYPE (oprnd0));
-  widen_pattern_optab =
-    optab_for_tree_code (ops->code, TREE_TYPE (oprnd0), optab_default);
   if (ops->code == WIDEN_MULT_PLUS_EXPR
       || ops->code == WIDEN_MULT_MINUS_EXPR)
-    icode = (int) optab_handler (widen_pattern_optab,
+    {
+      widen_pattern_optab =
+       optab_for_tree_code (ops->code, TREE_TYPE (ops->op2),
optab_default);
+      icode = (int) optab_handler (widen_pattern_optab,
                                 TYPE_MODE (TREE_TYPE (ops->op2)));
+    }
   else
-    icode = (int) optab_handler (widen_pattern_optab, tmode0);
+    {
+      widen_pattern_optab =
+       optab_for_tree_code (ops->code, TREE_TYPE (oprnd0),
optab_default);
+      icode = (int) optab_handler (widen_pattern_optab, tmode0);
+    }
   gcc_assert (icode != CODE_FOR_nothing);
   xmode0 = insn_data[icode].operand[1].mode;
 



> 
> 
> Bernd

Re: Extend widening_mul pass to handle fixed-point types

[Bernd Schmidt]

On 07/27/2010 02:18 PM, Ramana Radhakrishnan wrote:
> 
> On Mon, 2010-07-19 at 23:53 +0200, Bernd Schmidt wrote:
>> That's pretty much what I had in mind.  Ok if it passes testing.
> 
> This broke bootstraps on arm-linux-gnueabi and caused
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45067 . 
> 
> Taking a brief look at it this morning, I think the problem here is that
> the call to optab_for_tree_code is made with the wrong type of the
> expression. The presence of a widening optab for a WIDEN_MULT_PLUS_EXPR
> should be based on the type of ops->op2 rather than opnd0 because you
> have something like intval0 = shortval1 * shortval2 + intval1 whereas
> the expander is testing for a widening mult and plus where the results
> are into a HImode value and thus effectively for a widening operation
> into an HImode value.
> 
> The assert that fails is because 
> gcc_assert (icode != CODE_FOR_nothing);
> 
> Something like this fixes the problem for the test-cases under question
> or would you prefer something else. A full bootstrap and test is
> underway.
> 
> Ok to commit if no regressions ? 

I think this is consistent with the code in tree-ssa-math-opts (there we
use the type of the lhs, which should match that of operand 2).  So, OK
- I think the ARM testsuite has some widening-macc cases so you should
notice if there's something wrong.

> +    {
> +      widen_pattern_optab =
> +       optab_for_tree_code (ops->code, TREE_TYPE (ops->op2),
> optab_default);
> +      icode = (int) optab_handler (widen_pattern_optab,
>                                  TYPE_MODE (TREE_TYPE (ops->op2)));
> +    }
>    else
> -    icode = (int) optab_handler (widen_pattern_optab, tmode0);
> +    {
> +      widen_pattern_optab =
> +       optab_for_tree_code (ops->code, TREE_TYPE (oprnd0),
> optab_default);
> +      icode = (int) optab_handler (widen_pattern_optab, tmode0);
> +    }

This can be written slightly more simply assigning the type to some
variable and then using that when computing widen_pattern_optab and
icode.  It would be nice to change that but I won't require it.


Bernd

Re: Extend widening_mul pass to handle fixed-point types

[Ramana Radhakrishnan]

[-- Attachment #1: Type: text/plain, Size: 2913 bytes --]


On Tue, 2010-07-27 at 17:51 +0200, Bernd Schmidt wrote:
> On 07/27/2010 02:18 PM, Ramana Radhakrishnan wrote:
> > 
> > On Mon, 2010-07-19 at 23:53 +0200, Bernd Schmidt wrote:
> >> That's pretty much what I had in mind.  Ok if it passes testing.
> > 
> > This broke bootstraps on arm-linux-gnueabi and caused
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45067 . 
> > 
> > Taking a brief look at it this morning, I think the problem here is that
> > the call to optab_for_tree_code is made with the wrong type of the
> > expression. The presence of a widening optab for a WIDEN_MULT_PLUS_EXPR
> > should be based on the type of ops->op2 rather than opnd0 because you
> > have something like intval0 = shortval1 * shortval2 + intval1 whereas
> > the expander is testing for a widening mult and plus where the results
> > are into a HImode value and thus effectively for a widening operation
> > into an HImode value.
> > 
> > The assert that fails is because 
> > gcc_assert (icode != CODE_FOR_nothing);
> > 
> > Something like this fixes the problem for the test-cases under question
> > or would you prefer something else. A full bootstrap and test is
> > underway.
> > 
> > Ok to commit if no regressions ? 
> 
> I think this is consistent with the code in tree-ssa-math-opts (there we
> use the type of the lhs, which should match that of operand 2).  So, OK
> - I think the ARM testsuite has some widening-macc cases so you should
> notice if there's something wrong.

Yep so I thought as well. Yes the tests in gcc.target/arm should cover
these cases.

Bootstrap proceeded further and ICE'd again in stage2 because we were
applying gimple_assign_lhs on a non GIMPLE_ASSIGN statement. 

Attached is a simple test case that shows this problem with a
cross-compiler as well. 

I am testing with this extra patch[1] applied on top of my previous
patch which I think is obvious and will see what else is needed to be
done to deal with the fallout of this patch with bootstrap on
arm-linux-gnueabi. Verified with a simple testcase that we still do
generate widening multiplies for the original testcase. I'll submit both
these after I'm sure that bootstrap and regression testing completes. 


cheers
Ramana



1. New patch being tested on top.

2010-07-27  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

	* tree-ssa-math-opts.c (is_widening_mult_p): Return false 
	if not an assignment.

[ramrad01@e102325-lin gcc]$ svndiff tree-ssa-math-opts.c
Index: tree-ssa-math-opts.c
===================================================================
--- tree-ssa-math-opts.c        (revision 162571)
+++ tree-ssa-math-opts.c        (working copy)
@@ -1323,6 +1323,9 @@ is_widening_mult_p (gimple stmt,
 {
   tree type;
 
+  if (!is_gimple_assign (stmt))
+    return false;
+
   type = TREE_TYPE (gimple_assign_lhs (stmt));
   if (TREE_CODE (type) != INTEGER_TYPE
       && TREE_CODE (type) != FIXED_POINT_TYPE)



[-- Attachment #2: reduced.c --]
[-- Type: text/x-csrc, Size: 2320 bytes --]

/* ./cc1 -O2 -mcpu=cortex-a9 -quiet reduced.c   */
typedef union tree_node *tree;
enum tree_code {
ERROR_MARK,
};
enum tree_code_class {
  tcc_type,
};
extern const enum tree_code_class tree_code_type[];
struct tree_base {
  __extension__ enum tree_code code : 16;
  unsigned unsigned_flag : 1;
};
struct tree_type {
  unsigned int precision : 10;
};
union
                                                         tree_node {
  struct tree_base base;
  struct tree_type type;
};
enum integer_type_kind
{
  itk_int,
  itk_none
};
extern tree integer_types[itk_none];
typedef struct cpp_reader cpp_reader;
enum c_tree_index
{
    CTI_INTMAX_TYPE,
    CTI_MAX
};
extern tree c_global_trees[CTI_MAX];
static void builtin_define_constants (const char *, tree);
static void
builtin_define_stdint_macros (void)
{
  builtin_define_constants ("__INTMAX_C", c_global_trees[CTI_INTMAX_TYPE]);
}
void
c_cpp_builtins (cpp_reader *pfile)
{
  builtin_define_stdint_macros ();
}
static const char *
type_suffix (tree type)
{
  static const char *const suffixes[] = { "", "U", "L", "UL", "LL", "ULL" };
  int unsigned_suffix;
  int is_long;
  unsigned_suffix = (__extension__ ({ __typeof (type) const __t = (type); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1084, __FUNCTION__); __t; })->base.unsigned_flag);
  if ((__extension__ ({ __typeof (type) const __t = (type); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1085, __FUNCTION__); __t; })->type.precision) < (__extension__ ({ __typeof (integer_types[itk_int]) const __t = (integer_types[itk_int]); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1085, __FUNCTION__); __t; })->type.precision))
  return suffixes[is_long * 2 + unsigned_suffix];
}
static void
builtin_define_constants (const char *macro, tree type)
{
  const char *suffix;
  char *buf;
  suffix = type_suffix (type);
  if (suffix[0] == 0)
    {
      buf = (char *) __builtin_alloca(strlen (macro) + 6);
    }
}

Re: Extend widening_mul pass to handle fixed-point types

[Ramana Radhakrishnan]

On Tue, 2010-07-27 at 17:51 +0200, Bernd Schmidt wrote:
> On 07/27/2010 02:18 PM, Ramana Radhakrishnan wrote:
> > 
> > On Mon, 2010-07-19 at 23:53 +0200, Bernd Schmidt wrote:
> >> That's pretty much what I had in mind.  Ok if it passes testing.
> > 
> > This broke bootstraps on arm-linux-gnueabi and caused
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45067 . 
> > 
> > Taking a brief look at it this morning, I think the problem here is that
> > the call to optab_for_tree_code is made with the wrong type of the
> > expression. The presence of a widening optab for a WIDEN_MULT_PLUS_EXPR
> > should be based on the type of ops->op2 rather than opnd0 because you
> > have something like intval0 = shortval1 * shortval2 + intval1 whereas
> > the expander is testing for a widening mult and plus where the results
> > are into a HImode value and thus effectively for a widening operation
> > into an HImode value.
> > 
> > The assert that fails is because 
> > gcc_assert (icode != CODE_FOR_nothing);
> > 
> > Something like this fixes the problem for the test-cases under question
> > or would you prefer something else. A full bootstrap and test is
> > underway.
> > 
> > Ok to commit if no regressions ? 
> 
> I think this is consistent with the code in tree-ssa-math-opts (there we
> use the type of the lhs, which should match that of operand 2).  So, OK
> - I think the ARM testsuite has some widening-macc cases so you should
> notice if there's something wrong.
> 
> > +    {
> > +      widen_pattern_optab =
> > +       optab_for_tree_code (ops->code, TREE_TYPE (ops->op2),
> > optab_default);
> > +      icode = (int) optab_handler (widen_pattern_optab,
> >                                  TYPE_MODE (TREE_TYPE (ops->op2)));
> > +    }
> >    else
> > -    icode = (int) optab_handler (widen_pattern_optab, tmode0);
> > +    {
> > +      widen_pattern_optab =
> > +       optab_for_tree_code (ops->code, TREE_TYPE (oprnd0),
> > optab_default);
> > +      icode = (int) optab_handler (widen_pattern_optab, tmode0);
> > +    }
> 
> This can be written slightly more simply assigning the type to some
> variable and then using that when computing widen_pattern_optab and
> icode.  It would be nice to change that but I won't require it.

Hmmm there are regressions after my patch was applied. I had to do some
rebuilds because trunk appears to be broken for other reasons at the
minute for ARM which I shall investigate next. 

After these 2 patches to fix the ICEs on top of r162301 - I see
breakages in the testsuite with execute/arith-rand.c and
execute/arith-rand-ll.c .

The difference in code generated is in the following case for -O2 is the
following for arith-rand.c 

smlabb  sl, r0, r7, sl       |         mla     sl, r7, r0, sl

with the failing case on the left. 

The problem appears to be as though the gimple pass for generating
widening macs now generates a widening multiply and add for the case
under question here while it didn't earlier.

D.2130_36 = WIDEN_MULT_PLUS_EXPR <r1_30, yy_29, D.2129_35>;

We weren't generating any widening operations before this patch (and I
suspect my 2 patches are just for fixing the ICE's and doing the right
thing). If it were just adding support for fixed point arithmetic then
this is just broken because it shouldn't have changed the way in  which
code was generated for normal integer operations.  

Ideas ? I haven't looked at this in any great depth but the essential
breakages in the C only testsuite are as below [2].

The native compiler has been configured with the following options - you
can see the same result using a cross-compiler with the following
parameters. 

--with-cpu=cortex-a9 --with-fpu=vfpv3-d16 --with-float=softfp


cheers
Ramana

[1] Trunk as of this morning gives me a miscompare in stage2 and stage3
when my patches are applied which indicates a different underlying
problem, the testsuite breakages are 162301 + the two patches applied.

[2] 

FAIL: gcc.c-torture/execute/builtins/abs-1.c execution,  -O2 -fwhopr 
FAIL: gcc.c-torture/execute/builtins/strstr-asm.c execution,  -O2
-fwhopr 
FAIL: gcc.c-torture/execute/arith-rand-ll.c execution,  -O2 
FAIL: gcc.c-torture/execute/arith-rand-ll.c execution,  -O3
-fomit-frame-pointer 
FAIL: gcc.c-torture/execute/arith-rand-ll.c execution,  -O3
-fomit-frame-pointer -funroll-loops 
FAIL: gcc.c-torture/execute/arith-rand-ll.c execution,  -O3
-fomit-frame-pointer -funroll-all-loops -finline-functions 
FAIL: gcc.c-torture/execute/arith-rand-ll.c execution,  -O3 -g 
FAIL: gcc.c-torture/execute/arith-rand-ll.c execution,  -Os 
FAIL: gcc.c-torture/execute/arith-rand-ll.c execution,  -O2 -flto 
FAIL: gcc.c-torture/execute/arith-rand-ll.c execution,  -O2 -fwhopr 
FAIL: gcc.c-torture/execute/arith-rand.c execution,  -O2 
FAIL: gcc.c-torture/execute/arith-rand.c execution,  -O3
-fomit-frame-pointer 
FAIL: gcc.c-torture/execute/arith-rand.c execution,  -O3
-fomit-frame-pointer -funroll-loops 
FAIL: gcc.c-torture/execute/arith-rand.c execution,  -O3
-fomit-frame-pointer -funroll-all-loops -finline-functions 
FAIL: gcc.c-torture/execute/arith-rand.c execution,  -O3 -g 
FAIL: gcc.c-torture/execute/arith-rand.c execution,  -Os 
FAIL: gcc.c-torture/execute/arith-rand.c execution,  -O2 -flto 
FAIL: gcc.c-torture/execute/arith-rand.c execution,  -O2 -fwhopr 
FAIL: gcc.dg/c99-stdint-1.c (test for excess errors)
FAIL: gcc.dg/c99-stdint-7.c (test for excess errors)
FAIL: gcc.dg/cproj-fails-with-broken-glibc.c execution test
FAIL: gcc.dg/uninit-13.c unconditional (test for warnings, line 8)
FAIL: gcc.dg/uninit-13.c (test for excess errors)
FAIL: gcc.dg/graphite/pr44391.c (test for excess errors)
FAIL: gcc.dg/lto/20081111 c_lto_20081111_0.o-c_lto_20081111_1.o execute
-O2 -fwhopr
FAIL: gcc.dg/lto/20081112 c_lto_20081112_0.o-c_lto_20081112_1.o execute
-O2 -fwhopr
FAIL: gcc.dg/lto/ipareference2
c_lto_ipareference2_0.o-c_lto_ipareference2_1.o execute  -O1 -fwhopr
-fwhole-program
FAIL: gcc.dg/torture/fp-int-convert-double.c  -O1  execution test
FAIL: gcc.dg/torture/fp-int-convert-long-double.c  -O1  execution test
FAIL: gcc.dg/torture/fp-int-convert-timode.c  -O1  execution test
FAIL: gcc.dg/torture/pr43879_1.c  -O2 -fwhopr  execution test
FAIL: gcc.dg/tree-ssa/pr42585.c scan-tree-dump-times optimized "struct
_fat_ptr _ans" 0
FAIL: gcc.dg/tree-ssa/pr42585.c scan-tree-dump-times optimized "struct
_fat_ptr _T2" 0
FAIL: gcc.dg/vect/pr43430-1.c scan-tree-dump-times vect "vectorized 1
loops" 1
FAIL: gcc.dg/vect/vect-35.c scan-tree-dump-times vect "vectorized 2
loops" 1
FAIL: gcc.dg/vect/vect-peel-3.c scan-tree-dump-times vect "Vectorizing
an unaligned access" 1
FAIL: gcc.dg/vect/vect-peel-4.c scan-tree-dump-times vect "Vectorizing
an unaligned access" 1
FAIL: gcc.dg/vect/slp-reduc-3.c scan-tree-dump-times vect "vectorizing
stmts using SLP" 1
FAIL: gcc.dg/vect/slp-reduc-6.c scan-tree-dump-times vect "vectorized 1
loops" 2







> 
> 
> Bernd

Re: Extend widening_mul pass to handle fixed-point types

[Richard Sandiford]

Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> writes:
> Hmmm there are regressions after my patch was applied. I had to do some
> rebuilds because trunk appears to be broken for other reasons at the
> minute for ARM which I shall investigate next. 

Yeah, I think it's the other way about: expand_widen_pattern_expr
is using the right type and convert_plusminus_to_widen is using
the wrong one.  The type we pass to optab_for_tree_code determines
the _sign_ of the multiplication, not the size, so we want to pass
in the unwidened type.

> We weren't generating any widening operations before this patch (and I
> suspect my 2 patches are just for fixing the ICE's and doing the right
> thing). If it were just adding support for fixed point arithmetic then
> this is just broken because it shouldn't have changed the way in  which
> code was generated for normal integer operations.  

For the record, the patch wasn't just about adding fixed-point support.
It was also about allowing multiply-accumulate to be used where no plain
multiplication exists.  So the problem for ARM was that the use of the
wrong type in convert_plusminus_to_widen was being hidden by the use of
the right type in convert_mult_widen (i.e. the patch exposed a latent bug).

Could you give the attached patch a try?  It also fixes a case where
the code could create mixed sign-and-unsigned maccs (a bug that existed
before my patch) and a botched call to is_widening_mult_p (a bug
introduced by my patch, sorry).

Richard


gcc/
	* tree-ssa-math-opts.c (convert_plusminus_to_widen): Fix type
	used in the call to optab_for_tree_code.  Fix the second
	is_widening_mult_p call.  Check that both unwidened operands
	have the same sign.

Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c	2010-07-28 21:09:59.000000000 +0100
+++ gcc/tree-ssa-math-opts.c	2010-07-28 21:10:00.000000000 +0100
@@ -1414,13 +1414,6 @@ convert_plusminus_to_widen (gimple_stmt_
   else
     wmult_code = WIDEN_MULT_PLUS_EXPR;
 
-  /* Verify that the machine can perform a widening multiply
-     accumulate in this mode/signedness combination, otherwise
-     this transformation is likely to pessimize code.  */
-  this_optab = optab_for_tree_code (wmult_code, type, optab_default);
-  if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing)
-    return false;
-
   rhs1 = gimple_assign_rhs1 (stmt);
   rhs2 = gimple_assign_rhs2 (stmt);
 
@@ -1447,37 +1440,49 @@ convert_plusminus_to_widen (gimple_stmt_
       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,
+      if (!is_widening_mult_p (rhs2_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);
+      type1 = TREE_TYPE (mult_rhs1);
+      type2 = TREE_TYPE (mult_rhs2);
       add_rhs = rhs2;
     }
   else if (rhs2_code == WIDEN_MULT_EXPR)
     {
       mult_rhs1 = gimple_assign_rhs1 (rhs2_stmt);
       mult_rhs2 = gimple_assign_rhs2 (rhs2_stmt);
+      type1 = TREE_TYPE (mult_rhs1);
+      type2 = TREE_TYPE (mult_rhs2);
       add_rhs = rhs1;
     }
   else
     return false;
 
+  if (TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2))
+    return false;
+
+  /* Verify that the machine can perform a widening multiply
+     accumulate in this mode/signedness combination, otherwise
+     this transformation is likely to pessimize code.  */
+  this_optab = optab_for_tree_code (wmult_code, type1, optab_default);
+  if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing)
+    return false;
+
   /* ??? May need some type verification here?  */
 
-  gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs1, mult_rhs2,
+  gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code,
+				    fold_convert (type1, mult_rhs1),
+				    fold_convert (type2, mult_rhs2),
 				    add_rhs);
   update_stmt (gsi_stmt (*gsi));
   return true;

Re: Extend widening_mul pass to handle fixed-point types

[Ramana Radhakrishnan]

[-- Attachment #1: Type: text/plain, Size: 6180 bytes --]

Hi Richard,


On Wed, 2010-07-28 at 21:15 +0100, Richard Sandiford wrote:
> Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> writes:
> > Hmmm there are regressions after my patch was applied. I had to do some
> > rebuilds because trunk appears to be broken for other reasons at the
> > minute for ARM which I shall investigate next. 
> 
> Yeah, I think it's the other way about: expand_widen_pattern_expr
> is using the right type and convert_plusminus_to_widen is using
> the wrong one.  The type we pass to optab_for_tree_code determines
> the _sign_ of the multiplication, not the size, so we want to pass
> in the unwidened type.

Thanks for looking at this. Ah - ok. I see what you mean here. 

> 
> > We weren't generating any widening operations before this patch (and I
> > suspect my 2 patches are just for fixing the ICE's and doing the right
> > thing). If it were just adding support for fixed point arithmetic then
> > this is just broken because it shouldn't have changed the way in  which
> > code was generated for normal integer operations.  
> 
> For the record, the patch wasn't just about adding fixed-point support.
> It was also about allowing multiply-accumulate to be used where no plain
> multiplication exists.  So the problem for ARM was that the use of the
> wrong type in convert_plusminus_to_widen was being hidden by the use of
> the right type in convert_mult_widen (i.e. the patch exposed a latent bug).

Thanks for the clarification , that makes more sense now.

> 
> Could you give the attached patch a try?  It also fixes a case where
> the code could create mixed sign-and-unsigned maccs (a bug that existed
> before my patch) and a botched call to is_widening_mult_p (a bug
> introduced by my patch, sorry).

Your patch didn't apply cleanly to pristine trunk or to 162431 and I had
to do a manual merge so I'm not sure if I got all parts of it. 

Just to be absolutely clear, you still need to check for
is_gimple_assign before using the stmt in gimple_assign_lhs in
is_widening_mult_p or do you expect that to get subsumed by your patch ?
Otherwise the compiler ICEs with this testcase [ice-on-gimple-phi.c]. 

Here's what I came up with that includes a manual merge of your patch
assuming I got all the hunks, and my original trivial hunk to
is_widening_mult_p which appeared to fix all the problems with simple
testcases on trunk with a cross-compiler and what I'm bootstrapping and
testing right now.


cheers
Ramana



gcc/
        * tree-ssa-math-opts.c (convert_plusminus_to_widen): Fix type
        used in the call to optab_for_tree_code.  Fix the second
        is_widening_mult_p call.  Check that both unwidened operands
        have the same sign.
	(is_widening_mult_p): Return false if stmt is not a GIMPLE_ASSIGN.



[1]

./xgcc -B`pwd` -S -O2 -mcpu=cortex-a9 ~/ice-on-gimple-phi.c 

assignment./home/ramrad01/ice-on-gimple-phi.c:57:1: internal compiler
error: gimple check: expected gimple_assign(error_mark), have
gimple_phi() in gimple_assign_lhs, at gimple.h:1724

	





> 
> Richard
> 
> 
> gcc/
> 	* tree-ssa-math-opts.c (convert_plusminus_to_widen): Fix type
> 	used in the call to optab_for_tree_code.  Fix the second
> 	is_widening_mult_p call.  Check that both unwidened operands
> 	have the same sign.
> 
> Index: gcc/tree-ssa-math-opts.c
> ===================================================================
> --- gcc/tree-ssa-math-opts.c	2010-07-28 21:09:59.000000000 +0100
> +++ gcc/tree-ssa-math-opts.c	2010-07-28 21:10:00.000000000 +0100
> @@ -1414,13 +1414,6 @@ convert_plusminus_to_widen (gimple_stmt_
>    else
>      wmult_code = WIDEN_MULT_PLUS_EXPR;
>  
> -  /* Verify that the machine can perform a widening multiply
> -     accumulate in this mode/signedness combination, otherwise
> -     this transformation is likely to pessimize code.  */
> -  this_optab = optab_for_tree_code (wmult_code, type, optab_default);
> -  if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing)
> -    return false;
> -
>    rhs1 = gimple_assign_rhs1 (stmt);
>    rhs2 = gimple_assign_rhs2 (stmt);
>  
> @@ -1447,37 +1440,49 @@ convert_plusminus_to_widen (gimple_stmt_
>        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,
> +      if (!is_widening_mult_p (rhs2_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);
> +      type1 = TREE_TYPE (mult_rhs1);
> +      type2 = TREE_TYPE (mult_rhs2);
>        add_rhs = rhs2;
>      }
>    else if (rhs2_code == WIDEN_MULT_EXPR)
>      {
>        mult_rhs1 = gimple_assign_rhs1 (rhs2_stmt);
>        mult_rhs2 = gimple_assign_rhs2 (rhs2_stmt);
> +      type1 = TREE_TYPE (mult_rhs1);
> +      type2 = TREE_TYPE (mult_rhs2);
>        add_rhs = rhs1;
>      }
>    else
>      return false;
>  
> +  if (TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2))
> +    return false;
> +
> +  /* Verify that the machine can perform a widening multiply
> +     accumulate in this mode/signedness combination, otherwise
> +     this transformation is likely to pessimize code.  */
> +  this_optab = optab_for_tree_code (wmult_code, type1, optab_default);
> +  if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing)
> +    return false;
> +
>    /* ??? May need some type verification here?  */
>  
> -  gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs1, mult_rhs2,
> +  gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code,
> +				    fold_convert (type1, mult_rhs1),
> +				    fold_convert (type2, mult_rhs2),
>  				    add_rhs);
>    update_stmt (gsi_stmt (*gsi));
>    return true;

[-- Attachment #2: pr45067-patch-take4.txt --]
[-- Type: text/x-patch, Size: 3023 bytes --]

Index: tree-ssa-math-opts.c
===================================================================
--- tree-ssa-math-opts.c	(revision 162431)
+++ tree-ssa-math-opts.c	(working copy)
@@ -1323,6 +1323,9 @@ is_widening_mult_p (gimple stmt,
 {
   tree type;
 
+  if (!is_gimple_assign (stmt))
+    return false;
+
   type = TREE_TYPE (gimple_assign_lhs (stmt));
   if (TREE_CODE (type) != INTEGER_TYPE
       && TREE_CODE (type) != FIXED_POINT_TYPE)
@@ -1414,13 +1417,6 @@ convert_plusminus_to_widen (gimple_stmt_
   else
     wmult_code = WIDEN_MULT_PLUS_EXPR;
 
-  /* Verify that the machine can perform a widening multiply
-     accumulate in this mode/signedness combination, otherwise
-     this transformation is likely to pessimize code.  */
-  this_optab = optab_for_tree_code (wmult_code, type, optab_default);
-  if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing)
-    return false;
-
   rhs1 = gimple_assign_rhs1 (stmt);
   rhs2 = gimple_assign_rhs2 (stmt);
 
@@ -1447,8 +1443,6 @@ convert_plusminus_to_widen (gimple_stmt_
       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)
@@ -1456,14 +1450,14 @@ convert_plusminus_to_widen (gimple_stmt_
       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);
+      type1 = TREE_TYPE (mult_rhs1);
+      type2 = TREE_TYPE (mult_rhs2);
       add_rhs = rhs2;
     }
   else if (rhs2_code == WIDEN_MULT_EXPR)
@@ -1471,13 +1465,27 @@ convert_plusminus_to_widen (gimple_stmt_
       mult_rhs1 = gimple_assign_rhs1 (rhs2_stmt);
       mult_rhs2 = gimple_assign_rhs2 (rhs2_stmt);
       add_rhs = rhs1;
+      type1 = TREE_TYPE (mult_rhs1);
+      type2 = TREE_TYPE (mult_rhs2);
     }
   else
     return false;
 
+  if (TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2))
+    return false;
+
+  /* Verify that the machine can perform a widening multiply
+     accumulate in this mode/signedness combination, otherwise
+     this transformation is likely to pessimize code.  */
+  this_optab = optab_for_tree_code (wmult_code, type1, optab_default);
+  if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing)
+    return false;
+
   /* ??? May need some type verification here?  */
 
-  gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs1, mult_rhs2,
+  gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code,
+				    fold_convert (type1, mult_rhs1),
+				    fold_convert (type2, mult_rhs2),
 				    add_rhs);
   update_stmt (gsi_stmt (*gsi));
   return true;

[-- Attachment #3: ice-on-gimple-phi.c --]
[-- Type: text/x-csrc, Size: 2320 bytes --]

/* ./cc1 -O2 -mcpu=cortex-a9 -quiet reduced.c   */
typedef union tree_node *tree;
enum tree_code {
ERROR_MARK,
};
enum tree_code_class {
  tcc_type,
};
extern const enum tree_code_class tree_code_type[];
struct tree_base {
  __extension__ enum tree_code code : 16;
  unsigned unsigned_flag : 1;
};
struct tree_type {
  unsigned int precision : 10;
};
union
                                                         tree_node {
  struct tree_base base;
  struct tree_type type;
};
enum integer_type_kind
{
  itk_int,
  itk_none
};
extern tree integer_types[itk_none];
typedef struct cpp_reader cpp_reader;
enum c_tree_index
{
    CTI_INTMAX_TYPE,
    CTI_MAX
};
extern tree c_global_trees[CTI_MAX];
static void builtin_define_constants (const char *, tree);
static void
builtin_define_stdint_macros (void)
{
  builtin_define_constants ("__INTMAX_C", c_global_trees[CTI_INTMAX_TYPE]);
}
void
c_cpp_builtins (cpp_reader *pfile)
{
  builtin_define_stdint_macros ();
}
static const char *
type_suffix (tree type)
{
  static const char *const suffixes[] = { "", "U", "L", "UL", "LL", "ULL" };
  int unsigned_suffix;
  int is_long;
  unsigned_suffix = (__extension__ ({ __typeof (type) const __t = (type); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1084, __FUNCTION__); __t; })->base.unsigned_flag);
  if ((__extension__ ({ __typeof (type) const __t = (type); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1085, __FUNCTION__); __t; })->type.precision) < (__extension__ ({ __typeof (integer_types[itk_int]) const __t = (integer_types[itk_int]); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1085, __FUNCTION__); __t; })->type.precision))
  return suffixes[is_long * 2 + unsigned_suffix];
}
static void
builtin_define_constants (const char *macro, tree type)
{
  const char *suffix;
  char *buf;
  suffix = type_suffix (type);
  if (suffix[0] == 0)
    {
      buf = (char *) __builtin_alloca(strlen (macro) + 6);
    }
}

Re: Extend widening_mul pass to handle fixed-point types

[Richard Sandiford]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> writes:
>> Hmmm there are regressions after my patch was applied. I had to do some
>> rebuilds because trunk appears to be broken for other reasons at the
>> minute for ARM which I shall investigate next. 
>
> Yeah, I think it's the other way about: expand_widen_pattern_expr
> is using the right type and convert_plusminus_to_widen is using
> the wrong one.  The type we pass to optab_for_tree_code determines
> the _sign_ of the multiplication, not the size, so we want to pass
> in the unwidened type.
>
>> We weren't generating any widening operations before this patch (and I
>> suspect my 2 patches are just for fixing the ICE's and doing the right
>> thing). If it were just adding support for fixed point arithmetic then
>> this is just broken because it shouldn't have changed the way in  which
>> code was generated for normal integer operations.  
>
> For the record, the patch wasn't just about adding fixed-point support.
> It was also about allowing multiply-accumulate to be used where no plain
> multiplication exists.  So the problem for ARM was that the use of the
> wrong type in convert_plusminus_to_widen was being hidden by the use of
> the right type in convert_mult_widen (i.e. the patch exposed a latent bug).
>
> Could you give the attached patch a try?  It also fixes a case where
> the code could create mixed sign-and-unsigned maccs (a bug that existed
> before my patch) and a botched call to is_widening_mult_p (a bug
> introduced by my patch, sorry).

Now bootstrapped & regression-tested on x86_64-linux-gnu.  Ramana
also confirms that it fixes the ICEs and incorrect arith-rand-ll.c
code on ARM.  OK to install?

Richard

> gcc/
> 	* tree-ssa-math-opts.c (convert_plusminus_to_widen): Fix type
> 	used in the call to optab_for_tree_code.  Fix the second
> 	is_widening_mult_p call.  Check that both unwidened operands
> 	have the same sign.
>
> Index: gcc/tree-ssa-math-opts.c
> ===================================================================
> --- gcc/tree-ssa-math-opts.c	2010-07-28 21:09:59.000000000 +0100
> +++ gcc/tree-ssa-math-opts.c	2010-07-28 21:10:00.000000000 +0100
> @@ -1414,13 +1414,6 @@ convert_plusminus_to_widen (gimple_stmt_
>    else
>      wmult_code = WIDEN_MULT_PLUS_EXPR;
>  
> -  /* Verify that the machine can perform a widening multiply
> -     accumulate in this mode/signedness combination, otherwise
> -     this transformation is likely to pessimize code.  */
> -  this_optab = optab_for_tree_code (wmult_code, type, optab_default);
> -  if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing)
> -    return false;
> -
>    rhs1 = gimple_assign_rhs1 (stmt);
>    rhs2 = gimple_assign_rhs2 (stmt);
>  
> @@ -1447,37 +1440,49 @@ convert_plusminus_to_widen (gimple_stmt_
>        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,
> +      if (!is_widening_mult_p (rhs2_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);
> +      type1 = TREE_TYPE (mult_rhs1);
> +      type2 = TREE_TYPE (mult_rhs2);
>        add_rhs = rhs2;
>      }
>    else if (rhs2_code == WIDEN_MULT_EXPR)
>      {
>        mult_rhs1 = gimple_assign_rhs1 (rhs2_stmt);
>        mult_rhs2 = gimple_assign_rhs2 (rhs2_stmt);
> +      type1 = TREE_TYPE (mult_rhs1);
> +      type2 = TREE_TYPE (mult_rhs2);
>        add_rhs = rhs1;
>      }
>    else
>      return false;
>  
> +  if (TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2))
> +    return false;
> +
> +  /* Verify that the machine can perform a widening multiply
> +     accumulate in this mode/signedness combination, otherwise
> +     this transformation is likely to pessimize code.  */
> +  this_optab = optab_for_tree_code (wmult_code, type1, optab_default);
> +  if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing)
> +    return false;
> +
>    /* ??? May need some type verification here?  */
>  
> -  gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs1, mult_rhs2,
> +  gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code,
> +				    fold_convert (type1, mult_rhs1),
> +				    fold_convert (type2, mult_rhs2),
>  				    add_rhs);
>    update_stmt (gsi_stmt (*gsi));
>    return true;

Re: Extend widening_mul pass to handle fixed-point types

[Richard Sandiford]

Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> writes:
>> Could you give the attached patch a try?  It also fixes a case where
>> the code could create mixed sign-and-unsigned maccs (a bug that existed
>> before my patch) and a botched call to is_widening_mult_p (a bug
>> introduced by my patch, sorry).
>
> Your patch didn't apply cleanly to pristine trunk or to 162431 and I had
> to do a manual merge so I'm not sure if I got all parts of it. 
>
> Just to be absolutely clear, you still need to check for
> is_gimple_assign before using the stmt in gimple_assign_lhs in
> is_widening_mult_p or do you expect that to get subsumed by your patch ?
> Otherwise the compiler ICEs with this testcase [ice-on-gimple-phi.c]. 

For the record -- since I accidentally took this off-list when I started
using the gmail web interface, sorry about that -- the problems Ramana
had applying the patch were due to some kind of mailer issue.  The ICE
in ice-on-gimple-phi.c was due to the s/rhs1_stmt/rhs2_stmt/ part being
missing from the manually-applied patch.

Richard

Re: Extend widening_mul pass to handle fixed-point types

[Bernd Schmidt]

On 07/31/2010 09:30 AM, Richard Sandiford wrote:
> Now bootstrapped & regression-tested on x86_64-linux-gnu.  Ramana
> also confirms that it fixes the ICEs and incorrect arith-rand-ll.c
> code on ARM.  OK to install?

I think this is OK.  While looking at it I've been somewhat confused by
the fact that we choose an optab based on the type of the narrow
operands, then choose the right entry in the optab based on the mode of
the wide operand... but that seems unrelated to your patches.

So, please install, and thanks.


Bernd