[wide-int] Fix aarch{32,64} builds

I decided to lump these together since the problems were the same.
There were some typos in the real_to_integer invocation, while changing:

	/* There must be no padding.  */
	if (!host_integerp (TYPE_SIZE (type), 1)
	    || (tree_low_cst (TYPE_SIZE (type), 1)
		!= count * GET_MODE_BITSIZE (*modep)))
	  return -1;

to:

	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
	    || (tree_to_uhwi (TYPE_SIZE (type))
		!= count * GET_MODE_BITSIZE (*modep)))
 	  return -1;

introduced a signed/unsigned warning.

Tested on aarch64-linux-gnueabi & arm-linux-gnueabi and applied as
obvious.

Thanks,
Richard

I always like patches that change host dependencies into more general code.

ok from my perspective.

kenny

On 11/02/2013 06:13 AM, Richard Sandiford wrote:
> I decided to lump these together since the problems were the same.
> There were some typos in the real_to_integer invocation, while changing:
>
> 	/* There must be no padding.  */
> 	if (!host_integerp (TYPE_SIZE (type), 1)
> 	    || (tree_low_cst (TYPE_SIZE (type), 1)
> 		!= count * GET_MODE_BITSIZE (*modep)))
> 	  return -1;
>
> to:
>
> 	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
> 	    || (tree_to_uhwi (TYPE_SIZE (type))
> 		!= count * GET_MODE_BITSIZE (*modep)))
>   	  return -1;
>
> introduced a signed/unsigned warning.
>
> Tested on aarch64-linux-gnueabi & arm-linux-gnueabi and applied as
> obvious.
>
> Thanks,
> Richard
>
>
> Index: gcc/config/aarch64/aarch64.c
> ===================================================================
> --- gcc/config/aarch64/aarch64.c	(revision 204311)
> +++ gcc/config/aarch64/aarch64.c	(working copy)
> @@ -6030,9 +6030,7 @@
>   		      - tree_to_uhwi (TYPE_MIN_VALUE (index)));
>   
>   	/* There must be no padding.  */
> -	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
> -	    || (tree_to_uhwi (TYPE_SIZE (type))
> -		!= count * GET_MODE_BITSIZE (*modep)))
> +	if (wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep)))
>   	  return -1;
>   
>   	return count;
> @@ -6060,9 +6058,7 @@
>   	  }
>   
>   	/* There must be no padding.  */
> -	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
> -	    || (tree_to_uhwi (TYPE_SIZE (type))
> -		!= count * GET_MODE_BITSIZE (*modep)))
> +	if (wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep)))
>   	  return -1;
>   
>   	return count;
> @@ -6092,9 +6088,7 @@
>   	  }
>   
>   	/* There must be no padding.  */
> -	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
> -	    || (tree_to_uhwi (TYPE_SIZE (type))
> -		!= count * GET_MODE_BITSIZE (*modep)))
> +	if (wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep)))
>   	  return -1;
>   
>   	return count;
> @@ -7433,7 +7427,7 @@
>     int exponent;
>     unsigned HOST_WIDE_INT mantissa, mask;
>     REAL_VALUE_TYPE r, m;
> -  bool &fail
> +  bool fail;
>   
>     if (!CONST_DOUBLE_P (x))
>       return false;
> @@ -7457,7 +7451,7 @@
>        WARNING: If we ever have a representation using more than 2 * H_W_I - 1
>        bits for the mantissa, this can fail (low bits will be lost).  */
>     real_ldexp (&m, &r, point_pos - exponent);
> -  w = real_to_integer (m, &fail, HOST_BITS_PER_WIDE_INT * 2);
> +  wide_int w = real_to_integer (&m, &fail, HOST_BITS_PER_WIDE_INT * 2);
>   
>     /* If the low part of the mantissa has bits set we cannot represent
>        the value.  */
> Index: gcc/config/arm/arm.c
> ===================================================================
> --- gcc/config/arm/arm.c	(revision 204311)
> +++ gcc/config/arm/arm.c	(working copy)
> @@ -4678,9 +4678,7 @@
>   		      - tree_to_uhwi (TYPE_MIN_VALUE (index)));
>   
>   	/* There must be no padding.  */
> -	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
> -	    || (tree_to_uhwi (TYPE_SIZE (type))
> -		!= count * GET_MODE_BITSIZE (*modep)))
> +	if (wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep)))
>   	  return -1;
>   
>   	return count;
> @@ -4708,9 +4706,7 @@
>   	  }
>   
>   	/* There must be no padding.  */
> -	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
> -	    || (tree_to_uhwi (TYPE_SIZE (type))
> -		!= count * GET_MODE_BITSIZE (*modep)))
> +	if (wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep)))
>   	  return -1;
>   
>   	return count;
> @@ -4740,9 +4736,7 @@
>   	  }
>   
>   	/* There must be no padding.  */
> -	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
> -	    || (tree_to_uhwi (TYPE_SIZE (type))
> -		!= count * GET_MODE_BITSIZE (*modep)))
> +	if (wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep)))
>   	  return -1;
>   
>   	return count;
> @@ -11299,7 +11293,7 @@
>        WARNING: If there's ever a VFP version which uses more than 2 * H_W_I - 1
>        bits for the mantissa, this may fail (low bits would be lost).  */
>     real_ldexp (&m, &r, point_pos - exponent);
> -  wide_int w = real_to_integer (m, &fail, HOST_BITS_PER_WIDE_INT * 2);
> +  wide_int w = real_to_integer (&m, &fail, HOST_BITS_PER_WIDE_INT * 2);
>     mantissa = w.elt (0);
>     mant_hi = w.elt (1);
>

