diff mbox

[(5/7)] Widening multiplies for mis-matched mode inputs

Message ID 4E11CE4F.1020102@codesourcery.com
State New
Headers show

Commit Message

Andrew Stubbs July 4, 2011, 2:29 p.m. UTC 
On 28/06/11 16:08, Andrew Stubbs wrote:
> On 23/06/11 15:41, Andrew Stubbs wrote:
>> This patch removes the restriction that the inputs to a widening
>> multiply must be of the same mode.
>>
>> It does this by extending the smaller of the two inputs to match the
>> larger; therefore, it remains the case that subsequent code (in the
>> expand pass, for example) can rely on the type of rhs1 being the input
>> type of the operation, and the gimple verification code is still valid.
>>
>> OK?
>
> This update fixes the testcase issue Janis highlighted.

And this one updates the context changed by my update to patch 3.

The content of the patch has not changed.

Andrew

Comments

Richard Biener July 7, 2011, 10:10 a.m. UTC | #1 
On Mon, Jul 4, 2011 at 4:29 PM, Andrew Stubbs <ams@codesourcery.com> wrote:
> On 28/06/11 16:08, Andrew Stubbs wrote:
>>
>> On 23/06/11 15:41, Andrew Stubbs wrote:
>>>
>>> This patch removes the restriction that the inputs to a widening
>>> multiply must be of the same mode.
>>>
>>> It does this by extending the smaller of the two inputs to match the
>>> larger; therefore, it remains the case that subsequent code (in the
>>> expand pass, for example) can rely on the type of rhs1 being the input
>>> type of the operation, and the gimple verification code is still valid.
>>>
>>> OK?
>>
>> This update fixes the testcase issue Janis highlighted.
>
> And this one updates the context changed by my update to patch 3.
>
> The content of the patch has not changed.

Similar to the previous patch

+  if (TYPE_MODE (type2) != from_mode)
+    {
+      type2 = lang_hooks.types.type_for_mode (from_mode,
+                                             TYPE_UNSIGNED (type2));

use build_nonstandard_integer_type.

+  if (cast1)
+    rhs1 = build_and_insert_cast (gsi, gimple_location (stmt),
+                                 create_tmp_var (type1, NULL), rhs1, type1);
+  if (cast2)
+    rhs2 = build_and_insert_cast (gsi, gimple_location (stmt),
+                                 create_tmp_var (type2, NULL), rhs2, type2);

and CSE create_tmp_var - at this point type1 and type2 should be
the same, right?  So I guess it would be a good place to assert
types_compatible_p (type1, type2).

   gimple_assign_set_rhs1 (stmt, fold_convert (type1, rhs1));
   gimple_assign_set_rhs2 (stmt, fold_convert (type2, rhs2));

and that's now seemingly redundant ... it should probably be
gimple_assign_set_rhs1 (stmt, rhs1);, no?  A conversion isn't
a valid rhs1/2.  Similar oddity in convert_plusminus_to_widen.

+  if (TYPE_MODE (type2) != TYPE_MODE (type1))
+    {
+      type2 = lang_hooks.types.type_for_mode (TYPE_MODE (type1),
+                                             TYPE_UNSIGNED (type2));
+      cast2 = true;
+    }
+
+  if (cast1)
+    mult_rhs1 = build_and_insert_cast (gsi, gimple_location (stmt),
+                                      create_tmp_var (type1, NULL),
+                                      mult_rhs1, type1);
+  if (cast2)
+    mult_rhs2 = build_and_insert_cast (gsi, gimple_location (stmt),
+                                      create_tmp_var (type2, NULL),
+                                      mult_rhs2, type2);

see above.

Thanks,
Richard.

> Andrew
>
diff mbox

Patch

2011-06-28  Andrew Stubbs  <ams@codesourcery.com>

	gcc/
	* tree-ssa-math-opts.c (is_widening_mult_p): Remove FIXME.
	Ensure the the larger type is the first operand.
	(convert_mult_to_widen): Insert cast if type2 is smaller than type1.
	(convert_plusminus_to_widen): Likewise.

	gcc/testsuite/
	* gcc.target/arm/wmul-7.c: New file.

--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/wmul-7.c
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=armv7-a" } */
+
+unsigned long long
+foo (unsigned long long a, unsigned char *b, unsigned short *c)
+{
+  return a + *b * *c;
+}
+
+/* { dg-final { scan-assembler "umlal" } } */
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -2051,9 +2051,17 @@  is_widening_mult_p (gimple stmt,
       *type2_out = *type1_out;
     }
 
-  /* FIXME: remove this restriction.  */
-  if (TYPE_PRECISION (*type1_out) != TYPE_PRECISION (*type2_out))
-    return false;
+  /* Ensure that the larger of the two operands comes first. */
+  if (TYPE_PRECISION (*type1_out) < TYPE_PRECISION (*type2_out))
+    {
+      tree tmp;
+      tmp = *type1_out;
+      *type1_out = *type2_out;
+      *type2_out = tmp;
+      tmp = *rhs1_out;
+      *rhs1_out = *rhs2_out;
+      *rhs2_out = tmp;
+    }
 
   return true;
 }
