diff mbox

[(9/7)] Widening multiplies with constant inputs

Message ID 4E2967FC.3090101@codesourcery.com
State New
Headers show

Commit Message

Andrew Stubbs July 22, 2011, 12:07 p.m. UTC 
ENOPATCH ....

On 22/07/11 12:57, Andrew Stubbs wrote:
> On 21/07/11 14:22, Richard Guenther wrote:
>> On Thu, Jul 21, 2011 at 2:53 PM, Andrew Stubbs<ams@codesourcery.com>
>> wrote:
>>> This patch is part bug fix, part better optimization.
>>>
>>> Firstly, my initial patch series introduced a bug that caused an
>>> internal
>>> compiler error when the input to a multiply was a constant. This was
>>> caused
>>> by the gimple verification rejecting such things. I'm not totally
>>> clear how
>>> this ever worked, but I've corrected it by inserting a temporary
>>> SSA_NAME
>>> between the constant and the multiply.
>>
>> Huh? Constant operands should be perfectly fine. What was the error
>> you got?
>
> Ok, so it seems that the fold_convert we thought was redundant in patch
> 5 (now moved to patch 2) was in fact responsible for making constants
> the right type.
>
> I've rewritten it to use fold_convert to change the constant.
>
>>> I also discovered that widening multiply-and-accumulate operations
>>> were not
>>> recognised if any one of the three inputs were a constant. I've
>>> corrected
>>> this by adjusting the pattern matching. This also required inserting new
>>> SSA_NAMEs to make it work.
>>
>> See above.
>
> The pattern matching stuff remains the same, but the constant
> conversions have been updated.
>
>>> In order to insert the new SSA_NAME, I've simply reused the existing
>>> type
>>> conversion code - the only difference is that the conversion may be a
>>> no-op,
>>> so it just generates a straight forward assignment.
>>>
>>> OK?
>>
>> Nope. I suppose you forget to adjust the constants type? Just
>> fold-convert it before using it as input to a macc.
>
> Done.
>
> OK?
>
> Andrew
>

Comments

Richard Biener July 22, 2011, 12:17 p.m. UTC | #1 
On Fri, Jul 22, 2011 at 2:07 PM, Andrew Stubbs <ams@codesourcery.com> wrote:
> ENOPATCH ....
>
> On 22/07/11 12:57, Andrew Stubbs wrote:
>>
>> On 21/07/11 14:22, Richard Guenther wrote:
>>>
>>> On Thu, Jul 21, 2011 at 2:53 PM, Andrew Stubbs<ams@codesourcery.com>
>>> wrote:
>>>>
>>>> This patch is part bug fix, part better optimization.
>>>>
>>>> Firstly, my initial patch series introduced a bug that caused an
>>>> internal
>>>> compiler error when the input to a multiply was a constant. This was
>>>> caused
>>>> by the gimple verification rejecting such things. I'm not totally
>>>> clear how
>>>> this ever worked, but I've corrected it by inserting a temporary
>>>> SSA_NAME
>>>> between the constant and the multiply.
>>>
>>> Huh? Constant operands should be perfectly fine. What was the error
>>> you got?
>>
>> Ok, so it seems that the fold_convert we thought was redundant in patch
>> 5 (now moved to patch 2) was in fact responsible for making constants
>> the right type.
>>
>> I've rewritten it to use fold_convert to change the constant.
>>
>>>> I also discovered that widening multiply-and-accumulate operations
>>>> were not
>>>> recognised if any one of the three inputs were a constant. I've
>>>> corrected
>>>> this by adjusting the pattern matching. This also required inserting new
>>>> SSA_NAMEs to make it work.
>>>
>>> See above.
>>
>> The pattern matching stuff remains the same, but the constant
>> conversions have been updated.
>>
>>>> In order to insert the new SSA_NAME, I've simply reused the existing
>>>> type
>>>> conversion code - the only difference is that the conversion may be a
>>>> no-op,
>>>> so it just generates a straight forward assignment.
>>>>
>>>> OK?
>>>
>>> Nope. I suppose you forget to adjust the constants type? Just
>>> fold-convert it before using it as input to a macc.
>>
>> Done.
>>
>> OK?