On Nov 2, 2013, at 3:13 AM, Richard Sandiford <rdsandiford@googlemail.com> wrote:
> I decided to lump these together since the problems were the same.
> There were some typos in the real_to_integer invocation, while changing:
> 
> 	/* There must be no padding.  */
> 	if (!host_integerp (TYPE_SIZE (type), 1)
> 	    || (tree_low_cst (TYPE_SIZE (type), 1)
> 		!= count * GET_MODE_BITSIZE (*modep)))
> 	  return -1;
> 
> to:
> 
> 	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
> 	    || (tree_to_uhwi (TYPE_SIZE (type))
> 		!= count * GET_MODE_BITSIZE (*modep)))
> 	  return -1;
> 
> introduced a signed/unsigned warning.
> 
> Tested on aarch64-linux-gnueabi & arm-linux-gnueabi and applied as
> obvious.
> 
> Thanks,
> Richard
> 
> 
> Index: gcc/config/aarch64/aarch64.c
> ===================================================================
> --- gcc/config/aarch64/aarch64.c	(revision 204311)
> +++ gcc/config/aarch64/aarch64.c	(working copy)
> @@ -6030,9 +6030,7 @@
> 		      - tree_to_uhwi (TYPE_MIN_VALUE (index)));
> 
> 	/* There must be no padding.  */
> -	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
> -	    || (tree_to_uhwi (TYPE_SIZE (type))
> -		!= count * GET_MODE_BITSIZE (*modep)))
> +	if (wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep)))
> 	  return -1;

So, one of the review comments concerns this type of change.  The specific comment was from David on rs6000 point #5.

My (our) question is, doesn't Ada have non-INTEGER_CST TYPE_SIZE (type), and the old code had this type of check:

bool
tree_fits_uhwi_p (const_tree t)
{
  return (t != NULL_TREE
          && TREE_CODE (t) == INTEGER_CST
          && TREE_INT_CST_HIGH (t) == 0);
}

to ensure that things that are not INTEGER_CSTs return -1.  In the new code, won't this just call wi::ne_p, and die?

I'm not an Ada person, so, I don't know if my fears are founded, and I don't claim to know all the checks that happen in the callers before this point.

Mike Stump <mikestump@comcast.net> writes:
> On Nov 2, 2013, at 3:13 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> I decided to lump these together since the problems were the same.
>> There were some typos in the real_to_integer invocation, while changing:
>> 
>> 	/* There must be no padding.  */
>> 	if (!host_integerp (TYPE_SIZE (type), 1)
>> 	    || (tree_low_cst (TYPE_SIZE (type), 1)
>> 		!= count * GET_MODE_BITSIZE (*modep)))
>> 	  return -1;
>> 
>> to:
>> 
>> 	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
>> 	    || (tree_to_uhwi (TYPE_SIZE (type))
>> 		!= count * GET_MODE_BITSIZE (*modep)))
>> 	  return -1;
>> 
>> introduced a signed/unsigned warning.
>> 
>> Tested on aarch64-linux-gnueabi & arm-linux-gnueabi and applied as
>> obvious.
>> 
>> Thanks,
>> Richard
>> 
>> 
>> Index: gcc/config/aarch64/aarch64.c
>> ===================================================================
>> --- gcc/config/aarch64/aarch64.c	(revision 204311)
>> +++ gcc/config/aarch64/aarch64.c	(working copy)
>> @@ -6030,9 +6030,7 @@
>> 		      - tree_to_uhwi (TYPE_MIN_VALUE (index)));
>> 
>> 	/* There must be no padding.  */
>> -	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
>> -	    || (tree_to_uhwi (TYPE_SIZE (type))
>> -		!= count * GET_MODE_BITSIZE (*modep)))
>> +	if (wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep)))
>> 	  return -1;
>
> So, one of the review comments concerns this type of change.  The
> specific comment was from David on rs6000 point #5.
>
> My (our) question is, doesn't Ada have non-INTEGER_CST TYPE_SIZE
> (type), and the old code had this type of check:
>
> bool
> tree_fits_uhwi_p (const_tree t)
> {
>   return (t != NULL_TREE
>           && TREE_CODE (t) == INTEGER_CST
>           && TREE_INT_CST_HIGH (t) == 0);
> }
>
> to ensure that things that are not INTEGER_CSTs return -1.  In the new
> code, won't this just call wi::ne_p, and die?

Ah, sorry, hadn't realised that was possible here.  In that case I agree
we should keep the INTEGER_CST check.  I.e.:

	if (TREE_CODE (TYPE_SIZE (type)) != INTEGER_CST
            || wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep)))
 	  return -1;

Thanks,
Richard

> My (our) question is, doesn't Ada have non-INTEGER_CST TYPE_SIZE (type), and
> the old code had this type of check:
> 
> bool
> tree_fits_uhwi_p (const_tree t)
> {
>   return (t != NULL_TREE
>           && TREE_CODE (t) == INTEGER_CST
>           && TREE_INT_CST_HIGH (t) == 0);
> }
> 
> to ensure that things that are not INTEGER_CSTs return -1.  In the new code,
> won't this just call wi::ne_p, and die?

TYPE_SIZE (type) can be anything even in C:

int foo (int i, int j)
{
  char a[i *2 + j + 1];
  a [0] = 1;
}

(gdb) p debug_generic_expr(type->type_common.size)
(bitsizetype) (sizetype) (SAVE_EXPR <(i * 2 + j) + 1>) * 8