diff mbox

Fix pr50717: widening multiply bad code

Message ID 4E99FAB8.8030806@codesourcery.com
State New
Headers show

Commit Message

Andrew Stubbs Oct. 15, 2011, 9:27 p.m. UTC 
This patch fixes a bug in which the widening multiply-and-accumulate 
optimization failed to take the intermediate types into account.

The effect of this is that the compiler would do what the programmer 
expected to happen, rather than what the C standard requires to happen 
(in many cases), so obviously this needed fixing. :)

It still needs to optimize the cases where the optimization doesn't 
actually change anything (because it's known that overflow cannot 
occur), so I don't want to completely disallow extends between multiply 
and plus, and I believe this patch achieves this.

OK?

Andrew

Comments

Richard Biener Oct. 17, 2011, 10:43 a.m. UTC | #1 
On Sat, Oct 15, 2011 at 11:27 PM, Andrew Stubbs <ams@codesourcery.com> wrote:
> This patch fixes a bug in which the widening multiply-and-accumulate
> optimization failed to take the intermediate types into account.
>
> The effect of this is that the compiler would do what the programmer
> expected to happen, rather than what the C standard requires to happen (in
> many cases), so obviously this needed fixing. :)
>
> It still needs to optimize the cases where the optimization doesn't actually
> change anything (because it's known that overflow cannot occur), so I don't
> want to completely disallow extends between multiply and plus, and I believe
> this patch achieves this.
>
> OK?

Ok.

THanks,
Richard.

> Andrew
>
Andrew Stubbs Oct. 18, 2011, 7:58 p.m. UTC | #2 
On 17/10/11 11:43, Richard Guenther wrote:
> Ok.

Committed, thanks.

Andrew
diff mbox

Patch

2011-10-15  Andrew Stubbs  <ams@codesourcery.com>

	gcc/
	* tree-ssa-math-opts.c (is_widening_mult_p): Remove the 'type'
	parameter.  Calculate 'type' from stmt.
	(convert_mult_to_widen): Update call the is_widening_mult_p.
	(convert_plusminus_to_widen): Likewise.

	gcc/testsuite/
	* gcc.dg/pr50717-1.c: New file.
	* gcc.target/arm/wmul-12.c: Correct types.
	* gcc.target/arm/wmul-8.c: Correct types.

--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr50717-1.c
@@ -0,0 +1,26 @@ 
+/* PR tree-optimization/50717  */
+/* Ensure that widening multiply-and-accumulate is not used where integer
+   type promotion or users' casts should prevent it.  */
+
+/* { dg-options "-O2 -fdump-tree-widening_mul" } */
+
+long long
+f (unsigned int a, char b, long long c)
+{
+  return (a * b) + c;
+}
+
+int
+g (short a, short b, int c)
+{
+  return (short)(a * b) + c;
+}
+
+int
+h (char a, char b, int c)
+{
+  return (char)(a * b) + c;
+}
+
+/* { dg-final { scan-tree-dump-times "WIDEN_MULT_PLUS_EXPR" 0 "widening_mul" } } */
+/* { dg-final { cleanup-tree-dump "widening_mul" } } */
--- a/gcc/testsuite/gcc.target/arm/wmul-12.c
+++ b/gcc/testsuite/gcc.target/arm/wmul-12.c
@@ -5,8 +5,8 @@ 
 long long
 foo (int *b, int *c)
 {
-  int tmp = *b * *c;
-  return 10 + (long long)tmp;
+  long long tmp = (long long)*b * *c;
+  return 10 + tmp;
 }
 
 /* { dg-final { scan-assembler "smlal" } } */
--- a/gcc/testsuite/gcc.target/arm/wmul-8.c
+++ b/gcc/testsuite/gcc.target/arm/wmul-8.c
@@ -5,7 +5,7 @@ 
 long long
 foo (long long a, int *b, int *c)
 {
-  return a + *b * *c;
+  return a + (long long)*b * *c;
 }
 
 /* { dg-final { scan-assembler "smlal" } } */
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -2039,10 +2039,12 @@  is_widening_mult_rhs_p (tree type, tree rhs, tree *type_out,
    and *TYPE2_OUT would give the operands of the multiplication.  */
 
 static bool
-is_widening_mult_p (tree type, gimple stmt,
+is_widening_mult_p (gimple stmt,
 		    tree *type1_out, tree *rhs1_out,
 		    tree *type2_out, tree *rhs2_out)
 {
+  tree type = TREE_TYPE (gimple_assign_lhs (stmt));
+
   if (TREE_CODE (type) != INTEGER_TYPE
       && TREE_CODE (type) != FIXED_POINT_TYPE)
     return false;
@@ -2104,7 +2106,7 @@  convert_mult_to_widen (gimple stmt, gimple_stmt_iterator *gsi)
   if (TREE_CODE (type) != INTEGER_TYPE)
     return false;
 
-  if (!is_widening_mult_p (type, stmt, &type1, &rhs1, &type2, &rhs2))
+  if (!is_widening_mult_p (stmt, &type1, &rhs1, &type2, &rhs2))
     return false;
 
   to_mode = TYPE_MODE (type);
@@ -2281,7 +2283,7 @@  convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt,
   if (code == PLUS_EXPR
       && (rhs1_code == MULT_EXPR || rhs1_code == WIDEN_MULT_EXPR))
     {
-      if (!is_widening_mult_p (type, rhs1_stmt, &type1, &mult_rhs1,
+      if (!is_widening_mult_p (rhs1_stmt, &type1, &mult_rhs1,
 			       &type2, &mult_rhs2))
 	return false;
       add_rhs = rhs2;
@@ -2289,7 +2291,7 @@  convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt,
     }
   else if (rhs2_code == MULT_EXPR || rhs2_code == WIDEN_MULT_EXPR)
     {
-      if (!is_widening_mult_p (type, rhs2_stmt, &type1, &mult_rhs1,
+      if (!is_widening_mult_p (rhs2_stmt, &type1, &mult_rhs1,
 			       &type2, &mult_rhs2))
 	return false;
       add_rhs = rhs1;