rs6000: Fix signed integer overflows

bootstrap-ubsan on gcc112 shows a couple of signed integer overflows:

config/rs6000/constraints.md:143:33: runtime error: signed integer overflow: 9223372036854775807 + 32768 cannot be represented in type 'long int'
config/rs6000/predicates.md:396:22: runtime error: signed integer overflow: 9223372036854775807 + 2147516416 cannot be represented in type 'long int'
config/rs6000/predicates.md:856:11: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself
config/rs6000/predicates.md:862:12: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself
config/rs6000/predicates.md:865:11: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself
config/rs6000/predicates.md:868:12: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself
config/rs6000/predicates.md:914:11: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself
config/rs6000/predicates.md:917:12: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself
config/rs6000/predicates.md:940:11: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself
config/rs6000/predicates.md:946:12: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself
config/rs6000/predicates.md:949:11: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself
config/rs6000/predicates.md:955:12: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself

The fix was tested on powerpc64-unknown-linux-gnu.
OK for trunk?

Thank you.

2014-11-19  Markus Trippelsdorf  <markus@trippelsdorf.de>

	* config/rs6000/constraints.md: Avoid signed integer overflows.
	* config/rs6000/predicates.md: Likewise.

On Wed, Nov 19, 2014 at 04:24:25PM +0100, Markus Trippelsdorf wrote:
>  ;; Return 1 if op is a constant integer valid for addition
> @@ -827,7 +827,7 @@
>  (define_predicate "mask_operand"
>    (match_code "const_int")
>  {
> -  HOST_WIDE_INT c, lsb;
> +  unsigned HOST_WIDE_INT c, lsb;
>  
>    c = INTVAL (op);
>  
> @@ -872,7 +872,7 @@
>  (define_predicate "mask_operand_wrap"
>    (match_code "const_int")
>  {
> -  HOST_WIDE_INT c, lsb;
> +  unsigned HOST_WIDE_INT c, lsb;
>  
>    c = INTVAL (op);
>  
> @@ -897,7 +897,7 @@
>  (define_predicate "mask64_operand"
>    (match_code "const_int")
>  {
> -  HOST_WIDE_INT c, lsb;
> +  unsigned HOST_WIDE_INT c, lsb;
>  
>    c = INTVAL (op);
>  
> @@ -923,7 +923,7 @@
>  (define_predicate "mask64_2_operand"
>    (match_code "const_int")
>  {
> -  HOST_WIDE_INT c, lsb;
> +  unsigned HOST_WIDE_INT c, lsb;
>  
>    c = INTVAL (op);

Shouldn't you use UINTVAL then?

	Marek

On Wed, Nov 19, 2014 at 04:24:25PM +0100, Markus Trippelsdorf wrote:
> --- a/gcc/config/rs6000/constraints.md
> +++ b/gcc/config/rs6000/constraints.md
> @@ -140,7 +140,7 @@
>  (define_constraint "I"
>    "A signed 16-bit constant"
>    (and (match_code "const_int")
> -       (match_test "(unsigned HOST_WIDE_INT) (ival + 0x8000) < 0x10000")))
> +       (match_test "((unsigned HOST_WIDE_INT) ival + 0x8000) < 0x10000")))

Use IN_RANGE instead?

> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -392,8 +392,8 @@
>  ;; Return 1 if op is a constant integer valid for addition with addis, addi.
>  (define_predicate "add_cint_operand"
>    (and (match_code "const_int")
> -       (match_test "(unsigned HOST_WIDE_INT)
> -		      (INTVAL (op) + (mode == SImode ? 0x80000000 : 0x80008000))
> +       (match_test "((unsigned HOST_WIDE_INT) INTVAL (op)
> +		       + (mode == SImode ? 0x80000000 : 0x80008000))
>  		    < (unsigned HOST_WIDE_INT) 0x100000000ll")))

Same here.  Best readable if you make it two IN_RANGE conditions, one
for SImode and one for the other :-)

Rest looks fine.


Segher

On Wed, Nov 19, 2014 at 04:41:49PM +0100, Marek Polacek wrote:
> > -  HOST_WIDE_INT c, lsb;
> > +  unsigned HOST_WIDE_INT c, lsb;
> >  
> >    c = INTVAL (op);
> 
> Shouldn't you use UINTVAL then?

That doesn't really matter here.  It looks a bit cleaner, of course.

I wouldn't spend too much time on mask*operand, I'll replace them
wholesale if all plans fall in place...


Segher

On Wed, Nov 19, 2014 at 10:24 AM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> bootstrap-ubsan on gcc112 shows a couple of signed integer overflows:

> The fix was tested on powerpc64-unknown-linux-gnu.
> OK for trunk?
>
> Thank you.
>
> 2014-11-19  Markus Trippelsdorf  <markus@trippelsdorf.de>
>
>         * config/rs6000/constraints.md: Avoid signed integer overflows.
>         * config/rs6000/predicates.md: Likewise.

This is okay.  IN_RANGE is clearer, but you don't have to clean up comparison.

UINTVAL is prettier, but not necessary.  A lot of code in GCC is not
careful about that.

Thanks, David

On 2014.11.19 at 11:20 -0500, David Edelsohn wrote:
> On Wed, Nov 19, 2014 at 10:24 AM, Markus Trippelsdorf
> <markus@trippelsdorf.de> wrote:
> > bootstrap-ubsan on gcc112 shows a couple of signed integer overflows:
> 
> > The fix was tested on powerpc64-unknown-linux-gnu.
> > OK for trunk?
> >
> > Thank you.
> >
> > 2014-11-19  Markus Trippelsdorf  <markus@trippelsdorf.de>
> >
> >         * config/rs6000/constraints.md: Avoid signed integer overflows.
> >         * config/rs6000/predicates.md: Likewise.
> 
> This is okay.  IN_RANGE is clearer, but you don't have to clean up comparison.
> 
> UINTVAL is prettier, but not necessary.  A lot of code in GCC is not
> careful about that.

Thanks for all the comments. I've checked in the original patch, but feel free
to follow up with aesthetic improvements.