Fix wi::lshift

Message ID alpine.DEB.2.02.1907100006060.30934@grove.saclay.inria.fr
State New
Headers show
Series
  • Fix wi::lshift
Related show

Commit Message

Marc Glisse July 9, 2019, 10:12 p.m.
Hello,

because of an other bug, __builtin_constant_p is ignored in some cases, 
and this bug in wide_int went unnoticed. I am fixing it by making it match 
more closely the comment above.

Bootstrap+regtest together with a fix for __builtin_constant_p on 
x86_64-pc-linux-gnu.

2019-07-11  Marc Glisse  <marc.glisse@inria.fr>

 	* wide-int.h (wi::lshift): Reject negative values for the fast path.

-- 
Marc Glisse

Comments

Richard Sandiford July 10, 2019, 11:03 a.m. | #1
Marc Glisse <marc.glisse@inria.fr> writes:
> Hello,

>

> because of an other bug, __builtin_constant_p is ignored in some cases, 

> and this bug in wide_int went unnoticed. I am fixing it by making it match 

> more closely the comment above.

>

> Bootstrap+regtest together with a fix for __builtin_constant_p on 

> x86_64-pc-linux-gnu.

>

> 2019-07-11  Marc Glisse  <marc.glisse@inria.fr>

>

>  	* wide-int.h (wi::lshift): Reject negative values for the fast path.

>

> Index: gcc/wide-int.h

> ===================================================================

> --- gcc/wide-int.h	(revision 273306)

> +++ gcc/wide-int.h	(working copy)

> @@ -3025,22 +3025,22 @@ wi::lshift (const T1 &x, const T2 &y)

>  	 handle the case where the shift value is constant and the

>  	 result is a single nonnegative HWI (meaning that we don't

>  	 need to worry about val[1]).  This is particularly common

>  	 for converting a byte count to a bit count.

> 

>  	 For variable-precision integers like wide_int, handle HWI

>  	 and sub-HWI integers inline.  */

>        if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT)

>  	  ? (STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT - 1)

>  	     && xi.len == 1

> -	     && xi.val[0] <= (HOST_WIDE_INT) ((unsigned HOST_WIDE_INT)

> -					      HOST_WIDE_INT_MAX >> shift))

> +	     && xi.val[0] >= 0

> +	     && xi.val[0] <= HOST_WIDE_INT_MAX >> shift)


Slightly prefer IN_RANGE (xi.val[0], 0, HOST_WIDE_INT_MAX >> shift)
for this, although the compiler should be smart enough to do that
transformation itself.

OK with that change, thanks.

The testing obviously shows that we don't need the UWHI cast
before the shift any more.  Maybe we didn't in 2013 either and
I was just being overcautious.

Richard

Patch

Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h	(revision 273306)
+++ gcc/wide-int.h	(working copy)
@@ -3025,22 +3025,22 @@  wi::lshift (const T1 &x, const T2 &y)
 	 handle the case where the shift value is constant and the
 	 result is a single nonnegative HWI (meaning that we don't
 	 need to worry about val[1]).  This is particularly common
 	 for converting a byte count to a bit count.
 
 	 For variable-precision integers like wide_int, handle HWI
 	 and sub-HWI integers inline.  */
       if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT)
 	  ? (STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT - 1)
 	     && xi.len == 1
-	     && xi.val[0] <= (HOST_WIDE_INT) ((unsigned HOST_WIDE_INT)
-					      HOST_WIDE_INT_MAX >> shift))
+	     && xi.val[0] >= 0
+	     && xi.val[0] <= HOST_WIDE_INT_MAX >> shift)
 	  : precision <= HOST_BITS_PER_WIDE_INT)
 	{
 	  val[0] = xi.ulow () << shift;
 	  result.set_len (1);
 	}
       else
 	result.set_len (lshift_large (val, xi.val, xi.len,
 				      precision, shift));
     }
   return result;