Ok.

Thanks,
Richard.

>> Andrew
>>
>
>
diff mbox

Patch

2011-07-22  Andrew Stubbs  <ams@codesourcery.com>

	gcc/
	* tree-ssa-math-opts.c (is_widening_mult_rhs_p): Handle constants
	beyond conversions.
	(convert_mult_to_widen): Convert constant inputs to the right type.
	(convert_plusminus_to_widen): Don't automatically reject inputs that
	are not an SSA_NAME.
	Convert constant inputs to the right type.

	gcc/testsuite/
	* gcc.target/arm/wmul-11.c: New file.
	* gcc.target/arm/wmul-12.c: New file.
	* gcc.target/arm/wmul-13.c: New file.

--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/wmul-11.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-require-effective-target arm_dsp } */
+
+long long
+foo (int *b)
+{
+  return 10 * (long long)*b;
+}
+
+/* { dg-final { scan-assembler "smull" } } */
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/wmul-12.c
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-require-effective-target arm_dsp } */
+
+long long
+foo (int *b, int *c)
+{
+  int tmp = *b * *c;
+  return 10 + (long long)tmp;
+}
+
+/* { dg-final { scan-assembler "smlal" } } */
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/wmul-13.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-require-effective-target arm_dsp } */
+
+long long
+foo (int *a, int *b)
+{
+  return *a + (long long)*b * 10;
+}
+
+/* { dg-final { scan-assembler "smlal" } } */
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -1997,6 +1997,13 @@  is_widening_mult_rhs_p (tree type, tree rhs, tree *type_out,
 	  type1 = TREE_TYPE (rhs1);
 	}
 
+      if (TREE_CODE (rhs1) == INTEGER_CST)
+	{
+	  *new_rhs_out = rhs1;
+	  *type_out = NULL;
+	  return true;
+	}
+
       if (TREE_CODE (type1) != TREE_CODE (type)
 	  || TYPE_PRECISION (type1) * 2 > TYPE_PRECISION (type))
 	return false;
@@ -2170,6 +2177,12 @@  convert_mult_to_widen (gimple stmt, gimple_stmt_iterator *gsi)
       rhs2 = build_and_insert_cast (gsi, loc, tmp, rhs2);
     }
 
+  /* Handle constants.  */
+  if (TREE_CODE (rhs1) == INTEGER_CST)
+    rhs1 = fold_convert (type1, rhs1);
+  if (TREE_CODE (rhs2) == INTEGER_CST)
+    rhs2 = fold_convert (type2, rhs2);
+
   gimple_assign_set_rhs1 (stmt, rhs1);
   gimple_assign_set_rhs2 (stmt, rhs2);
   gimple_assign_set_rhs_code (stmt, WIDEN_MULT_EXPR);
@@ -2221,8 +2234,6 @@  convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt,
       if (is_gimple_assign (rhs1_stmt))
 	rhs1_code = gimple_assign_rhs_code (rhs1_stmt);
     }
-  else
-    return false;
 
   if (TREE_CODE (rhs2) == SSA_NAME)
     {
@@ -2230,8 +2241,6 @@  convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt,
       if (is_gimple_assign (rhs2_stmt))
 	rhs2_code = gimple_assign_rhs_code (rhs2_stmt);
     }
-  else
-    return false;
 
   /* Allow for one conversion statement between the multiply
      and addition/subtraction statement.  If there are more than
@@ -2379,6 +2388,12 @@  convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt,
     add_rhs = build_and_insert_cast (gsi, loc, create_tmp_var (type, NULL),
 				     add_rhs);
 
+  /* Handle constants.  */
+  if (TREE_CODE (mult_rhs1) == INTEGER_CST)
+    rhs1 = fold_convert (type1, mult_rhs1);
+  if (TREE_CODE (mult_rhs2) == INTEGER_CST)
+    rhs2 = fold_convert (type2, mult_rhs2);
+
   gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs1, mult_rhs2,
 				    add_rhs);
   update_stmt (gsi_stmt (*gsi));