[PATCH] Fix __mulv[dt]i3 and expand_mul_overflow (PR target/82274)

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

An overflow isn't detected on multiplication of signed integer
with all ones in the upper half of bits and all zeros in the lower half
by itself, either with -ftrapv, or __builtin_mul_overflow{,_p},
or signed-integer-overflow sanitization.

For libgcc, we have various cases we handle earlier (e.g. when one or both
operands are properly sign-extended from half precision to the full
precision), and for other operands we mostly abort, except for some special
cases (when upper half is all 0s or all 1s, but the msb of lower half is
different from that).  When upper halves of both operands are all 1s,
we mishandle the above mentioned special case:
                      DWunion ww = {.ll = (UDWtype) (UWtype) uu.s.low
                                    * (UDWtype) (UWtype) vv.s.low};

                      ww.s.high -= uu.s.low;
                      ww.s.high -= vv.s.low;
                      if (__builtin_expect (ww.s.high >= 0, 1))
                        return ww.ll;
ww.ll will be 0 and so will be ww.s.high, so we return 0, but
the only case when multiplication result is 0 without overflow is when
either of the operands is 0 (but in that case it is properly sign-extended
from lower half and thus we handle it earlier).
So, either we could do if (__builtin_expect (ww.s.high >= 0 && ww.ll, 1))
or what the patch does (which is as expensive as that IMHO, but without
performing the multiplication in that case).

The expand_mul_overflow fix is similar, in that case we know that
either both operands have all upper half bits 1 and then 0 msb of lower half,
or both operands have all upper half bits 0 and then 1 msb of lower half.
In any case, we know that neither operand is 0, and thus if the result is
0, we certainly have overflow.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
release branches?

2017-09-22  Jakub Jelinek  <jakub@redhat.com>

	PR target/82274
	* internal-fn.c (expand_mul_overflow): If both operands have
	the same highpart of -1 or 0 and the topmost bit of lowpart
	is different, overflow is if res <= 0 rather than res < 0.

	* libgcc2.c (__mulvDI3): If both operands have
	the same highpart of -1 and the topmost bit of lowpart is 0,
	multiplication overflows even if both lowparts are 0.

	* gcc.dg/pr82274-1.c: New test.
	* gcc.dg/pr82274-2.c: New test.

--- gcc/internal-fn.c.jj	2017-09-01 09:25:44.000000000 +0200
+++ gcc/internal-fn.c	2017-09-22 11:10:13.901438868 +0200
@@ -1770,8 +1770,8 @@ expand_mul_overflow (location_t loc, tre
 		}
 
 	      /* At this point hipart{0,1} are both in [-1, 0].  If they are
-		 the same, overflow happened if res is negative, if they are
-		 different, overflow happened if res is positive.  */
+		 the same, overflow happened if res is non-positive, if they
+		 are different, overflow happened if res is positive.  */
 	      if (op0_sign != 1 && op1_sign != 1 && op0_sign != op1_sign)
 		emit_jump (hipart_different);
 	      else if (op0_sign == 1 || op1_sign == 1)
@@ -1779,7 +1779,7 @@ expand_mul_overflow (location_t loc, tre
 					 NULL_RTX, NULL, hipart_different,
 					 profile_probability::even ());
 
-	      do_compare_rtx_and_jump (res, const0_rtx, LT, false, mode,
+	      do_compare_rtx_and_jump (res, const0_rtx, LE, false, mode,
 				       NULL_RTX, NULL, do_error,
 				       profile_probability::very_unlikely ());
 	      emit_jump (done_label);
--- libgcc/libgcc2.c.jj	2017-01-01 12:45:50.000000000 +0100
+++ libgcc/libgcc2.c	2017-09-22 10:17:56.010757022 +0200
@@ -375,7 +375,8 @@ __mulvDI3 (DWtype u, DWtype v)
 		}
 	      else
 		{
-		  if (uu.s.high == (Wtype) -1 && vv.s.high == (Wtype) - 1)
+		  if ((uu.s.high & vv.s.high) == (Wtype) -1
+		      && (uu.s.low | vv.s.low) != 0)
 		    {
 		      DWunion ww = {.ll = (UDWtype) (UWtype) uu.s.low
 				    * (UDWtype) (UWtype) vv.s.low};
--- gcc/testsuite/gcc.dg/pr82274-1.c.jj	2017-09-22 09:58:06.016739604 +0200
+++ gcc/testsuite/gcc.dg/pr82274-1.c	2017-09-22 10:10:53.466071164 +0200
@@ -0,0 +1,16 @@
+/* PR target/82274 */
+/* { dg-do run } */
+/* { dg-shouldfail "trapv" } */
+/* { dg-options "-ftrapv" } */
+
+int
+main ()
+{
+#ifdef __SIZEOF_INT128__
+  volatile __int128 m = -(((__int128) 1) << (__CHAR_BIT__ * __SIZEOF_INT128__ / 2));
+#else
+  volatile long long m = -(1LL << (__CHAR_BIT__ * __SIZEOF_LONG_LONG__ / 2));
+#endif
+  m = m * m;
+  return 0;
+}
--- gcc/testsuite/gcc.dg/pr82274-2.c.jj	2017-09-22 11:17:59.534630946 +0200
+++ gcc/testsuite/gcc.dg/pr82274-2.c	2017-09-22 11:19:17.340661013 +0200
@@ -0,0 +1,26 @@
+/* PR target/82274 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+int
+main ()
+{
+#ifdef __SIZEOF_INT128__
+  __int128 m = -(((__int128) 1) << (__CHAR_BIT__ * __SIZEOF_INT128__ / 2));
+  volatile __int128 mv = m;
+  __int128 r;
+#else
+  long long m = -(1LL << (__CHAR_BIT__ * __SIZEOF_LONG_LONG__ / 2));
+  volatile long long mv = m;
+  long long r;
+#endif
+  if (!__builtin_mul_overflow (mv, mv, &r))
+    __builtin_abort ();
+  if (!__builtin_mul_overflow_p (mv, mv, r))
+    __builtin_abort ();
+  if (!__builtin_mul_overflow (m, m, &r))
+    __builtin_abort ();
+  if (!__builtin_mul_overflow_p (m, m, r))
+    __builtin_abort ();
+  return 0;
+}

	Jakub