Re: [PATCH] Handle overflow in dependence analysis lambda ops gracefully

[Richard Biener]

On Wed, 20 Jan 2021, Jakub Jelinek wrote:

> On Wed, Jan 20, 2021 at 12:29:23PM +0100, Richard Biener wrote:
> > +/* Compute the product of signed A and B and indicate in *OVERFLOW whether
> > +   that operation overflowed.  */
> > +
> > +inline HOST_WIDE_INT
> > +mul_hwi (HOST_WIDE_INT a, HOST_WIDE_INT b, bool *overflow)
> > +{
> > +  unsigned HOST_WIDE_INT result = a * (unsigned HOST_WIDE_INT)b;
> > +  if (a != 0 && (HOST_WIDE_INT)result / a != b)
> > +    *overflow = true;
> > +  else
> > +    *overflow = false;
> 
> This isn't sufficiently bulletproof.
> It should be
>   if ((a == -1 && b == HOST_WIDE_INT_MIN)
>       || (a != 0 && (HOST_WIDE_INT)result / a != b))
>     *overflow = true;
>   else
>     *overflow = false;
> or so.
> 
> And except that a == -1 && b == min checks my recent widening_mul changes
> should match that to __builtin_mul_overflow, so it will not be perfect code,
> but at least will not be unnecessarily slow on x86.

OK, fixed.  Guess we could also use __builtin_mul_overflow directly
if supported via a GCC_VERSION check.  Looks like it's present
since GCC 5 at least.  So sth like (incremental)

diff --git a/gcc/hwint.h b/gcc/hwint.h
index 53f4ed5dcad..6d2d491acfa 100644
--- a/gcc/hwint.h
+++ b/gcc/hwint.h
@@ -354,12 +354,19 @@ add_hwi (HOST_WIDE_INT a, HOST_WIDE_INT b, bool 
*overflow)
 inline HOST_WIDE_INT
 mul_hwi (HOST_WIDE_INT a, HOST_WIDE_INT b, bool *overflow)
 {
+#if GCC_VERSION < 5001
   unsigned HOST_WIDE_INT result = a * (unsigned HOST_WIDE_INT)b;
-  if (a != 0 && (HOST_WIDE_INT)result / a != b)
+  if ((a == -1 && b == HOST_WIDE_INT_MIN)
+      || (a != 0 && (HOST_WIDE_INT)result / a != b))
     *overflow = true;
   else
     *overflow = false;
   return result;
+#else
+  HOST_WIDE_INT result;
+  *overflow = __builtin_mul_overflow (a, b, &result);
+  return result;
+#endif
 }

for the add case we should match all of the function I guess.

Richard.