@@ -2069,6 +2077,7 @@  convert_mult_to_widen (gimple stmt, gimple_stmt_iterator *gsi)
   enum insn_code handler;
   enum machine_mode to_mode, from_mode;
   optab op;
+  int cast1 = false, cast2 = false;
 
   lhs = gimple_assign_lhs (stmt);
   type = TREE_TYPE (lhs);
@@ -2107,16 +2116,26 @@  convert_mult_to_widen (gimple stmt, gimple_stmt_iterator *gsi)
 	    return false;
 
 	  type1 = type2 = lang_hooks.types.type_for_mode (from_mode, 0);
-
-	  rhs1 = build_and_insert_cast (gsi, gimple_location (stmt),
-					create_tmp_var (type1, NULL), rhs1, type1);
-	  rhs2 = build_and_insert_cast (gsi, gimple_location (stmt),
-					create_tmp_var (type2, NULL), rhs2, type2);
+	  cast1 = cast2 = true;
 	}
       else
 	return false;
     }
 
+  if (TYPE_MODE (type2) != from_mode)
+    {
+      type2 = lang_hooks.types.type_for_mode (from_mode,
+					      TYPE_UNSIGNED (type2));
+      cast2 = true;
+    }
+
+  if (cast1)
+    rhs1 = build_and_insert_cast (gsi, gimple_location (stmt),
+				  create_tmp_var (type1, NULL), rhs1, type1);
+  if (cast2)
+    rhs2 = build_and_insert_cast (gsi, gimple_location (stmt),
+				  create_tmp_var (type2, NULL), rhs2, type2);
+
   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);
@@ -2215,6 +2234,7 @@  convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt,
   optab this_optab;
   enum tree_code wmult_code;
   enum insn_code handler;
+  int cast1 = false, cast2 = false;
 
   lhs = gimple_assign_lhs (stmt);
   type = TREE_TYPE (lhs);
@@ -2302,17 +2322,28 @@  convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt,
       if (GET_MODE_SIZE (mode) < GET_MODE_SIZE (TYPE_MODE (type)))
 	{
 	  type1 = type2 = lang_hooks.types.type_for_mode (mode, 0);
-	  mult_rhs1 = build_and_insert_cast (gsi, gimple_location (stmt),
-					     create_tmp_var (type1, NULL),
-					     mult_rhs1, type1);
-	  mult_rhs2 = build_and_insert_cast (gsi, gimple_location (stmt),
-					     create_tmp_var (type2, NULL),
-					     mult_rhs2, type2);
+	  cast1 = cast2 = true;
 	}
       else
 	return false;
     }
 
+  if (TYPE_MODE (type2) != TYPE_MODE (type1))
+    {
+      type2 = lang_hooks.types.type_for_mode (TYPE_MODE (type1),
+					      TYPE_UNSIGNED (type2));
+      cast2 = true;
+    }
+
+  if (cast1)
+    mult_rhs1 = build_and_insert_cast (gsi, gimple_location (stmt),
+				       create_tmp_var (type1, NULL),
+				       mult_rhs1, type1);
+  if (cast2)
+    mult_rhs2 = build_and_insert_cast (gsi, gimple_location (stmt),
+				       create_tmp_var (type2, NULL),
+				       mult_rhs2, type2);
+
   /* Verify that the convertions between the mult and the add doesn't do
      anything unexpected.  */
   if (!valid_types_for_madd_p (type1, type2, mult_rhs))