diff mbox

Fix wide-int multiplication (PR tree-optimization/65170)

Message ID 20150223210012.GL1746@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 23, 2015, 9 p.m. UTC 
Hi!

To represent 128-bit unsigned multiplication results of 64-bit unsigned
operands one sometimes needs 3 HWIs instead of 2.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk?

2015-02-23  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/65170
	* wide-int.cc (wi::mul_internal): For the umul_ppmm optimization,
	if val[1] < 0, clear also val[2] and return 3.

	* gcc.c-torture/execute/pr65170.c: New test.
	* gcc.dg/tree-ssa/vrp96.c: New test.


	Jakub

Comments

Richard Sandiford Feb. 23, 2015, 9:51 p.m. UTC | #1 
Jakub Jelinek <jakub@redhat.com> writes:
> Hi!
>
> To represent 128-bit unsigned multiplication results of 64-bit unsigned
> operands one sometimes needs 3 HWIs instead of 2.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk?
>
> 2015-02-23  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR tree-optimization/65170
> 	* wide-int.cc (wi::mul_internal): For the umul_ppmm optimization,
> 	if val[1] < 0, clear also val[2] and return 3.
>
> 	* gcc.c-torture/execute/pr65170.c: New test.
> 	* gcc.dg/tree-ssa/vrp96.c: New test.
>
> --- gcc/wide-int.cc.jj	2015-01-27 09:25:38.000000000 +0100
> +++ gcc/wide-int.cc	2015-02-23 15:43:20.303280261 +0100
> @@ -1297,6 +1297,11 @@ wi::mul_internal (HOST_WIDE_INT *val, co
>  	      return 1;
>  	    }
>  	  umul_ppmm (val[1], val[0], op1.ulow (), op2.ulow ());
> +	  if (val[1] < 0)
> +	    {
> +	      val[2] = 0;
> +	      return 3;
> +	    }

Think this needs to check "prec > HOST_BITS_PER_WIDE_INT * 2",
otherwise val[2] isn't relevant and might not exist.

Lookd good to me with that change, thanks.

Richard
diff mbox

Patch

--- gcc/wide-int.cc.jj	2015-01-27 09:25:38.000000000 +0100
+++ gcc/wide-int.cc	2015-02-23 15:43:20.303280261 +0100
@@ -1297,6 +1297,11 @@  wi::mul_internal (HOST_WIDE_INT *val, co
 	      return 1;
 	    }
 	  umul_ppmm (val[1], val[0], op1.ulow (), op2.ulow ());
+	  if (val[1] < 0)
+	    {
+	      val[2] = 0;
+	      return 3;
+	    }
 	  return 1 + (val[1] != 0 || val[0] < 0);
 	}
       /* Likewise if the output is a full single HWI, except that the
--- gcc/testsuite/gcc.c-torture/execute/pr65170.c.jj	2015-02-23 15:55:10.678713455 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr65170.c	2015-02-23 15:57:40.851272695 +0100
@@ -0,0 +1,27 @@ 
+/* PR tree-optimization/65170 */
+
+#ifdef __SIZEOF_INT128__
+typedef unsigned __int128 V;
+typedef unsigned long long int H;
+#else
+typedef unsigned long long int V;
+typedef unsigned int H;
+#endif
+
+__attribute__((noinline, noclone)) void
+foo (V b, V c)
+{
+  V a;
+  b &= (H) -1;
+  c &= (H) -1;
+  a = b * c;
+  if (a != 1)
+    __builtin_abort ();
+}
+
+int
+main ()
+{
+  foo (1, 1);
+  return 0;
+}
--- gcc/testsuite/gcc.dg/tree-ssa/vrp96.c.jj	2015-02-23 16:33:45.157112406 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/vrp96.c	2015-02-23 16:35:22.861527289 +0100
@@ -0,0 +1,53 @@ 
+/* PR tree-optimization/65170 */
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-O2 -fdump-tree-vrp1" } */
+
+typedef unsigned __int128 T;
+extern void link_error (void);
+extern void required_check (void);
+
+T
+foo (T b, T c)
+{
+  T a;
+  b &= 0xffffffffffffffffULL;
+  c &= 0xffffffffffffffffULL;
+  if (b < 7 || c < 7)
+    return 0;
+  a = b * c;
+  if (a < 49 || a > (((T) 0xfffffffffffffffeULL << 64) | 1))
+    link_error ();
+  return a;
+}
+
+T
+bar (T b, T c)
+{
+  T a;
+  b &= 0xffffffffffffffffULL;
+  c &= 0xffffffffffffffffULL;
+  if (b < 7 || c < 7)
+    return 0;
+  a = b * c;
+  if (a == 49)
+    required_check ();
+  return a;
+}
+
+T
+baz (T b, T c)
+{
+  T a;
+  b &= 0xffffffffffffffffULL;
+  c &= 0xffffffffffffffffULL;
+  if (b < 7 || c < 7)
+    return 0;
+  a = b * c;
+  if (a == (((T) 0xfffffffffffffffeULL << 64) | 1))
+    required_check ();
+  return a;
+}
+
+/* { dg-final { scan-tree-dump-not "link_error" "vrp1" } } */
+/* { dg-final { scan-tree-dump-times "required_check" 2 "vrp1" } } */
+/* { dg-final { cleanup-tree-dump "vrp1" } } */