public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [wide-int] small cleanup in wide-int.*
@ 2013-11-28 23:04 Kenneth Zadeck
  2013-11-29 11:58 ` Richard Biener
  2013-11-29 13:39 ` Richard Sandiford
  0 siblings, 2 replies; 25+ messages in thread
From: Kenneth Zadeck @ 2013-11-28 23:04 UTC (permalink / raw)
  To: Richard Sandiford, gcc-patches, Richard Biener, Mike Stump

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

This patch does three things in wide-int:

1) it cleans up some comments.
2) removes a small amount of trash.
3) it changes the max size of the wide int from being 4x of 
MAX_BITSIZE_MODE_ANY_INT to 2x +1.   This should improve large muls and 
divs as well as perhaps help with some cache behavior.

ok to commit

[-- Attachment #2: wide-int-cleanup.diff --]
[-- Type: text/x-patch, Size: 4476 bytes --]

Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h	(revision 205488)
+++ gcc/wide-int.h	(working copy)
@@ -55,10 +55,12 @@ along with GCC; see the file COPYING3.
 
      2) offset_int.  This is a fixed size representation that is
      guaranteed to be large enough to compute any bit or byte sized
-     address calculation on the target.  Currently the value is 64 + 4
-     bits rounded up to the next number even multiple of
+     address calculation on the target.  Currently the value is 64 + 3
+     + 1 bits rounded up to the next number even multiple of
      HOST_BITS_PER_WIDE_INT (but this can be changed when the first
-     port needs more than 64 bits for the size of a pointer).
+     port needs more than 64 bits for the size of a pointer).  The 3
+     bits allow the bits of byte to accessed, the 1 allows any
+     unsigned value to be converted to signed without overflowing.
 
      This flavor can be used for all address math on the target.  In
      this representation, the values are sign or zero extended based
@@ -112,11 +114,11 @@ along with GCC; see the file COPYING3.
    two, the default is the prefered representation.
 
    All three flavors of wide_int are represented as a vector of
-   HOST_WIDE_INTs.  The default and widest_int vectors contain enough elements
-   to hold a value of MAX_BITSIZE_MODE_ANY_INT bits.  offset_int contains only
-   enough elements to hold ADDR_MAX_PRECISION bits.  The values are stored
-   in the vector with the least significant HOST_BITS_PER_WIDE_INT bits
-   in element 0.
+   HOST_WIDE_INTs.  The default and widest_int vectors contain enough
+   elements to hold a value of MAX_BITSIZE_MODE_ANY_INT bits.
+   offset_int contains only enough elements to hold ADDR_MAX_PRECISION
+   bits.  The values are stored in the vector with the least
+   significant HOST_BITS_PER_WIDE_INT bits in element 0.
 
    The default wide_int contains three fields: the vector (VAL),
    the precision and a length (LEN).  The length is the number of HWIs
@@ -223,10 +225,6 @@ along with GCC; see the file COPYING3.
 #include "signop.h"
 #include "insn-modes.h"
 
-#if 0
-#define DEBUG_WIDE_INT
-#endif
-
 /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
    early examination of the target's mode file.  Thus it is safe that
    some small multiple of this number is easily larger than any number
@@ -235,8 +233,8 @@ along with GCC; see the file COPYING3.
    range of a multiply.  This code needs 2n + 2 bits.  */
 
 #define WIDE_INT_MAX_ELTS \
-  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
-   / HOST_BITS_PER_WIDE_INT)
+  (((2 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
+    / HOST_BITS_PER_WIDE_INT) + 1)
 
 /* This is the max size of any pointer on any machine.  It does not
    seem to be as easy to sniff this out of the machine description as
Index: gcc/wide-int.cc
===================================================================
--- gcc/wide-int.cc	(revision 205488)
+++ gcc/wide-int.cc	(working copy)
@@ -2090,59 +2090,5 @@ void gt_ggc_mx (widest_int *) { }
 void gt_pch_nx (widest_int *, void (*) (void *, void *), void *) { }
 void gt_pch_nx (widest_int *) { }
 
-/*
- * Private debug printing routines.
- */
-#ifdef DEBUG_WIDE_INT
-/* The debugging routines print results of wide operations into the
-   dump files of the respective passes in which they were called.  */
-static char *
-dumpa (const HOST_WIDE_INT *val, unsigned int len, unsigned int prec, char *buf)
-{
-  int i;
-  unsigned int l;
-  const char * sep = "";
 
-  l = sprintf (buf, "[%d (", prec);
-  for (i = len - 1; i >= 0; i--)
-    {
-      l += sprintf (&buf[l], "%s" HOST_WIDE_INT_PRINT_HEX, sep, val[i]);
-      sep = " ";
-    }
 
-  gcc_assert (len != 0);
-
-  l += sprintf (&buf[l], ")]");
-
-  gcc_assert (l < MAX_SIZE);
-  return buf;
-
-
-}
-#endif
-
-#if 0
-/* The debugging routines print results of wide operations into the
-   dump files of the respective passes in which they were called.  */
-char *
-wide_int_ro::dump (char* buf) const
-{
-  int i;
-  unsigned int l;
-  const char * sep = "";
-
-  l = sprintf (buf, "[%d (", precision);
-  for (i = len - 1; i >= 0; i--)
-    {
-      l += sprintf (&buf[l], "%s" HOST_WIDE_INT_PRINT_HEX, sep, val[i]);
-      sep = " ";
-    }
-
-  gcc_assert (len != 0);
-
-  l += sprintf (&buf[l], ")]");
-
-  gcc_assert (l < MAX_SIZE);
-  return buf;
-}
-#endif

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-11-28 23:04 [wide-int] small cleanup in wide-int.* Kenneth Zadeck
@ 2013-11-29 11:58 ` Richard Biener
  2013-11-29 12:51   ` Richard Sandiford
                     ` (3 more replies)
  2013-11-29 13:39 ` Richard Sandiford
  1 sibling, 4 replies; 25+ messages in thread
From: Richard Biener @ 2013-11-29 11:58 UTC (permalink / raw)
  To: Kenneth Zadeck; +Cc: Richard Sandiford, gcc-patches, Mike Stump

On Thu, Nov 28, 2013 at 6:11 PM, Kenneth Zadeck
<zadeck@naturalbridge.com> wrote:
> This patch does three things in wide-int:
>
> 1) it cleans up some comments.
> 2) removes a small amount of trash.
> 3) it changes the max size of the wide int from being 4x of
> MAX_BITSIZE_MODE_ANY_INT to 2x +1.   This should improve large muls and divs
> as well as perhaps help with some cache behavior.

@@ -235,8 +233,8 @@ along with GCC; see the file COPYING3.
    range of a multiply.  This code needs 2n + 2 bits.  */

 #define WIDE_INT_MAX_ELTS \
-  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
-   / HOST_BITS_PER_WIDE_INT)
+  (((2 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
+    / HOST_BITS_PER_WIDE_INT) + 1)

I always wondered why VRP (if that is the only reason we do 2*n+1)
cannot simply use FIXED_WIDE_INT(MAX_BITSIZE_MODE_ANY_INT*2 + 1)?
Other widest_int users should not suffer IMHO.  widest_int should
strictly cover all modes that the target can do any arithmetic on
(thus not XImode or OImode on x86_64).

Richard.

> ok to commit

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-11-29 11:58 ` Richard Biener
@ 2013-11-29 12:51   ` Richard Sandiford
  2013-11-29 13:23     ` Richard Biener
  2013-11-29 17:06   ` Kenneth Zadeck
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 25+ messages in thread
From: Richard Sandiford @ 2013-11-29 12:51 UTC (permalink / raw)
  To: Richard Biener; +Cc: Kenneth Zadeck, gcc-patches, Mike Stump

Richard Biener <richard.guenther@gmail.com> writes:
> On Thu, Nov 28, 2013 at 6:11 PM, Kenneth Zadeck
> <zadeck@naturalbridge.com> wrote:
>> This patch does three things in wide-int:
>>
>> 1) it cleans up some comments.
>> 2) removes a small amount of trash.
>> 3) it changes the max size of the wide int from being 4x of
>> MAX_BITSIZE_MODE_ANY_INT to 2x +1.   This should improve large muls and divs
>> as well as perhaps help with some cache behavior.
>
> @@ -235,8 +233,8 @@ along with GCC; see the file COPYING3.
>     range of a multiply.  This code needs 2n + 2 bits.  */
>
>  #define WIDE_INT_MAX_ELTS \
> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
> -   / HOST_BITS_PER_WIDE_INT)
> +  (((2 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
> +    / HOST_BITS_PER_WIDE_INT) + 1)
>
> I always wondered why VRP (if that is the only reason we do 2*n+1)
> cannot simply use FIXED_WIDE_INT(MAX_BITSIZE_MODE_ANY_INT*2 + 1)?
> Other widest_int users should not suffer IMHO.  widest_int should
> strictly cover all modes that the target can do any arithmetic on
> (thus not XImode or OImode on x86_64).

Do you want it to be 128 bits or 128+1 (rounded up)?  If we stick to 128
bits, we'll still have the problem that we like to see tree constants as
"infinite precision" while the integer we use to represent them cannot
distinguish between maximum-width signed and unsigned constants.
E.g. ~(uint64_t)0 is different from -(int64_t)1 but ~(uint128_t)0 is the
same as -(int128_t)1.  That's true for double_int too but you said a few
months back that you saw that as a bug.

By being able to represent 129 bits we're able to get rid of that corner
case (and consequently things like TREE_INT_CST_LT_UNSIGNED).

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-11-29 12:51   ` Richard Sandiford
@ 2013-11-29 13:23     ` Richard Biener
  0 siblings, 0 replies; 25+ messages in thread
From: Richard Biener @ 2013-11-29 13:23 UTC (permalink / raw)
  To: Richard Biener, Kenneth Zadeck, gcc-patches, Mike Stump,
	Richard Sandiford

On Fri, Nov 29, 2013 at 12:26 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Thu, Nov 28, 2013 at 6:11 PM, Kenneth Zadeck
>> <zadeck@naturalbridge.com> wrote:
>>> This patch does three things in wide-int:
>>>
>>> 1) it cleans up some comments.
>>> 2) removes a small amount of trash.
>>> 3) it changes the max size of the wide int from being 4x of
>>> MAX_BITSIZE_MODE_ANY_INT to 2x +1.   This should improve large muls and divs
>>> as well as perhaps help with some cache behavior.
>>
>> @@ -235,8 +233,8 @@ along with GCC; see the file COPYING3.
>>     range of a multiply.  This code needs 2n + 2 bits.  */
>>
>>  #define WIDE_INT_MAX_ELTS \
>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>> -   / HOST_BITS_PER_WIDE_INT)
>> +  (((2 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>>
>> I always wondered why VRP (if that is the only reason we do 2*n+1)
>> cannot simply use FIXED_WIDE_INT(MAX_BITSIZE_MODE_ANY_INT*2 + 1)?
>> Other widest_int users should not suffer IMHO.  widest_int should
>> strictly cover all modes that the target can do any arithmetic on
>> (thus not XImode or OImode on x86_64).
>
> Do you want it to be 128 bits or 128+1 (rounded up)?  If we stick to 128
> bits, we'll still have the problem that we like to see tree constants as
> "infinite precision" while the integer we use to represent them cannot
> distinguish between maximum-width signed and unsigned constants.
> E.g. ~(uint64_t)0 is different from -(int64_t)1 but ~(uint128_t)0 is the
> same as -(int128_t)1.  That's true for double_int too but you said a few
> months back that you saw that as a bug.
>
> By being able to represent 129 bits we're able to get rid of that corner
> case (and consequently things like TREE_INT_CST_LT_UNSIGNED).

Yeah, we probably need the +1 for that.

Richard.

> Thanks,
> Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-11-28 23:04 [wide-int] small cleanup in wide-int.* Kenneth Zadeck
  2013-11-29 11:58 ` Richard Biener
@ 2013-11-29 13:39 ` Richard Sandiford
  2013-11-29 19:32   ` Kenneth Zadeck
  1 sibling, 1 reply; 25+ messages in thread
From: Richard Sandiford @ 2013-11-29 13:39 UTC (permalink / raw)
  To: Kenneth Zadeck; +Cc: gcc-patches, Richard Biener, Mike Stump

Looks good to me FWIW, except:

Kenneth Zadeck <zadeck@naturalbridge.com> writes:
> @@ -112,11 +114,11 @@ along with GCC; see the file COPYING3.
>     two, the default is the prefered representation.
>  
>     All three flavors of wide_int are represented as a vector of
> -   HOST_WIDE_INTs.  The default and widest_int vectors contain enough elements
> -   to hold a value of MAX_BITSIZE_MODE_ANY_INT bits.  offset_int contains only
> -   enough elements to hold ADDR_MAX_PRECISION bits.  The values are stored
> -   in the vector with the least significant HOST_BITS_PER_WIDE_INT bits
> -   in element 0.
> +   HOST_WIDE_INTs.  The default and widest_int vectors contain enough
> +   elements to hold a value of MAX_BITSIZE_MODE_ANY_INT bits.
> +   offset_int contains only enough elements to hold ADDR_MAX_PRECISION
> +   bits.  The values are stored in the vector with the least
> +   significant HOST_BITS_PER_WIDE_INT bits in element 0.
>  
>     The default wide_int contains three fields: the vector (VAL),
>     the precision and a length (LEN).  The length is the number of HWIs

is this just changing the line breaks?

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-11-29 11:58 ` Richard Biener
  2013-11-29 12:51   ` Richard Sandiford
@ 2013-11-29 17:06   ` Kenneth Zadeck
  2013-11-30 18:06   ` Kenneth Zadeck
  2013-12-03 16:20   ` Kenneth Zadeck
  3 siblings, 0 replies; 25+ messages in thread
From: Kenneth Zadeck @ 2013-11-29 17:06 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Sandiford, gcc-patches, Mike Stump





On Nov 29, 2013, at 4:24 AM, Richard Biener <richard.guenther@gmail.com> wrote:

> On Thu, Nov 28, 2013 at 6:11 PM, Kenneth Zadeck
> <zadeck@naturalbridge.com> wrote:
>> This patch does three things in wide-int:
>> 
>> 1) it cleans up some comments.
>> 2) removes a small amount of trash.
>> 3) it changes the max size of the wide int from being 4x of
>> MAX_BITSIZE_MODE_ANY_INT to 2x +1.   This should improve large muls and divs
>> as well as perhaps help with some cache behavior.
> 
> @@ -235,8 +233,8 @@ along with GCC; see the file COPYING3.
>    range of a multiply.  This code needs 2n + 2 bits.  */
> 
> #define WIDE_INT_MAX_ELTS \
> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
> -   / HOST_BITS_PER_WIDE_INT)
> +  (((2 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
> +    / HOST_BITS_PER_WIDE_INT) + 1)
> 
> I always wondered why VRP (if that is the only reason we do 2*n+1)
> cannot simply use FIXED_WIDE_INT(MAX_BITSIZE_MODE_ANY_INT*2 + 1)?
> Other widest_int users should not suffer IMHO.  widest_int should
> strictly cover all modes that the target can do any arithmetic on
> (thus not XImode or OImode on x86_64).
> 
> Richard.

We did not used to be able to do that.  This code is much older.   So how do you detect if someone has a mode and does not do math it? I had thought that the only reason that people defined these mode and did not use them was because it took a long time to properly understand how broken gcc really is.   Anyway there is I call to mul-full that would need reworking.
> 
>> ok to commit

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-11-29 13:39 ` Richard Sandiford
@ 2013-11-29 19:32   ` Kenneth Zadeck
  0 siblings, 0 replies; 25+ messages in thread
From: Kenneth Zadeck @ 2013-11-29 19:32 UTC (permalink / raw)
  To: gcc-patches, Richard Biener, Mike Stump, rdsandiford

could be.
On 11/29/2013 06:57 AM, Richard Sandiford wrote:
> Looks good to me FWIW, except:
>
> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>> @@ -112,11 +114,11 @@ along with GCC; see the file COPYING3.
>>      two, the default is the prefered representation.
>>   
>>      All three flavors of wide_int are represented as a vector of
>> -   HOST_WIDE_INTs.  The default and widest_int vectors contain enough elements
>> -   to hold a value of MAX_BITSIZE_MODE_ANY_INT bits.  offset_int contains only
>> -   enough elements to hold ADDR_MAX_PRECISION bits.  The values are stored
>> -   in the vector with the least significant HOST_BITS_PER_WIDE_INT bits
>> -   in element 0.
>> +   HOST_WIDE_INTs.  The default and widest_int vectors contain enough
>> +   elements to hold a value of MAX_BITSIZE_MODE_ANY_INT bits.
>> +   offset_int contains only enough elements to hold ADDR_MAX_PRECISION
>> +   bits.  The values are stored in the vector with the least
>> +   significant HOST_BITS_PER_WIDE_INT bits in element 0.
>>   
>>      The default wide_int contains three fields: the vector (VAL),
>>      the precision and a length (LEN).  The length is the number of HWIs
> is this just changing the line breaks?
>
> Thanks,
> Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-11-29 11:58 ` Richard Biener
  2013-11-29 12:51   ` Richard Sandiford
  2013-11-29 17:06   ` Kenneth Zadeck
@ 2013-11-30 18:06   ` Kenneth Zadeck
  2013-12-02 10:50     ` Richard Biener
  2013-12-03 16:20   ` Kenneth Zadeck
  3 siblings, 1 reply; 25+ messages in thread
From: Kenneth Zadeck @ 2013-11-30 18:06 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Sandiford, gcc-patches, Mike Stump

[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]

Richi,

this is the first of either 2 or 3 patches to fix this.    There are two 
places that need be fixed for us to do 1X + 1 and this patch fixes the 
first one.   There was an unnecessary call to mul_full and this was the 
only call to mul_full.   So this patch removes the call and also the 
function itself.

The other place is the tree-vpn that is discussed here and will be dealt 
with in the other patches.

tested on x86-64.

Ok to commit?

Kenny


On 11/29/2013 05:24 AM, Richard Biener wrote:
> On Thu, Nov 28, 2013 at 6:11 PM, Kenneth Zadeck
> <zadeck@naturalbridge.com> wrote:
>> This patch does three things in wide-int:
>>
>> 1) it cleans up some comments.
>> 2) removes a small amount of trash.
>> 3) it changes the max size of the wide int from being 4x of
>> MAX_BITSIZE_MODE_ANY_INT to 2x +1.   This should improve large muls and divs
>> as well as perhaps help with some cache behavior.
> @@ -235,8 +233,8 @@ along with GCC; see the file COPYING3.
>      range of a multiply.  This code needs 2n + 2 bits.  */
>
>   #define WIDE_INT_MAX_ELTS \
> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
> -   / HOST_BITS_PER_WIDE_INT)
> +  (((2 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
> +    / HOST_BITS_PER_WIDE_INT) + 1)
>
> I always wondered why VRP (if that is the only reason we do 2*n+1)
> cannot simply use FIXED_WIDE_INT(MAX_BITSIZE_MODE_ANY_INT*2 + 1)?
> Other widest_int users should not suffer IMHO.  widest_int should
> strictly cover all modes that the target can do any arithmetic on
> (thus not XImode or OImode on x86_64).
>
> Richard.
>
>> ok to commit


[-- Attachment #2: RemoveMulFull.diff --]
[-- Type: text/x-patch, Size: 5964 bytes --]

Index: fold-const.c
===================================================================
--- fold-const.c	(revision 205496)
+++ fold-const.c	(working copy)
@@ -5962,11 +5962,12 @@ extract_muldiv_1 (tree t, tree c, enum t
 	 assuming no overflow.  */
       if (tcode == code)
 	{
-	  bool overflow_p;
+	  bool overflow_p = false;
+	  bool overflow_mul_p;
 	  signop sign = TYPE_SIGN (ctype);
-	  wide_int mul = wi::mul_full (op1, c, sign);
+	  wide_int mul = wi::mul (op1, c, sign, &overflow_mul_p);
 	  overflow_p = TREE_OVERFLOW (c) | TREE_OVERFLOW (op1);
-	  if (!wi::fits_to_tree_p (mul, ctype)
+	  if (overflow_mul_p
 	      && ((sign == UNSIGNED && tcode != MULT_EXPR) || sign == SIGNED))
 	    overflow_p = true;
 	  if (!overflow_p)
Index: wide-int.cc
===================================================================
--- wide-int.cc	(revision 205508)
+++ wide-int.cc	(working copy)
@@ -1247,22 +1247,18 @@ wi_pack (unsigned HOST_WIDE_INT *result,
 }
 
 /* Multiply Op1 by Op2.  If HIGH is set, only the upper half of the
-   result is returned.  If FULL is set, the entire result is returned
-   in a mode that is twice the width of the inputs.  However, that
-   mode needs to exist if the value is to be usable.  Clients that use
-   FULL need to check for this.
-
-   If HIGH or FULL are not set, throw away the upper half after the
-   check is made to see if it overflows.  Unfortunately there is no
-   better way to check for overflow than to do this.  If OVERFLOW is
-   nonnull, record in *OVERFLOW whether the result overflowed.  SGN
-   controls the signedness and is used to check overflow or if HIGH or
-   FULL is set.  */
+   result is returned.  
+
+   If HIGH is not set, throw away the upper half after the check is
+   made to see if it overflows.  Unfortunately there is no better way
+   to check for overflow than to do this.  If OVERFLOW is nonnull,
+   record in *OVERFLOW whether the result overflowed.  SGN controls
+   the signedness and is used to check overflow or if HIGH is set.  */
 unsigned int
 wi::mul_internal (HOST_WIDE_INT *val, const HOST_WIDE_INT *op1,
 		  unsigned int op1len, const HOST_WIDE_INT *op2,
 		  unsigned int op2len, unsigned int prec, signop sgn,
-		  bool *overflow, bool high, bool full)
+		  bool *overflow, bool high)
 {
   unsigned HOST_WIDE_INT o0, o1, k, t;
   unsigned int i;
@@ -1292,7 +1288,7 @@ wi::mul_internal (HOST_WIDE_INT *val, co
   /* If we need to check for overflow, we can only do half wide
      multiplies quickly because we need to look at the top bits to
      check for the overflow.  */
-  if ((high || full || needs_overflow)
+  if ((high || needs_overflow)
       && (prec <= HOST_BITS_PER_HALF_WIDE_INT))
     {
       unsigned HOST_WIDE_INT r;
@@ -1351,7 +1347,7 @@ wi::mul_internal (HOST_WIDE_INT *val, co
 
   /* We did unsigned math above.  For signed we must adjust the
      product (assuming we need to see that).  */
-  if (sgn == SIGNED && (full || high || needs_overflow))
+  if (sgn == SIGNED && (high || needs_overflow))
     {
       unsigned HOST_WIDE_INT b;
       if (op1[op1len-1] < 0)
@@ -1399,13 +1395,7 @@ wi::mul_internal (HOST_WIDE_INT *val, co
 	  *overflow = true;
     }
 
-  if (full)
-    {
-      /* compute [2prec] <- [prec] * [prec] */
-      wi_pack ((unsigned HOST_WIDE_INT *) val, r, 2 * half_blocks_needed);
-      return canonize (val, blocks_needed * 2, prec * 2);
-    }
-  else if (high)
+  if (high)
     {
       /* compute [prec] <- ([prec] * [prec]) >> [prec] */
       wi_pack ((unsigned HOST_WIDE_INT *) val,
Index: wide-int.h
===================================================================
--- wide-int.h	(revision 205496)
+++ wide-int.h	(working copy)
@@ -1566,7 +1566,7 @@ namespace wi
   unsigned int mul_internal (HOST_WIDE_INT *, const HOST_WIDE_INT *,
 			     unsigned int, const HOST_WIDE_INT *,
 			     unsigned int, unsigned int, signop, bool *,
-			     bool, bool);
+			     bool);
   unsigned int divmod_internal (HOST_WIDE_INT *, unsigned int *,
 				HOST_WIDE_INT *, const HOST_WIDE_INT *,
 				unsigned int, unsigned int,
@@ -2308,7 +2308,7 @@ wi::mul (const T1 &x, const T2 &y)
     }
   else
     result.set_len (mul_internal (val, xi.val, xi.len, yi.val, yi.len,
-				  precision, UNSIGNED, 0, false, false));
+				  precision, UNSIGNED, 0, false));
   return result;
 }
 
@@ -2324,7 +2324,7 @@ wi::mul (const T1 &x, const T2 &y, signo
   WIDE_INT_REF_FOR (T2) yi (y, precision);
   result.set_len (mul_internal (val, xi.val, xi.len,
 				yi.val, yi.len, precision,
-				sgn, overflow, false, false));
+				sgn, overflow, false));
   return result;
 }
 
@@ -2358,7 +2358,7 @@ wi::mul_high (const T1 &x, const T2 &y,
   WIDE_INT_REF_FOR (T2) yi (y, precision);
   result.set_len (mul_internal (val, xi.val, xi.len,
 				yi.val, yi.len, precision,
-				sgn, 0, true, false));
+				sgn, 0, true));
   return result;
 }
 
@@ -2924,8 +2924,6 @@ namespace wi
   wide_int max_value (never_used1 *);
   wide_int max_value (never_used2 *);
 
-  wide_int mul_full (const wide_int_ref &, const wide_int_ref &, signop);
-
   /* FIXME: this is target dependent, so should be elsewhere.
      It also seems to assume that CHAR_BIT == BITS_PER_UNIT.  */
   wide_int from_buffer (const unsigned char *, unsigned int);
@@ -2956,19 +2954,6 @@ namespace wi
 			   unsigned int, unsigned int, bool);
 }
 
-/* Perform a widening multiplication of X and Y, extending the values
-   according according to SGN.  */
-inline wide_int
-wi::mul_full (const wide_int_ref &x, const wide_int_ref &y, signop sgn)
-{
-  gcc_checking_assert (x.precision == y.precision);
-  wide_int result = wide_int::create (x.precision * 2);
-  result.set_len (mul_internal (result.write_val (), x.val, x.len,
-				y.val, y.len, x.precision,
-				sgn, 0, false, true));
-  return result;
-}
-
 /* Return a PRECISION-bit integer in which the low WIDTH bits are set
    and the other bits are clear, or the inverse if NEGATE_P.  */
 inline wide_int

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-11-30 18:06   ` Kenneth Zadeck
@ 2013-12-02 10:50     ` Richard Biener
  2013-12-02 19:57       ` Kenneth Zadeck
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Biener @ 2013-12-02 10:50 UTC (permalink / raw)
  To: Kenneth Zadeck; +Cc: Richard Sandiford, gcc-patches, Mike Stump

On Sat, Nov 30, 2013 at 1:55 AM, Kenneth Zadeck
<zadeck@naturalbridge.com> wrote:
> Richi,
>
> this is the first of either 2 or 3 patches to fix this.    There are two
> places that need be fixed for us to do 1X + 1 and this patch fixes the first
> one.   There was an unnecessary call to mul_full and this was the only call
> to mul_full.   So this patch removes the call and also the function itself.
>
> The other place is the tree-vpn that is discussed here and will be dealt
> with in the other patches.
>
> tested on x86-64.
>
> Ok to commit?

Ok.

Thanks,
Richard.

> Kenny
>
>
>
> On 11/29/2013 05:24 AM, Richard Biener wrote:
>>
>> On Thu, Nov 28, 2013 at 6:11 PM, Kenneth Zadeck
>> <zadeck@naturalbridge.com> wrote:
>>>
>>> This patch does three things in wide-int:
>>>
>>> 1) it cleans up some comments.
>>> 2) removes a small amount of trash.
>>> 3) it changes the max size of the wide int from being 4x of
>>> MAX_BITSIZE_MODE_ANY_INT to 2x +1.   This should improve large muls and
>>> divs
>>> as well as perhaps help with some cache behavior.
>>
>> @@ -235,8 +233,8 @@ along with GCC; see the file COPYING3.
>>      range of a multiply.  This code needs 2n + 2 bits.  */
>>
>>   #define WIDE_INT_MAX_ELTS \
>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>> -   / HOST_BITS_PER_WIDE_INT)
>> +  (((2 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>>
>> I always wondered why VRP (if that is the only reason we do 2*n+1)
>> cannot simply use FIXED_WIDE_INT(MAX_BITSIZE_MODE_ANY_INT*2 + 1)?
>> Other widest_int users should not suffer IMHO.  widest_int should
>> strictly cover all modes that the target can do any arithmetic on
>> (thus not XImode or OImode on x86_64).
>>
>> Richard.
>>
>>> ok to commit
>
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-02 10:50     ` Richard Biener
@ 2013-12-02 19:57       ` Kenneth Zadeck
  0 siblings, 0 replies; 25+ messages in thread
From: Kenneth Zadeck @ 2013-12-02 19:57 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Sandiford, gcc-patches, Mike Stump

[-- Attachment #1: Type: text/plain, Size: 1946 bytes --]

committed as revision 205599 to wide-int branch.

kenny

On 12/02/2013 05:50 AM, Richard Biener wrote:
> On Sat, Nov 30, 2013 at 1:55 AM, Kenneth Zadeck
> <zadeck@naturalbridge.com> wrote:
>> Richi,
>>
>> this is the first of either 2 or 3 patches to fix this.    There are two
>> places that need be fixed for us to do 1X + 1 and this patch fixes the first
>> one.   There was an unnecessary call to mul_full and this was the only call
>> to mul_full.   So this patch removes the call and also the function itself.
>>
>> The other place is the tree-vpn that is discussed here and will be dealt
>> with in the other patches.
>>
>> tested on x86-64.
>>
>> Ok to commit?
> Ok.
>
> Thanks,
> Richard.
>
>> Kenny
>>
>>
>>
>> On 11/29/2013 05:24 AM, Richard Biener wrote:
>>> On Thu, Nov 28, 2013 at 6:11 PM, Kenneth Zadeck
>>> <zadeck@naturalbridge.com> wrote:
>>>> This patch does three things in wide-int:
>>>>
>>>> 1) it cleans up some comments.
>>>> 2) removes a small amount of trash.
>>>> 3) it changes the max size of the wide int from being 4x of
>>>> MAX_BITSIZE_MODE_ANY_INT to 2x +1.   This should improve large muls and
>>>> divs
>>>> as well as perhaps help with some cache behavior.
>>> @@ -235,8 +233,8 @@ along with GCC; see the file COPYING3.
>>>       range of a multiply.  This code needs 2n + 2 bits.  */
>>>
>>>    #define WIDE_INT_MAX_ELTS \
>>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>> -   / HOST_BITS_PER_WIDE_INT)
>>> +  (((2 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>>>
>>> I always wondered why VRP (if that is the only reason we do 2*n+1)
>>> cannot simply use FIXED_WIDE_INT(MAX_BITSIZE_MODE_ANY_INT*2 + 1)?
>>> Other widest_int users should not suffer IMHO.  widest_int should
>>> strictly cover all modes that the target can do any arithmetic on
>>> (thus not XImode or OImode on x86_64).
>>>
>>> Richard.
>>>
>>>> ok to commit
>>


[-- Attachment #2: removeMulHigh.diff --]
[-- Type: text/x-patch, Size: 6000 bytes --]

Index: gcc/wide-int.cc
===================================================================
--- gcc/wide-int.cc	(revision 205597)
+++ gcc/wide-int.cc	(working copy)
@@ -1247,22 +1247,18 @@ wi_pack (unsigned HOST_WIDE_INT *result,
 }
 
 /* Multiply Op1 by Op2.  If HIGH is set, only the upper half of the
-   result is returned.  If FULL is set, the entire result is returned
-   in a mode that is twice the width of the inputs.  However, that
-   mode needs to exist if the value is to be usable.  Clients that use
-   FULL need to check for this.
-
-   If HIGH or FULL are not set, throw away the upper half after the
-   check is made to see if it overflows.  Unfortunately there is no
-   better way to check for overflow than to do this.  If OVERFLOW is
-   nonnull, record in *OVERFLOW whether the result overflowed.  SGN
-   controls the signedness and is used to check overflow or if HIGH or
-   FULL is set.  */
+   result is returned.  
+
+   If HIGH is not set, throw away the upper half after the check is
+   made to see if it overflows.  Unfortunately there is no better way
+   to check for overflow than to do this.  If OVERFLOW is nonnull,
+   record in *OVERFLOW whether the result overflowed.  SGN controls
+   the signedness and is used to check overflow or if HIGH is set.  */
 unsigned int
 wi::mul_internal (HOST_WIDE_INT *val, const HOST_WIDE_INT *op1,
 		  unsigned int op1len, const HOST_WIDE_INT *op2,
 		  unsigned int op2len, unsigned int prec, signop sgn,
-		  bool *overflow, bool high, bool full)
+		  bool *overflow, bool high)
 {
   unsigned HOST_WIDE_INT o0, o1, k, t;
   unsigned int i;
@@ -1313,7 +1309,7 @@ wi::mul_internal (HOST_WIDE_INT *val, co
   /* If we need to check for overflow, we can only do half wide
      multiplies quickly because we need to look at the top bits to
      check for the overflow.  */
-  if ((high || full || needs_overflow)
+  if ((high || needs_overflow)
       && (prec <= HOST_BITS_PER_HALF_WIDE_INT))
     {
       unsigned HOST_WIDE_INT r;
@@ -1372,7 +1368,7 @@ wi::mul_internal (HOST_WIDE_INT *val, co
 
   /* We did unsigned math above.  For signed we must adjust the
      product (assuming we need to see that).  */
-  if (sgn == SIGNED && (full || high || needs_overflow))
+  if (sgn == SIGNED && (high || needs_overflow))
     {
       unsigned HOST_WIDE_INT b;
       if (op1[op1len-1] < 0)
@@ -1420,13 +1416,7 @@ wi::mul_internal (HOST_WIDE_INT *val, co
 	  *overflow = true;
     }
 
-  if (full)
-    {
-      /* compute [2prec] <- [prec] * [prec] */
-      wi_pack ((unsigned HOST_WIDE_INT *) val, r, 2 * half_blocks_needed);
-      return canonize (val, blocks_needed * 2, prec * 2);
-    }
-  else if (high)
+  if (high)
     {
       /* compute [prec] <- ([prec] * [prec]) >> [prec] */
       wi_pack ((unsigned HOST_WIDE_INT *) val,
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 205597)
+++ gcc/fold-const.c	(working copy)
@@ -5962,11 +5962,12 @@ extract_muldiv_1 (tree t, tree c, enum t
 	 assuming no overflow.  */
       if (tcode == code)
 	{
-	  bool overflow_p;
+	  bool overflow_p = false;
+	  bool overflow_mul_p;
 	  signop sign = TYPE_SIGN (ctype);
-	  wide_int mul = wi::mul_full (op1, c, sign);
+	  wide_int mul = wi::mul (op1, c, sign, &overflow_mul_p);
 	  overflow_p = TREE_OVERFLOW (c) | TREE_OVERFLOW (op1);
-	  if (!wi::fits_to_tree_p (mul, ctype)
+	  if (overflow_mul_p
 	      && ((sign == UNSIGNED && tcode != MULT_EXPR) || sign == SIGNED))
 	    overflow_p = true;
 	  if (!overflow_p)
Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h	(revision 205597)
+++ gcc/wide-int.h	(working copy)
@@ -1575,7 +1575,7 @@ namespace wi
   unsigned int mul_internal (HOST_WIDE_INT *, const HOST_WIDE_INT *,
 			     unsigned int, const HOST_WIDE_INT *,
 			     unsigned int, unsigned int, signop, bool *,
-			     bool, bool);
+			     bool);
   unsigned int divmod_internal (HOST_WIDE_INT *, unsigned int *,
 				HOST_WIDE_INT *, const HOST_WIDE_INT *,
 				unsigned int, unsigned int,
@@ -2394,7 +2394,7 @@ wi::mul (const T1 &x, const T2 &y)
     }
   else
     result.set_len (mul_internal (val, xi.val, xi.len, yi.val, yi.len,
-				  precision, UNSIGNED, 0, false, false));
+				  precision, UNSIGNED, 0, false));
   return result;
 }
 
@@ -2410,7 +2410,7 @@ wi::mul (const T1 &x, const T2 &y, signo
   WIDE_INT_REF_FOR (T2) yi (y, precision);
   result.set_len (mul_internal (val, xi.val, xi.len,
 				yi.val, yi.len, precision,
-				sgn, overflow, false, false));
+				sgn, overflow, false));
   return result;
 }
 
@@ -2444,7 +2444,7 @@ wi::mul_high (const T1 &x, const T2 &y,
   WIDE_INT_REF_FOR (T2) yi (y, precision);
   result.set_len (mul_internal (val, xi.val, xi.len,
 				yi.val, yi.len, precision,
-				sgn, 0, true, false));
+				sgn, 0, true));
   return result;
 }
 
@@ -3033,8 +3033,6 @@ namespace wi
   wide_int max_value (never_used1 *);
   wide_int max_value (never_used2 *);
 
-  wide_int mul_full (const wide_int_ref &, const wide_int_ref &, signop);
-
   /* FIXME: this is target dependent, so should be elsewhere.
      It also seems to assume that CHAR_BIT == BITS_PER_UNIT.  */
   wide_int from_buffer (const unsigned char *, unsigned int);
@@ -3065,19 +3063,6 @@ namespace wi
 			   unsigned int, unsigned int, bool);
 }
 
-/* Perform a widening multiplication of X and Y, extending the values
-   according according to SGN.  */
-inline wide_int
-wi::mul_full (const wide_int_ref &x, const wide_int_ref &y, signop sgn)
-{
-  gcc_checking_assert (x.precision == y.precision);
-  wide_int result = wide_int::create (x.precision * 2);
-  result.set_len (mul_internal (result.write_val (), x.val, x.len,
-				y.val, y.len, x.precision,
-				sgn, 0, false, true));
-  return result;
-}
-
 /* Return a PRECISION-bit integer in which the low WIDTH bits are set
    and the other bits are clear, or the inverse if NEGATE_P.  */
 inline wide_int

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-11-29 11:58 ` Richard Biener
                     ` (2 preceding siblings ...)
  2013-11-30 18:06   ` Kenneth Zadeck
@ 2013-12-03 16:20   ` Kenneth Zadeck
  2013-12-03 16:52     ` Richard Sandiford
  3 siblings, 1 reply; 25+ messages in thread
From: Kenneth Zadeck @ 2013-12-03 16:20 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Sandiford, gcc-patches, Mike Stump

[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]

On 11/29/2013 05:24 AM, Richard Biener wrote:
> On Thu, Nov 28, 2013 at 6:11 PM, Kenneth Zadeck
> <zadeck@naturalbridge.com> wrote:
>> This patch does three things in wide-int:
>>
>> 1) it cleans up some comments.
>> 2) removes a small amount of trash.
>> 3) it changes the max size of the wide int from being 4x of
>> MAX_BITSIZE_MODE_ANY_INT to 2x +1.   This should improve large muls and divs
>> as well as perhaps help with some cache behavior.
> @@ -235,8 +233,8 @@ along with GCC; see the file COPYING3.
>      range of a multiply.  This code needs 2n + 2 bits.  */
>
>   #define WIDE_INT_MAX_ELTS \
> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
> -   / HOST_BITS_PER_WIDE_INT)
> +  (((2 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
> +    / HOST_BITS_PER_WIDE_INT) + 1)
>
> I always wondered why VRP (if that is the only reason we do 2*n+1)
> cannot simply use FIXED_WIDE_INT(MAX_BITSIZE_MODE_ANY_INT*2 + 1)?
> Other widest_int users should not suffer IMHO.  widest_int should
> strictly cover all modes that the target can do any arithmetic on
> (thus not XImode or OImode on x86_64).
>
> Richard.
>
>> ok to commit
enclosed is the code to change the large multiplies in tree-vrp as well 
as sets the size of the WIDE_INT_MAX_ELTS as we have discussed.

Bootstrapped and tested on x86-64.

ok to commit?

kenny

[-- Attachment #2: mulVRP.diff --]
[-- Type: text/x-patch, Size: 5694 bytes --]

Index: tree-vrp.c
===================================================================
--- tree-vrp.c	(revision 205597)
+++ tree-vrp.c	(working copy)
@@ -2611,22 +2611,28 @@ extract_range_from_binary_expr_1 (value_
 
       signop sign = TYPE_SIGN (expr_type);
       unsigned int prec = TYPE_PRECISION (expr_type);
-      unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
 
       if (range_int_cst_p (&vr0)
 	  && range_int_cst_p (&vr1)
 	  && TYPE_OVERFLOW_WRAPS (expr_type))
 	{
-	  wide_int sizem1 = wi::mask (prec, false, prec2);
-	  wide_int size = sizem1 + 1;
+	  /* vrp_int is twice as wide as anything that the target
+	     supports so it can support a full width multiply.  No
+	     need to add any more padding for an extra sign bit
+	     because that comes with the way that WIDE_INT_MAX_ELTS is
+	     defined.  */ 
+	  typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) 
+	    vrp_int;
+	  vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
+	  vrp_int size = sizem1 + 1;
 
 	  /* Extend the values using the sign of the result to PREC2.
 	     From here on out, everthing is just signed math no matter
 	     what the input types were.  */
-	  wide_int min0 = wide_int::from (vr0.min, prec2, sign);
-	  wide_int max0 = wide_int::from (vr0.max, prec2, sign);
-	  wide_int min1 = wide_int::from (vr1.min, prec2, sign);
-	  wide_int max1 = wide_int::from (vr1.max, prec2, sign);
+	  vrp_int min0 = wi::to_vrp (vr0.min);
+	  vrp_int max0 = wi::to_vrp (vr0.max);
+	  vrp_int min1 = wi::to_vrp (vr1.min);
+	  vrp_int max1 = wi::to_vrp (vr1.max);
 
 	  /* Canonicalize the intervals.  */
 	  if (sign == UNSIGNED)
@@ -2644,17 +2650,17 @@ extract_range_from_binary_expr_1 (value_
 		}
 	    }
 
-	  wide_int prod0 = min0 * min1;
-	  wide_int prod1 = min0 * max1;
-	  wide_int prod2 = max0 * min1;
-	  wide_int prod3 = max0 * max1;
+	  vrp_int prod0 = min0 * min1;
+	  vrp_int prod1 = min0 * max1;
+	  vrp_int prod2 = max0 * min1;
+	  vrp_int prod3 = max0 * max1;
 
 	  /* Sort the 4 products so that min is in prod0 and max is in
 	     prod3.  */
 	  /* min0min1 > max0max1 */
 	  if (wi::gts_p (prod0, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod0;
 	      prod0 = tmp;
 	    }
@@ -2662,21 +2668,21 @@ extract_range_from_binary_expr_1 (value_
 	  /* min0max1 > max0min1 */
 	  if (wi::gts_p (prod1, prod2))
 	    {
-	      wide_int tmp = prod2;
+	      vrp_int tmp = prod2;
 	      prod2 = prod1;
 	      prod1 = tmp;
 	    }
 
 	  if (wi::gts_p (prod0, prod1))
 	    {
-	      wide_int tmp = prod1;
+	      vrp_int tmp = prod1;
 	      prod1 = prod0;
 	      prod0 = tmp;
 	    }
 
 	  if (wi::gts_p (prod2, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod2;
 	      prod2 = tmp;
 	    }
Index: tree.h
===================================================================
--- tree.h	(revision 205597)
+++ tree.h	(working copy)
@@ -4565,10 +4565,12 @@ namespace wi
     static const unsigned int precision = N;
   };
 
-  generic_wide_int <extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+  generic_wide_int <extended_tree <WIDE_INT_MAX_PRECISION> >
   to_widest (const_tree);
 
   generic_wide_int <extended_tree <ADDR_MAX_PRECISION> > to_offset (const_tree);
+
+  generic_wide_int <extended_tree <WIDE_INT_MAX_PRECISION * 2> > to_vrp (const_tree);
 }
 
 inline unsigned int
@@ -4586,7 +4588,7 @@ wi::int_traits <const_tree>::decompose (
 			  precision);
 }
 
-inline generic_wide_int <wi::extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+inline generic_wide_int <wi::extended_tree <WIDE_INT_MAX_PRECISION> >
 wi::to_widest (const_tree t)
 {
   return t;
@@ -4597,6 +4599,12 @@ wi::to_offset (const_tree t)
 {
   return t;
 }
+
+inline generic_wide_int <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> >
+wi::to_vrp (const_tree t)
+{
+  return t;
+}
 
 template <int N>
 inline wi::extended_tree <N>::extended_tree (const_tree t)
Index: wide-int.h
===================================================================
--- wide-int.h	(revision 205599)
+++ wide-int.h	(working copy)
@@ -228,15 +228,16 @@ along with GCC; see the file COPYING3.
 #endif
 
 /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
-   early examination of the target's mode file.  Thus it is safe that
-   some small multiple of this number is easily larger than any number
-   that that target could compute.  The place in the compiler that
-   currently needs the widest ints is the code that determines the
-   range of a multiply.  This code needs 2n + 2 bits.  */
-
+   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
+   can accomodate at least 1 more bit so that unsigned numbers of that
+   mode can be represented.  This will accomodate every place in the
+   compiler except for a multiply routine in tree-vrp.  That function
+   makes its own arrangements for larger wide-ints.  */
 #define WIDE_INT_MAX_ELTS \
-  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
-   / HOST_BITS_PER_WIDE_INT)
+  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)	\
+    / HOST_BITS_PER_WIDE_INT) + 1)
+
+#define WIDE_INT_MAX_PRECISION (WIDE_INT_MAX_ELTS * HOST_BITS_PER_WIDE_INT)
 
 /* This is the max size of any pointer on any machine.  It does not
    seem to be as easy to sniff this out of the machine description as
@@ -291,7 +292,7 @@ struct wide_int_storage;
 
 typedef generic_wide_int <wide_int_storage> wide_int;
 typedef FIXED_WIDE_INT (ADDR_MAX_PRECISION) offset_int;
-typedef FIXED_WIDE_INT (MAX_BITSIZE_MODE_ANY_INT) widest_int;
+typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION) widest_int;
 
 template <bool SE>
 struct wide_int_ref_storage;

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-03 16:20   ` Kenneth Zadeck
@ 2013-12-03 16:52     ` Richard Sandiford
  2013-12-03 18:32       ` Mike Stump
  2013-12-06 16:45       ` Kenneth Zadeck
  0 siblings, 2 replies; 25+ messages in thread
From: Richard Sandiford @ 2013-12-03 16:52 UTC (permalink / raw)
  To: Kenneth Zadeck; +Cc: Richard Biener, gcc-patches, Mike Stump

Kenneth Zadeck <zadeck@naturalbridge.com> writes:
> Index: tree-vrp.c
> ===================================================================
> --- tree-vrp.c	(revision 205597)
> +++ tree-vrp.c	(working copy)
> @@ -2611,22 +2611,28 @@ extract_range_from_binary_expr_1 (value_
>  
>        signop sign = TYPE_SIGN (expr_type);
>        unsigned int prec = TYPE_PRECISION (expr_type);
> -      unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
>  
>        if (range_int_cst_p (&vr0)
>  	  && range_int_cst_p (&vr1)
>  	  && TYPE_OVERFLOW_WRAPS (expr_type))
>  	{
> -	  wide_int sizem1 = wi::mask (prec, false, prec2);
> -	  wide_int size = sizem1 + 1;
> +	  /* vrp_int is twice as wide as anything that the target
> +	     supports so it can support a full width multiply.  No
> +	     need to add any more padding for an extra sign bit
> +	     because that comes with the way that WIDE_INT_MAX_ELTS is
> +	     defined.  */ 
> +	  typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) 
> +	    vrp_int;
> +	  vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
> +	  vrp_int size = sizem1 + 1;
>  
>  	  /* Extend the values using the sign of the result to PREC2.
>  	     From here on out, everthing is just signed math no matter
>  	     what the input types were.  */
> -	  wide_int min0 = wide_int::from (vr0.min, prec2, sign);
> -	  wide_int max0 = wide_int::from (vr0.max, prec2, sign);
> -	  wide_int min1 = wide_int::from (vr1.min, prec2, sign);
> -	  wide_int max1 = wide_int::from (vr1.max, prec2, sign);
> +	  vrp_int min0 = wi::to_vrp (vr0.min);
> +	  vrp_int max0 = wi::to_vrp (vr0.max);
> +	  vrp_int min1 = wi::to_vrp (vr1.min);
> +	  vrp_int max1 = wi::to_vrp (vr1.max);

I think we should avoid putting to_vrp in tree.h if vrp_int is only
local to this block.  Instead you could have:

	  typedef generic_wide_int
             <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
          ...
          vrp_int_cst min0 = vr0.min;
          vrp_int_cst max0 = vr0.max;
          vrp_int_cst min1 = vr1.min;
          vrp_int_cst max1 = vr1.max;

> @@ -228,15 +228,16 @@ along with GCC; see the file COPYING3.
>  #endif
>  
>  /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
> -   early examination of the target's mode file.  Thus it is safe that
> -   some small multiple of this number is easily larger than any number
> -   that that target could compute.  The place in the compiler that
> -   currently needs the widest ints is the code that determines the
> -   range of a multiply.  This code needs 2n + 2 bits.  */
> -
> +   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
> +   can accomodate at least 1 more bit so that unsigned numbers of that
> +   mode can be represented.  This will accomodate every place in the
> +   compiler except for a multiply routine in tree-vrp.  That function
> +   makes its own arrangements for larger wide-ints.  */

I think we should drop the "This will accomodate..." bit, since it'll soon
get out of date.  Maybe something like:

    Note that it is still possible to create fixed_wide_ints that have
    precisions greater than MAX_BITSIZE_MODE_ANY_INT.  This can be useful
    when representing a double-width multiplication result, for example.  */

>  #define WIDE_INT_MAX_ELTS \
> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
> -   / HOST_BITS_PER_WIDE_INT)
> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)	\
> +    / HOST_BITS_PER_WIDE_INT) + 1)

I think this should be:

  (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)

We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact multiple
of HOST_BITS_PER_WIDE_INT.

Looks good to me otherwise FWIW.

You probably already realise this, but for avoidance of doubt, Richard
was also asking that we reduce MAX_BITSIZE_MODE_ANY_INT to 128 on x86_64,
since that's the largest scalar_mode_supported_p mode.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-03 16:52     ` Richard Sandiford
@ 2013-12-03 18:32       ` Mike Stump
  2013-12-06 16:45       ` Kenneth Zadeck
  1 sibling, 0 replies; 25+ messages in thread
From: Mike Stump @ 2013-12-03 18:32 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: Kenneth Zadeck, Richard Biener, gcc-patches

On Dec 3, 2013, at 8:52 AM, Richard Sandiford <rsandifo@linux.vnet.ibm.com> wrote:
> You probably already realise this, but for avoidance of doubt, Richard
> was also asking that we reduce MAX_BITSIZE_MODE_ANY_INT to 128 on x86_64,
> since that's the largest scalar_mode_supported_p mode.

Personally, I'd love to see scalar_mode_supported_p be auto-generated from the gen*.c programs; then, we could just add a line to that program to synthesize this value.  Absent that, which I think is a nice long term direction to shoot for, we'd need yet another #define for the target to explain the largest supported size.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-03 16:52     ` Richard Sandiford
  2013-12-03 18:32       ` Mike Stump
@ 2013-12-06 16:45       ` Kenneth Zadeck
  2013-12-06 18:14         ` Kenneth Zadeck
  2013-12-06 18:32         ` Richard Sandiford
  1 sibling, 2 replies; 25+ messages in thread
From: Kenneth Zadeck @ 2013-12-06 16:45 UTC (permalink / raw)
  To: Richard Biener, gcc-patches, Mike Stump, rsandifo

[-- Attachment #1: Type: text/plain, Size: 4447 bytes --]

On 12/03/2013 11:52 AM, Richard Sandiford wrote:
> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>> Index: tree-vrp.c
>> ===================================================================
>> --- tree-vrp.c	(revision 205597)
>> +++ tree-vrp.c	(working copy)
>> @@ -2611,22 +2611,28 @@ extract_range_from_binary_expr_1 (value_
>>   
>>         signop sign = TYPE_SIGN (expr_type);
>>         unsigned int prec = TYPE_PRECISION (expr_type);
>> -      unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
>>   
>>         if (range_int_cst_p (&vr0)
>>   	  && range_int_cst_p (&vr1)
>>   	  && TYPE_OVERFLOW_WRAPS (expr_type))
>>   	{
>> -	  wide_int sizem1 = wi::mask (prec, false, prec2);
>> -	  wide_int size = sizem1 + 1;
>> +	  /* vrp_int is twice as wide as anything that the target
>> +	     supports so it can support a full width multiply.  No
>> +	     need to add any more padding for an extra sign bit
>> +	     because that comes with the way that WIDE_INT_MAX_ELTS is
>> +	     defined.  */
>> +	  typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2)
>> +	    vrp_int;
>> +	  vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
>> +	  vrp_int size = sizem1 + 1;
>>   
>>   	  /* Extend the values using the sign of the result to PREC2.
>>   	     From here on out, everthing is just signed math no matter
>>   	     what the input types were.  */
>> -	  wide_int min0 = wide_int::from (vr0.min, prec2, sign);
>> -	  wide_int max0 = wide_int::from (vr0.max, prec2, sign);
>> -	  wide_int min1 = wide_int::from (vr1.min, prec2, sign);
>> -	  wide_int max1 = wide_int::from (vr1.max, prec2, sign);
>> +	  vrp_int min0 = wi::to_vrp (vr0.min);
>> +	  vrp_int max0 = wi::to_vrp (vr0.max);
>> +	  vrp_int min1 = wi::to_vrp (vr1.min);
>> +	  vrp_int max1 = wi::to_vrp (vr1.max);
> I think we should avoid putting to_vrp in tree.h if vrp_int is only
> local to this block.  Instead you could have:
>
> 	  typedef generic_wide_int
>               <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
>            ...
>            vrp_int_cst min0 = vr0.min;
>            vrp_int_cst max0 = vr0.max;
>            vrp_int_cst min1 = vr1.min;
>            vrp_int_cst max1 = vr1.max;
>
i did this in a different way because i had trouble doing it as you 
suggested.    the short answer is that all of the vrp_int code is now 
local to tree-vrp.c which i think was your primary goal
>> @@ -228,15 +228,16 @@ along with GCC; see the file COPYING3.
>>   #endif
>>   
>>   /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
>> -   early examination of the target's mode file.  Thus it is safe that
>> -   some small multiple of this number is easily larger than any number
>> -   that that target could compute.  The place in the compiler that
>> -   currently needs the widest ints is the code that determines the
>> -   range of a multiply.  This code needs 2n + 2 bits.  */
>> -
>> +   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
>> +   can accomodate at least 1 more bit so that unsigned numbers of that
>> +   mode can be represented.  This will accomodate every place in the
>> +   compiler except for a multiply routine in tree-vrp.  That function
>> +   makes its own arrangements for larger wide-ints.  */
> I think we should drop the "This will accomodate..." bit, since it'll soon
> get out of date.  Maybe something like:
>
>      Note that it is still possible to create fixed_wide_ints that have
>      precisions greater than MAX_BITSIZE_MODE_ANY_INT.  This can be useful
>      when representing a double-width multiplication result, for example.  */
done
>>   #define WIDE_INT_MAX_ELTS \
>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>> -   / HOST_BITS_PER_WIDE_INT)
>> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)	\
>> +    / HOST_BITS_PER_WIDE_INT) + 1)
> I think this should be:
>
>    (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>
> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact multiple
> of HOST_BITS_PER_WIDE_INT.
we will do this later when some other issues that Eric B raised are settled.

ok to commit to the branch?

kenny
> Looks good to me otherwise FWIW.
>
> You probably already realise this, but for avoidance of doubt, Richard
> was also asking that we reduce MAX_BITSIZE_MODE_ANY_INT to 128 on x86_64,
> since that's the largest scalar_mode_supported_p mode.
>
> Thanks,
> Richard
>


[-- Attachment #2: MulVRP2.diff --]
[-- Type: text/x-patch, Size: 6265 bytes --]

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 205726)
+++ gcc/tree-vrp.c	(working copy)
@@ -2213,6 +2213,22 @@ extract_range_from_multiplicative_op_1 (
     set_value_range (vr, type, min, max, NULL);
 }
 
+/* vrp_int is twice as wide as anything that the target supports so it
+   can support a full width multiply.  No need to add any more padding
+   for an extra sign bit because that comes with the way that
+   WIDE_INT_MAX_ELTS is defined.  */ 
+typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int;
+namespace wi
+{
+  generic_wide_int <extended_tree <WIDE_INT_MAX_PRECISION * 2> > to_vrp (const_tree);
+}
+
+inline generic_wide_int <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> >
+wi::to_vrp (const_tree t)
+{
+  return t;
+}
+
 /* Extract range information from a binary operation CODE based on
    the ranges of each of its operands, *VR0 and *VR1 with resulting
    type EXPR_TYPE.  The resulting range is stored in *VR.  */
@@ -2620,22 +2636,21 @@ extract_range_from_binary_expr_1 (value_
 
       signop sign = TYPE_SIGN (expr_type);
       unsigned int prec = TYPE_PRECISION (expr_type);
-      unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
 
       if (range_int_cst_p (&vr0)
 	  && range_int_cst_p (&vr1)
 	  && TYPE_OVERFLOW_WRAPS (expr_type))
 	{
-	  wide_int sizem1 = wi::mask (prec, false, prec2);
-	  wide_int size = sizem1 + 1;
+	  vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
+	  vrp_int size = sizem1 + 1;
 
 	  /* Extend the values using the sign of the result to PREC2.
 	     From here on out, everthing is just signed math no matter
 	     what the input types were.  */
-	  wide_int min0 = wide_int::from (vr0.min, prec2, sign);
-	  wide_int max0 = wide_int::from (vr0.max, prec2, sign);
-	  wide_int min1 = wide_int::from (vr1.min, prec2, sign);
-	  wide_int max1 = wide_int::from (vr1.max, prec2, sign);
+	  vrp_int min0 = wi::to_vrp (vr0.min);
+	  vrp_int max0 = wi::to_vrp (vr0.max);
+	  vrp_int min1 = wi::to_vrp (vr1.min);
+	  vrp_int max1 = wi::to_vrp (vr1.max);
 
 	  /* Canonicalize the intervals.  */
 	  if (sign == UNSIGNED)
@@ -2653,17 +2668,17 @@ extract_range_from_binary_expr_1 (value_
 		}
 	    }
 
-	  wide_int prod0 = min0 * min1;
-	  wide_int prod1 = min0 * max1;
-	  wide_int prod2 = max0 * min1;
-	  wide_int prod3 = max0 * max1;
+	  vrp_int prod0 = min0 * min1;
+	  vrp_int prod1 = min0 * max1;
+	  vrp_int prod2 = max0 * min1;
+	  vrp_int prod3 = max0 * max1;
 
 	  /* Sort the 4 products so that min is in prod0 and max is in
 	     prod3.  */
 	  /* min0min1 > max0max1 */
 	  if (wi::gts_p (prod0, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod0;
 	      prod0 = tmp;
 	    }
@@ -2671,21 +2686,21 @@ extract_range_from_binary_expr_1 (value_
 	  /* min0max1 > max0min1 */
 	  if (wi::gts_p (prod1, prod2))
 	    {
-	      wide_int tmp = prod2;
+	      vrp_int tmp = prod2;
 	      prod2 = prod1;
 	      prod1 = tmp;
 	    }
 
 	  if (wi::gts_p (prod0, prod1))
 	    {
-	      wide_int tmp = prod1;
+	      vrp_int tmp = prod1;
 	      prod1 = prod0;
 	      prod0 = tmp;
 	    }
 
 	  if (wi::gts_p (prod2, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod2;
 	      prod2 = tmp;
 	    }
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 205726)
+++ gcc/tree.h	(working copy)
@@ -4549,7 +4549,7 @@ namespace wi
     static const unsigned int precision = N;
   };
 
-  generic_wide_int <extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+  generic_wide_int <extended_tree <WIDE_INT_MAX_PRECISION> >
   to_widest (const_tree);
 
   generic_wide_int <extended_tree <ADDR_MAX_PRECISION> > to_offset (const_tree);
@@ -4570,7 +4570,7 @@ wi::int_traits <const_tree>::decompose (
 			  precision);
 }
 
-inline generic_wide_int <wi::extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+inline generic_wide_int <wi::extended_tree <WIDE_INT_MAX_PRECISION> >
 wi::to_widest (const_tree t)
 {
   return t;
@@ -4609,7 +4609,7 @@ wi::extended_tree <N>::get_len () const
 {
   if (N == ADDR_MAX_PRECISION)
     return TREE_INT_CST_OFFSET_NUNITS (m_t);
-  else if (N == MAX_BITSIZE_MODE_ANY_INT)
+  else if (N >= WIDE_INT_MAX_PRECISION)
     return TREE_INT_CST_EXT_NUNITS (m_t);
   else
     /* This class is designed to be used for specific output precisions
Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h	(revision 205726)
+++ gcc/wide-int.h	(working copy)
@@ -228,15 +228,17 @@ along with GCC; see the file COPYING3.
 #endif
 
 /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
-   early examination of the target's mode file.  Thus it is safe that
-   some small multiple of this number is easily larger than any number
-   that that target could compute.  The place in the compiler that
-   currently needs the widest ints is the code that determines the
-   range of a multiply.  This code needs 2n + 2 bits.  */
-
+   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
+   can accomodate at least 1 more bit so that unsigned numbers of that
+   mode can be represented.  Note that it is still possible to create
+   fixed_wide_ints that have precisions greater than
+   MAX_BITSIZE_MODE_ANY_INT.  This can be useful when representing a
+   double-width multiplication result, for example.  */
 #define WIDE_INT_MAX_ELTS \
-  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
-   / HOST_BITS_PER_WIDE_INT)
+  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)	\
+    / HOST_BITS_PER_WIDE_INT) + 1)
+
+#define WIDE_INT_MAX_PRECISION (WIDE_INT_MAX_ELTS * HOST_BITS_PER_WIDE_INT)
 
 /* This is the max size of any pointer on any machine.  It does not
    seem to be as easy to sniff this out of the machine description as
@@ -294,7 +296,7 @@ struct wide_int_storage;
 
 typedef generic_wide_int <wide_int_storage> wide_int;
 typedef FIXED_WIDE_INT (ADDR_MAX_PRECISION) offset_int;
-typedef FIXED_WIDE_INT (MAX_BITSIZE_MODE_ANY_INT) widest_int;
+typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION) widest_int;
 
 template <bool SE>
 struct wide_int_ref_storage;

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-06 16:45       ` Kenneth Zadeck
@ 2013-12-06 18:14         ` Kenneth Zadeck
  2013-12-06 18:32         ` Richard Sandiford
  1 sibling, 0 replies; 25+ messages in thread
From: Kenneth Zadeck @ 2013-12-06 18:14 UTC (permalink / raw)
  To: Richard Biener, gcc-patches, Mike Stump, rsandifo

[-- Attachment #1: Type: text/plain, Size: 6941 bytes --]

Richard asked to see the patch where i did the changes the way that he 
asked in his email. Doing it the way he wants potentially has advantages 
over the way that i did it, but his technique fails for non obvious 
reasons. The failure in building is:

g++ -c -g -DIN_GCC -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wwrite-strings -Wcast-qual 
-Wno-format -Wmissing-format-attribute -pedantic -Wno-long-long 
-Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H 
-I. -I. -I../../gccBadMulVrp/gcc -I../../gccBadMulVrp/gcc/. 
-I../../gccBadMulVrp/gcc/../include 
-I../../gccBadMulVrp/gcc/../libcpp/include 
-I../../gccBadMulVrp/gcc/../libdecnumber 
-I../../gccBadMulVrp/gcc/../libdecnumber/bid -I../libdecnumber 
-I../../gccBadMulVrp/gcc/../libbacktrace -o tree-vrp.o -MT tree-vrp.o 
-MMD -MP -MF ./.deps/tree-vrp.TPo ../../gccBadMulVrp/gcc/tree-vrp.c
In file included from ../../gccBadMulVrp/gcc/double-int.h:23:0,
from ../../gccBadMulVrp/gcc/tree-core.h:28,
from ../../gccBadMulVrp/gcc/tree.h:23,
from ../../gccBadMulVrp/gcc/tree-vrp.c:26:
../../gccBadMulVrp/gcc/wide-int.h: In member function 
‘generic_wide_int<storage>& generic_wide_int<T>::operator=(const T&) 
[with T = generic_wide_int<fixed_wide_int_storage<1152> >, storage = 
wi::extended_tree<1152>]Â’:
../../gccBadMulVrp/gcc/wide-int.h:701:3: instantiated from 
‘generic_wide_int<T>& generic_wide_int<T>::operator-=(const T&) [with T 
= generic_wide_int<fixed_wide_int_storage<1152> >, storage = 
wi::extended_tree<1152>, generic_wide_int<T> = 
generic_wide_int<wi::extended_tree<1152> >]Â’
../../gccBadMulVrp/gcc/tree-vrp.c:2646:13: instantiated from here
../../gccBadMulVrp/gcc/wide-int.h:860:3: error: no matching function for 
call to ‘generic_wide_int<wi::extended_tree<1152> >::operator=(const 
generic_wide_int<fixed_wide_int_storage<1152> >&)Â’
../../gccBadMulVrp/gcc/wide-int.h:860:3: note: candidate is:
../../gccBadMulVrp/gcc/tree.h:4529:9: note: wi::extended_tree<1152>& 
wi::extended_tree<1152>::operator=(const wi::extended_tree<1152>&)
../../gccBadMulVrp/gcc/tree.h:4529:9: note: no known conversion for 
argument 1 from ‘const generic_wide_int<fixed_wide_int_storage<1152> >’ 
to ‘const wi::extended_tree<1152>&’
make[3]: *** [tree-vrp.o] Error 1
make[3]: Leaving directory `/home/zadeck/gcc/gbbBadMulVrp/gcc'
make[2]: *** [all-stage1-gcc] Error 2
make[2]: Leaving directory `/home/zadeck/gcc/gbbBadMulVrp'
make[1]: *** [stage1-bubble] Error 2
make[1]: Leaving directory `/home/zadeck/gcc/gbbBadMulVrp'
make: *** [all] Error 2
heracles:~/gcc/gbbBadMulVrp(9) cd ../gccBadMulVrp/



On 12/06/2013 11:45 AM, Kenneth Zadeck wrote:
> On 12/03/2013 11:52 AM, Richard Sandiford wrote:
>> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>> Index: tree-vrp.c
>>> ===================================================================
>>> --- tree-vrp.c (revision 205597)
>>> +++ tree-vrp.c (working copy)
>>> @@ -2611,22 +2611,28 @@ extract_range_from_binary_expr_1 (value_
>>> signop sign = TYPE_SIGN (expr_type);
>>> unsigned int prec = TYPE_PRECISION (expr_type);
>>> - unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
>>> if (range_int_cst_p (&vr0)
>>> && range_int_cst_p (&vr1)
>>> && TYPE_OVERFLOW_WRAPS (expr_type))
>>> {
>>> - wide_int sizem1 = wi::mask (prec, false, prec2);
>>> - wide_int size = sizem1 + 1;
>>> + /* vrp_int is twice as wide as anything that the target
>>> + supports so it can support a full width multiply. No
>>> + need to add any more padding for an extra sign bit
>>> + because that comes with the way that WIDE_INT_MAX_ELTS is
>>> + defined. */
>>> + typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2)
>>> + vrp_int;
>>> + vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
>>> + vrp_int size = sizem1 + 1;
>>> /* Extend the values using the sign of the result to PREC2.
>>> From here on out, everthing is just signed math no matter
>>> what the input types were. */
>>> - wide_int min0 = wide_int::from (vr0.min, prec2, sign);
>>> - wide_int max0 = wide_int::from (vr0.max, prec2, sign);
>>> - wide_int min1 = wide_int::from (vr1.min, prec2, sign);
>>> - wide_int max1 = wide_int::from (vr1.max, prec2, sign);
>>> + vrp_int min0 = wi::to_vrp (vr0.min);
>>> + vrp_int max0 = wi::to_vrp (vr0.max);
>>> + vrp_int min1 = wi::to_vrp (vr1.min);
>>> + vrp_int max1 = wi::to_vrp (vr1.max);
>> I think we should avoid putting to_vrp in tree.h if vrp_int is only
>> local to this block. Instead you could have:
>>
>> typedef generic_wide_int
>> <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
>> ...
>> vrp_int_cst min0 = vr0.min;
>> vrp_int_cst max0 = vr0.max;
>> vrp_int_cst min1 = vr1.min;
>> vrp_int_cst max1 = vr1.max;
>>
> i did this in a different way because i had trouble doing it as you 
> suggested. the short answer is that all of the vrp_int code is now 
> local to tree-vrp.c which i think was your primary goal
>>> @@ -228,15 +228,16 @@ along with GCC; see the file COPYING3.
>>> #endif
>>> /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
>>> - early examination of the target's mode file. Thus it is safe that
>>> - some small multiple of this number is easily larger than any number
>>> - that that target could compute. The place in the compiler that
>>> - currently needs the widest ints is the code that determines the
>>> - range of a multiply. This code needs 2n + 2 bits. */
>>> -
>>> + early examination of the target's mode file. The WIDE_INT_MAX_ELTS
>>> + can accomodate at least 1 more bit so that unsigned numbers of that
>>> + mode can be represented. This will accomodate every place in the
>>> + compiler except for a multiply routine in tree-vrp. That function
>>> + makes its own arrangements for larger wide-ints. */
>> I think we should drop the "This will accomodate..." bit, since it'll 
>> soon
>> get out of date. Maybe something like:
>>
>> Note that it is still possible to create fixed_wide_ints that have
>> precisions greater than MAX_BITSIZE_MODE_ANY_INT. This can be useful
>> when representing a double-width multiplication result, for example. */
> done
>>> #define WIDE_INT_MAX_ELTS \
>>> - ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>> - / HOST_BITS_PER_WIDE_INT)
>>> + (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>> + / HOST_BITS_PER_WIDE_INT) + 1)
>> I think this should be:
>>
>> (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>>
>> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact 
>> multiple
>> of HOST_BITS_PER_WIDE_INT.
> we will do this later when some other issues that Eric B raised are 
> settled.
>
> ok to commit to the branch?
>
> kenny
>> Looks good to me otherwise FWIW.
>>
>> You probably already realise this, but for avoidance of doubt, Richard
>> was also asking that we reduce MAX_BITSIZE_MODE_ANY_INT to 128 on 
>> x86_64,
>> since that's the largest scalar_mode_supported_p mode.
>>
>> Thanks,
>> Richard
>>
>


[-- Attachment #2: badMulVrp.diff --]
[-- Type: text/x-patch, Size: 5578 bytes --]

Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h	(revision 205726)
+++ gcc/wide-int.h	(working copy)
@@ -228,15 +228,17 @@ along with GCC; see the file COPYING3.
 #endif
 
 /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
-   early examination of the target's mode file.  Thus it is safe that
-   some small multiple of this number is easily larger than any number
-   that that target could compute.  The place in the compiler that
-   currently needs the widest ints is the code that determines the
-   range of a multiply.  This code needs 2n + 2 bits.  */
-
+   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
+   can accomodate at least 1 more bit so that unsigned numbers of that
+   mode can be represented.  Note that it is still possible to create
+   fixed_wide_ints that have precisions greater than
+   MAX_BITSIZE_MODE_ANY_INT.  This can be useful when representing a
+   double-width multiplication result, for example.  */
 #define WIDE_INT_MAX_ELTS \
-  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
-   / HOST_BITS_PER_WIDE_INT)
+  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)	\
+    / HOST_BITS_PER_WIDE_INT) + 1)
+
+#define WIDE_INT_MAX_PRECISION (WIDE_INT_MAX_ELTS * HOST_BITS_PER_WIDE_INT)
 
 /* This is the max size of any pointer on any machine.  It does not
    seem to be as easy to sniff this out of the machine description as
@@ -294,7 +296,7 @@ struct wide_int_storage;
 
 typedef generic_wide_int <wide_int_storage> wide_int;
 typedef FIXED_WIDE_INT (ADDR_MAX_PRECISION) offset_int;
-typedef FIXED_WIDE_INT (MAX_BITSIZE_MODE_ANY_INT) widest_int;
+typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION) widest_int;
 
 template <bool SE>
 struct wide_int_ref_storage;
Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 205726)
+++ gcc/tree-vrp.c	(working copy)
@@ -2620,23 +2620,24 @@ extract_range_from_binary_expr_1 (value_
 
       signop sign = TYPE_SIGN (expr_type);
       unsigned int prec = TYPE_PRECISION (expr_type);
-      unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
 
       if (range_int_cst_p (&vr0)
 	  && range_int_cst_p (&vr1)
 	  && TYPE_OVERFLOW_WRAPS (expr_type))
 	{
-	  wide_int sizem1 = wi::mask (prec, false, prec2);
-	  wide_int size = sizem1 + 1;
+	  typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int;
+	  typedef generic_wide_int
+             <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
+	  vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
+	  vrp_int size = sizem1 + 1;
 
 	  /* Extend the values using the sign of the result to PREC2.
 	     From here on out, everthing is just signed math no matter
 	     what the input types were.  */
-	  wide_int min0 = wide_int::from (vr0.min, prec2, sign);
-	  wide_int max0 = wide_int::from (vr0.max, prec2, sign);
-	  wide_int min1 = wide_int::from (vr1.min, prec2, sign);
-	  wide_int max1 = wide_int::from (vr1.max, prec2, sign);
-
+          vrp_int_cst min0 = vr0.min;
+          vrp_int_cst max0 = vr0.max;
+          vrp_int_cst min1 = vr1.min;
+          vrp_int_cst max1 = vr1.max;
 	  /* Canonicalize the intervals.  */
 	  if (sign == UNSIGNED)
 	    {
@@ -2653,17 +2654,17 @@ extract_range_from_binary_expr_1 (value_
 		}
 	    }
 
-	  wide_int prod0 = min0 * min1;
-	  wide_int prod1 = min0 * max1;
-	  wide_int prod2 = max0 * min1;
-	  wide_int prod3 = max0 * max1;
+	  vrp_int prod0 = min0 * min1;
+	  vrp_int prod1 = min0 * max1;
+	  vrp_int prod2 = max0 * min1;
+	  vrp_int prod3 = max0 * max1;
 
 	  /* Sort the 4 products so that min is in prod0 and max is in
 	     prod3.  */
 	  /* min0min1 > max0max1 */
 	  if (wi::gts_p (prod0, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod0;
 	      prod0 = tmp;
 	    }
@@ -2671,21 +2672,21 @@ extract_range_from_binary_expr_1 (value_
 	  /* min0max1 > max0min1 */
 	  if (wi::gts_p (prod1, prod2))
 	    {
-	      wide_int tmp = prod2;
+	      vrp_int tmp = prod2;
 	      prod2 = prod1;
 	      prod1 = tmp;
 	    }
 
 	  if (wi::gts_p (prod0, prod1))
 	    {
-	      wide_int tmp = prod1;
+	      vrp_int tmp = prod1;
 	      prod1 = prod0;
 	      prod0 = tmp;
 	    }
 
 	  if (wi::gts_p (prod2, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod2;
 	      prod2 = tmp;
 	    }
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 205726)
+++ gcc/tree.h	(working copy)
@@ -4549,7 +4549,7 @@ namespace wi
     static const unsigned int precision = N;
   };
 
-  generic_wide_int <extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+  generic_wide_int <extended_tree <WIDE_INT_MAX_PRECISION> >
   to_widest (const_tree);
 
   generic_wide_int <extended_tree <ADDR_MAX_PRECISION> > to_offset (const_tree);
@@ -4570,7 +4570,7 @@ wi::int_traits <const_tree>::decompose (
 			  precision);
 }
 
-inline generic_wide_int <wi::extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+inline generic_wide_int <wi::extended_tree <WIDE_INT_MAX_PRECISION> >
 wi::to_widest (const_tree t)
 {
   return t;
@@ -4609,7 +4609,7 @@ wi::extended_tree <N>::get_len () const
 {
   if (N == ADDR_MAX_PRECISION)
     return TREE_INT_CST_OFFSET_NUNITS (m_t);
-  else if (N == MAX_BITSIZE_MODE_ANY_INT)
+  else if (N >= WIDE_INT_MAX_PRECISION)
     return TREE_INT_CST_EXT_NUNITS (m_t);
   else
     /* This class is designed to be used for specific output precisions

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-06 16:45       ` Kenneth Zadeck
  2013-12-06 18:14         ` Kenneth Zadeck
@ 2013-12-06 18:32         ` Richard Sandiford
  2013-12-06 22:27           ` Kenneth Zadeck
  1 sibling, 1 reply; 25+ messages in thread
From: Richard Sandiford @ 2013-12-06 18:32 UTC (permalink / raw)
  To: Kenneth Zadeck; +Cc: Richard Biener, gcc-patches, Mike Stump

Kenneth Zadeck <zadeck@naturalbridge.com> writes:
> On 12/03/2013 11:52 AM, Richard Sandiford wrote:
>> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>> Index: tree-vrp.c
>>> ===================================================================
>>> --- tree-vrp.c	(revision 205597)
>>> +++ tree-vrp.c	(working copy)
>>> @@ -2611,22 +2611,28 @@ extract_range_from_binary_expr_1 (value_
>>>   
>>>         signop sign = TYPE_SIGN (expr_type);
>>>         unsigned int prec = TYPE_PRECISION (expr_type);
>>> -      unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
>>>   
>>>         if (range_int_cst_p (&vr0)
>>>   	  && range_int_cst_p (&vr1)
>>>   	  && TYPE_OVERFLOW_WRAPS (expr_type))
>>>   	{
>>> -	  wide_int sizem1 = wi::mask (prec, false, prec2);
>>> -	  wide_int size = sizem1 + 1;
>>> +	  /* vrp_int is twice as wide as anything that the target
>>> +	     supports so it can support a full width multiply.  No
>>> +	     need to add any more padding for an extra sign bit
>>> +	     because that comes with the way that WIDE_INT_MAX_ELTS is
>>> +	     defined.  */
>>> +	  typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2)
>>> +	    vrp_int;
>>> +	  vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
>>> +	  vrp_int size = sizem1 + 1;
>>>   
>>>   	  /* Extend the values using the sign of the result to PREC2.
>>>   	     From here on out, everthing is just signed math no matter
>>>   	     what the input types were.  */
>>> -	  wide_int min0 = wide_int::from (vr0.min, prec2, sign);
>>> -	  wide_int max0 = wide_int::from (vr0.max, prec2, sign);
>>> -	  wide_int min1 = wide_int::from (vr1.min, prec2, sign);
>>> -	  wide_int max1 = wide_int::from (vr1.max, prec2, sign);
>>> +	  vrp_int min0 = wi::to_vrp (vr0.min);
>>> +	  vrp_int max0 = wi::to_vrp (vr0.max);
>>> +	  vrp_int min1 = wi::to_vrp (vr1.min);
>>> +	  vrp_int max1 = wi::to_vrp (vr1.max);
>> I think we should avoid putting to_vrp in tree.h if vrp_int is only
>> local to this block.  Instead you could have:
>>
>> 	  typedef generic_wide_int
>>               <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
>>            ...
>>            vrp_int_cst min0 = vr0.min;
>>            vrp_int_cst max0 = vr0.max;
>>            vrp_int_cst min1 = vr1.min;
>>            vrp_int_cst max1 = vr1.max;
>>
> i did this in a different way because i had trouble doing it as you 
> suggested.    the short answer is that all of the vrp_int code is now 
> local to tree-vrp.c which i think was your primary goal

Ah, so we later assign to these variables:

	  /* Canonicalize the intervals.  */
	  if (sign == UNSIGNED)
	    {
	      if (wi::ltu_p (size, min0 + max0))
		{
		  min0 -= size;
		  max0 -= size;
		}

	      if (wi::ltu_p (size, min1 + max1))
		{
		  min1 -= size;
		  max1 -= size;
		}
	    }

OK, in that case I suppose a temporary is needed.  But I'd prefer
not to put local stuff in the wi:: namespace.  You could just have:

	  typedef generic_wide_int
	    <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;

          vrp_int min0 = vrp_int_cst (vr0.min);
          vrp_int max0 = vrp_int_cst (vr0.max);
          vrp_int min1 = vrp_int_cst (vr1.min);
          vrp_int max1 = vrp_int_cst (vr1.max);

which removes the need for:

+/* vrp_int is twice as wide as anything that the target supports so it
+   can support a full width multiply.  No need to add any more padding
+   for an extra sign bit because that comes with the way that
+   WIDE_INT_MAX_ELTS is defined.  */ 
+typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int;
+namespace wi
+{
+  generic_wide_int <extended_tree <WIDE_INT_MAX_PRECISION * 2> > to_vrp (const_tree);
+}
+
+inline generic_wide_int <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> >
+wi::to_vrp (const_tree t)
+{
+  return t;
+}
+

>>>   #define WIDE_INT_MAX_ELTS \
>>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>> -   / HOST_BITS_PER_WIDE_INT)
>>> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)	\
>>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>> I think this should be:
>>
>>    (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>>
>> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact multiple
>> of HOST_BITS_PER_WIDE_INT.
> we will do this later when some other issues that Eric B raised are settled.

I think you're talking about defining MAX_BITSIZE_MODE_ANY_INT as
MAX_SIZE_MODE_ANY_INT * BITS_PER_UNIT, but that's orthogonal to the
change above.  IMO it doesn't make sense to both round up the division
and also add 1 to the result.  So I think we should make this change
regardless of whatever follows.

Looks good to me otherwise, thanks.

Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-06 18:32         ` Richard Sandiford
@ 2013-12-06 22:27           ` Kenneth Zadeck
  2013-12-07 10:40             ` Richard Sandiford
  0 siblings, 1 reply; 25+ messages in thread
From: Kenneth Zadeck @ 2013-12-06 22:27 UTC (permalink / raw)
  To: Richard Biener, gcc-patches, Mike Stump, rsandifo

[-- Attachment #1: Type: text/plain, Size: 5120 bytes --]

On 12/06/2013 01:32 PM, Richard Sandiford wrote:
> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>> On 12/03/2013 11:52 AM, Richard Sandiford wrote:
>>> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>>> Index: tree-vrp.c
>>>> ===================================================================
>>>> --- tree-vrp.c	(revision 205597)
>>>> +++ tree-vrp.c	(working copy)
>>>> @@ -2611,22 +2611,28 @@ extract_range_from_binary_expr_1 (value_
>>>>    
>>>>          signop sign = TYPE_SIGN (expr_type);
>>>>          unsigned int prec = TYPE_PRECISION (expr_type);
>>>> -      unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
>>>>    
>>>>          if (range_int_cst_p (&vr0)
>>>>    	  && range_int_cst_p (&vr1)
>>>>    	  && TYPE_OVERFLOW_WRAPS (expr_type))
>>>>    	{
>>>> -	  wide_int sizem1 = wi::mask (prec, false, prec2);
>>>> -	  wide_int size = sizem1 + 1;
>>>> +	  /* vrp_int is twice as wide as anything that the target
>>>> +	     supports so it can support a full width multiply.  No
>>>> +	     need to add any more padding for an extra sign bit
>>>> +	     because that comes with the way that WIDE_INT_MAX_ELTS is
>>>> +	     defined.  */
>>>> +	  typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2)
>>>> +	    vrp_int;
>>>> +	  vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
>>>> +	  vrp_int size = sizem1 + 1;
>>>>    
>>>>    	  /* Extend the values using the sign of the result to PREC2.
>>>>    	     From here on out, everthing is just signed math no matter
>>>>    	     what the input types were.  */
>>>> -	  wide_int min0 = wide_int::from (vr0.min, prec2, sign);
>>>> -	  wide_int max0 = wide_int::from (vr0.max, prec2, sign);
>>>> -	  wide_int min1 = wide_int::from (vr1.min, prec2, sign);
>>>> -	  wide_int max1 = wide_int::from (vr1.max, prec2, sign);
>>>> +	  vrp_int min0 = wi::to_vrp (vr0.min);
>>>> +	  vrp_int max0 = wi::to_vrp (vr0.max);
>>>> +	  vrp_int min1 = wi::to_vrp (vr1.min);
>>>> +	  vrp_int max1 = wi::to_vrp (vr1.max);
>>> I think we should avoid putting to_vrp in tree.h if vrp_int is only
>>> local to this block.  Instead you could have:
>>>
>>> 	  typedef generic_wide_int
>>>                <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
>>>             ...
>>>             vrp_int_cst min0 = vr0.min;
>>>             vrp_int_cst max0 = vr0.max;
>>>             vrp_int_cst min1 = vr1.min;
>>>             vrp_int_cst max1 = vr1.max;
>>>
>> i did this in a different way because i had trouble doing it as you
>> suggested.    the short answer is that all of the vrp_int code is now
>> local to tree-vrp.c which i think was your primary goal
> Ah, so we later assign to these variables:
>
> 	  /* Canonicalize the intervals.  */
> 	  if (sign == UNSIGNED)
> 	    {
> 	      if (wi::ltu_p (size, min0 + max0))
> 		{
> 		  min0 -= size;
> 		  max0 -= size;
> 		}
>
> 	      if (wi::ltu_p (size, min1 + max1))
> 		{
> 		  min1 -= size;
> 		  max1 -= size;
> 		}
> 	    }
>
> OK, in that case I suppose a temporary is needed.  But I'd prefer
> not to put local stuff in the wi:: namespace.  You could just have:
>
> 	  typedef generic_wide_int
> 	    <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
>
>            vrp_int min0 = vrp_int_cst (vr0.min);
>            vrp_int max0 = vrp_int_cst (vr0.max);
>            vrp_int min1 = vrp_int_cst (vr1.min);
>            vrp_int max1 = vrp_int_cst (vr1.max);
>
> which removes the need for:
>
> +/* vrp_int is twice as wide as anything that the target supports so it
> +   can support a full width multiply.  No need to add any more padding
> +   for an extra sign bit because that comes with the way that
> +   WIDE_INT_MAX_ELTS is defined.  */
> +typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int;
> +namespace wi
> +{
> +  generic_wide_int <extended_tree <WIDE_INT_MAX_PRECISION * 2> > to_vrp (const_tree);
> +}
> +
> +inline generic_wide_int <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> >
> +wi::to_vrp (const_tree t)
> +{
> +  return t;
> +}
> +
>
>>>>    #define WIDE_INT_MAX_ELTS \
>>>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>>> -   / HOST_BITS_PER_WIDE_INT)
>>>> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)	\
>>>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>>> I think this should be:
>>>
>>>     (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>>>
>>> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact multiple
>>> of HOST_BITS_PER_WIDE_INT.
>> we will do this later when some other issues that Eric B raised are settled.
> I think you're talking about defining MAX_BITSIZE_MODE_ANY_INT as
> MAX_SIZE_MODE_ANY_INT * BITS_PER_UNIT, but that's orthogonal to the
> change above.  IMO it doesn't make sense to both round up the division
> and also add 1 to the result.  So I think we should make this change
> regardless of whatever follows.
>
> Looks good to me otherwise, thanks.
>
> Richard
>
so this one works the way you want.    While it is true the the problems 
are disjoint, the solution will likely evolving change the same lines of 
source in two different ways.

ok to commit.

kenny

[-- Attachment #2: MulVRP2.diff --]
[-- Type: text/x-patch, Size: 5618 bytes --]

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 205726)
+++ gcc/tree-vrp.c	(working copy)
@@ -2620,23 +2620,24 @@ extract_range_from_binary_expr_1 (value_
 
       signop sign = TYPE_SIGN (expr_type);
       unsigned int prec = TYPE_PRECISION (expr_type);
-      unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
 
       if (range_int_cst_p (&vr0)
 	  && range_int_cst_p (&vr1)
 	  && TYPE_OVERFLOW_WRAPS (expr_type))
 	{
-	  wide_int sizem1 = wi::mask (prec, false, prec2);
-	  wide_int size = sizem1 + 1;
+	  typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int;
+	  typedef generic_wide_int
+             <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
+	  vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
+	  vrp_int size = sizem1 + 1;
 
 	  /* Extend the values using the sign of the result to PREC2.
 	     From here on out, everthing is just signed math no matter
 	     what the input types were.  */
-	  wide_int min0 = wide_int::from (vr0.min, prec2, sign);
-	  wide_int max0 = wide_int::from (vr0.max, prec2, sign);
-	  wide_int min1 = wide_int::from (vr1.min, prec2, sign);
-	  wide_int max1 = wide_int::from (vr1.max, prec2, sign);
-
+          vrp_int min0 = vrp_int_cst (vr0.min);
+          vrp_int max0 = vrp_int_cst (vr0.max);
+          vrp_int min1 = vrp_int_cst (vr1.min);
+          vrp_int max1 = vrp_int_cst (vr1.max);
 	  /* Canonicalize the intervals.  */
 	  if (sign == UNSIGNED)
 	    {
@@ -2653,17 +2654,17 @@ extract_range_from_binary_expr_1 (value_
 		}
 	    }
 
-	  wide_int prod0 = min0 * min1;
-	  wide_int prod1 = min0 * max1;
-	  wide_int prod2 = max0 * min1;
-	  wide_int prod3 = max0 * max1;
+	  vrp_int prod0 = min0 * min1;
+	  vrp_int prod1 = min0 * max1;
+	  vrp_int prod2 = max0 * min1;
+	  vrp_int prod3 = max0 * max1;
 
 	  /* Sort the 4 products so that min is in prod0 and max is in
 	     prod3.  */
 	  /* min0min1 > max0max1 */
 	  if (wi::gts_p (prod0, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod0;
 	      prod0 = tmp;
 	    }
@@ -2671,21 +2672,21 @@ extract_range_from_binary_expr_1 (value_
 	  /* min0max1 > max0min1 */
 	  if (wi::gts_p (prod1, prod2))
 	    {
-	      wide_int tmp = prod2;
+	      vrp_int tmp = prod2;
 	      prod2 = prod1;
 	      prod1 = tmp;
 	    }
 
 	  if (wi::gts_p (prod0, prod1))
 	    {
-	      wide_int tmp = prod1;
+	      vrp_int tmp = prod1;
 	      prod1 = prod0;
 	      prod0 = tmp;
 	    }
 
 	  if (wi::gts_p (prod2, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod2;
 	      prod2 = tmp;
 	    }
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 205726)
+++ gcc/tree.h	(working copy)
@@ -4549,7 +4549,7 @@ namespace wi
     static const unsigned int precision = N;
   };
 
-  generic_wide_int <extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+  generic_wide_int <extended_tree <WIDE_INT_MAX_PRECISION> >
   to_widest (const_tree);
 
   generic_wide_int <extended_tree <ADDR_MAX_PRECISION> > to_offset (const_tree);
@@ -4570,7 +4570,7 @@ wi::int_traits <const_tree>::decompose (
 			  precision);
 }
 
-inline generic_wide_int <wi::extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+inline generic_wide_int <wi::extended_tree <WIDE_INT_MAX_PRECISION> >
 wi::to_widest (const_tree t)
 {
   return t;
@@ -4609,7 +4609,7 @@ wi::extended_tree <N>::get_len () const
 {
   if (N == ADDR_MAX_PRECISION)
     return TREE_INT_CST_OFFSET_NUNITS (m_t);
-  else if (N == MAX_BITSIZE_MODE_ANY_INT)
+  else if (N >= WIDE_INT_MAX_PRECISION)
     return TREE_INT_CST_EXT_NUNITS (m_t);
   else
     /* This class is designed to be used for specific output precisions
Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h	(revision 205726)
+++ gcc/wide-int.h	(working copy)
@@ -228,15 +228,17 @@ along with GCC; see the file COPYING3.
 #endif
 
 /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
-   early examination of the target's mode file.  Thus it is safe that
-   some small multiple of this number is easily larger than any number
-   that that target could compute.  The place in the compiler that
-   currently needs the widest ints is the code that determines the
-   range of a multiply.  This code needs 2n + 2 bits.  */
-
+   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
+   can accomodate at least 1 more bit so that unsigned numbers of that
+   mode can be represented.  Note that it is still possible to create
+   fixed_wide_ints that have precisions greater than
+   MAX_BITSIZE_MODE_ANY_INT.  This can be useful when representing a
+   double-width multiplication result, for example.  */
 #define WIDE_INT_MAX_ELTS \
-  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
-   / HOST_BITS_PER_WIDE_INT)
+  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)	\
+    / HOST_BITS_PER_WIDE_INT) + 1)
+
+#define WIDE_INT_MAX_PRECISION (WIDE_INT_MAX_ELTS * HOST_BITS_PER_WIDE_INT)
 
 /* This is the max size of any pointer on any machine.  It does not
    seem to be as easy to sniff this out of the machine description as
@@ -294,7 +296,7 @@ struct wide_int_storage;
 
 typedef generic_wide_int <wide_int_storage> wide_int;
 typedef FIXED_WIDE_INT (ADDR_MAX_PRECISION) offset_int;
-typedef FIXED_WIDE_INT (MAX_BITSIZE_MODE_ANY_INT) widest_int;
+typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION) widest_int;
 
 template <bool SE>
 struct wide_int_ref_storage;

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-06 22:27           ` Kenneth Zadeck
@ 2013-12-07 10:40             ` Richard Sandiford
  2013-12-08 10:33               ` Richard Biener
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Sandiford @ 2013-12-07 10:40 UTC (permalink / raw)
  To: Kenneth Zadeck; +Cc: Richard Biener, gcc-patches, Mike Stump

Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>>>>    #define WIDE_INT_MAX_ELTS \
>>>>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>>>> -   / HOST_BITS_PER_WIDE_INT)
>>>>> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)	\
>>>>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>>>> I think this should be:
>>>>
>>>>     (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>>>>
>>>> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact multiple
>>>> of HOST_BITS_PER_WIDE_INT.
>>> we will do this later when some other issues that Eric B raised are settled.
>> I think you're talking about defining MAX_BITSIZE_MODE_ANY_INT as
>> MAX_SIZE_MODE_ANY_INT * BITS_PER_UNIT, but that's orthogonal to the
>> change above.  IMO it doesn't make sense to both round up the division
>> and also add 1 to the result.  So I think we should make this change
>> regardless of whatever follows.
>>
>> Looks good to me otherwise, thanks.
>>
>> Richard
>>
> so this one works the way you want.    While it is true the the problems 
> are disjoint, the solution will likely evolving change the same lines of 
> source in two different ways.

Well, if we're changing the definition of WIDE_INT_MAX_ELTS now (and we are)
I think we should change it to something that makes sense.  All we want is
1 extra bit.  We don't need to round up the size and then add a whole
HWI on top.  Doing that will potentially pessimise targets that don't
need anything beyond SImode.

It's not like I can approve the patch anyway though, so I'll leave it there.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-07 10:40             ` Richard Sandiford
@ 2013-12-08 10:33               ` Richard Biener
  2013-12-09 14:49                 ` Kenneth Zadeck
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Biener @ 2013-12-08 10:33 UTC (permalink / raw)
  To: Richard Sandiford, Kenneth Zadeck; +Cc: gcc-patches, Mike Stump

Richard Sandiford <rdsandiford@googlemail.com> wrote:
>Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>>>>>    #define WIDE_INT_MAX_ELTS \
>>>>>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>>>>> -   / HOST_BITS_PER_WIDE_INT)
>>>>>> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)	\
>>>>>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>>>>> I think this should be:
>>>>>
>>>>>     (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>>>>>
>>>>> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact
>multiple
>>>>> of HOST_BITS_PER_WIDE_INT.
>>>> we will do this later when some other issues that Eric B raised are
>settled.
>>> I think you're talking about defining MAX_BITSIZE_MODE_ANY_INT as
>>> MAX_SIZE_MODE_ANY_INT * BITS_PER_UNIT, but that's orthogonal to the
>>> change above.  IMO it doesn't make sense to both round up the
>division
>>> and also add 1 to the result.  So I think we should make this change
>>> regardless of whatever follows.
>>>
>>> Looks good to me otherwise, thanks.
>>>
>>> Richard
>>>
>> so this one works the way you want.    While it is true the the
>problems 
>> are disjoint, the solution will likely evolving change the same lines
>of 
>> source in two different ways.
>
>Well, if we're changing the definition of WIDE_INT_MAX_ELTS now (and we
>are)
>I think we should change it to something that makes sense.  All we want
>is
>1 extra bit.  We don't need to round up the size and then add a whole
>HWI on top.  Doing that will potentially pessimise targets that don't
>need anything beyond SImode.

I agree.

Richard.

>It's not like I can approve the patch anyway though, so I'll leave it
>there.
>
>Thanks,
>Richard


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-08 10:33               ` Richard Biener
@ 2013-12-09 14:49                 ` Kenneth Zadeck
  2013-12-09 15:01                   ` Richard Biener
  0 siblings, 1 reply; 25+ messages in thread
From: Kenneth Zadeck @ 2013-12-09 14:49 UTC (permalink / raw)
  To: Richard Biener, Richard Sandiford; +Cc: gcc-patches, Mike Stump

[-- Attachment #1: Type: text/plain, Size: 1842 bytes --]

On 12/08/2013 05:35 AM, Richard Biener wrote:
> Richard Sandiford <rdsandiford@googlemail.com> wrote:
>> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>>>>>>     #define WIDE_INT_MAX_ELTS \
>>>>>>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>>>>>> -   / HOST_BITS_PER_WIDE_INT)
>>>>>>> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)	\
>>>>>>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>>>>>> I think this should be:
>>>>>>
>>>>>>      (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>>>>>>
>>>>>> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact
>> multiple
>>>>>> of HOST_BITS_PER_WIDE_INT.
>>>>> we will do this later when some other issues that Eric B raised are
>> settled.
>>>> I think you're talking about defining MAX_BITSIZE_MODE_ANY_INT as
>>>> MAX_SIZE_MODE_ANY_INT * BITS_PER_UNIT, but that's orthogonal to the
>>>> change above.  IMO it doesn't make sense to both round up the
>> division
>>>> and also add 1 to the result.  So I think we should make this change
>>>> regardless of whatever follows.
>>>>
>>>> Looks good to me otherwise, thanks.
>>>>
>>>> Richard
>>>>
>>> so this one works the way you want.    While it is true the the
>> problems
>>> are disjoint, the solution will likely evolving change the same lines
>> of
>>> source in two different ways.
>> Well, if we're changing the definition of WIDE_INT_MAX_ELTS now (and we
>> are)
>> I think we should change it to something that makes sense.  All we want
>> is
>> 1 extra bit.  We don't need to round up the size and then add a whole
>> HWI on top.  Doing that will potentially pessimise targets that don't
>> need anything beyond SImode.
> I agree.
>
> Richard.
Done, ok to commit?

kenny

>> It's not like I can approve the patch anyway though, so I'll leave it
>> there.
>>
>> Thanks,
>> Richard
>


[-- Attachment #2: MulVRP3.diff --]
[-- Type: text/x-patch, Size: 5614 bytes --]

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 205726)
+++ gcc/tree-vrp.c	(working copy)
@@ -2620,23 +2620,24 @@ extract_range_from_binary_expr_1 (value_
 
       signop sign = TYPE_SIGN (expr_type);
       unsigned int prec = TYPE_PRECISION (expr_type);
-      unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
 
       if (range_int_cst_p (&vr0)
 	  && range_int_cst_p (&vr1)
 	  && TYPE_OVERFLOW_WRAPS (expr_type))
 	{
-	  wide_int sizem1 = wi::mask (prec, false, prec2);
-	  wide_int size = sizem1 + 1;
+	  typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int;
+	  typedef generic_wide_int
+             <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
+	  vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
+	  vrp_int size = sizem1 + 1;
 
 	  /* Extend the values using the sign of the result to PREC2.
 	     From here on out, everthing is just signed math no matter
 	     what the input types were.  */
-	  wide_int min0 = wide_int::from (vr0.min, prec2, sign);
-	  wide_int max0 = wide_int::from (vr0.max, prec2, sign);
-	  wide_int min1 = wide_int::from (vr1.min, prec2, sign);
-	  wide_int max1 = wide_int::from (vr1.max, prec2, sign);
-
+          vrp_int min0 = vrp_int_cst (vr0.min);
+          vrp_int max0 = vrp_int_cst (vr0.max);
+          vrp_int min1 = vrp_int_cst (vr1.min);
+          vrp_int max1 = vrp_int_cst (vr1.max);
 	  /* Canonicalize the intervals.  */
 	  if (sign == UNSIGNED)
 	    {
@@ -2653,17 +2654,17 @@ extract_range_from_binary_expr_1 (value_
 		}
 	    }
 
-	  wide_int prod0 = min0 * min1;
-	  wide_int prod1 = min0 * max1;
-	  wide_int prod2 = max0 * min1;
-	  wide_int prod3 = max0 * max1;
+	  vrp_int prod0 = min0 * min1;
+	  vrp_int prod1 = min0 * max1;
+	  vrp_int prod2 = max0 * min1;
+	  vrp_int prod3 = max0 * max1;
 
 	  /* Sort the 4 products so that min is in prod0 and max is in
 	     prod3.  */
 	  /* min0min1 > max0max1 */
 	  if (wi::gts_p (prod0, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod0;
 	      prod0 = tmp;
 	    }
@@ -2671,21 +2672,21 @@ extract_range_from_binary_expr_1 (value_
 	  /* min0max1 > max0min1 */
 	  if (wi::gts_p (prod1, prod2))
 	    {
-	      wide_int tmp = prod2;
+	      vrp_int tmp = prod2;
 	      prod2 = prod1;
 	      prod1 = tmp;
 	    }
 
 	  if (wi::gts_p (prod0, prod1))
 	    {
-	      wide_int tmp = prod1;
+	      vrp_int tmp = prod1;
 	      prod1 = prod0;
 	      prod0 = tmp;
 	    }
 
 	  if (wi::gts_p (prod2, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod2;
 	      prod2 = tmp;
 	    }
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 205726)
+++ gcc/tree.h	(working copy)
@@ -4549,7 +4549,7 @@ namespace wi
     static const unsigned int precision = N;
   };
 
-  generic_wide_int <extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+  generic_wide_int <extended_tree <WIDE_INT_MAX_PRECISION> >
   to_widest (const_tree);
 
   generic_wide_int <extended_tree <ADDR_MAX_PRECISION> > to_offset (const_tree);
@@ -4570,7 +4570,7 @@ wi::int_traits <const_tree>::decompose (
 			  precision);
 }
 
-inline generic_wide_int <wi::extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+inline generic_wide_int <wi::extended_tree <WIDE_INT_MAX_PRECISION> >
 wi::to_widest (const_tree t)
 {
   return t;
@@ -4609,7 +4609,7 @@ wi::extended_tree <N>::get_len () const
 {
   if (N == ADDR_MAX_PRECISION)
     return TREE_INT_CST_OFFSET_NUNITS (m_t);
-  else if (N == MAX_BITSIZE_MODE_ANY_INT)
+  else if (N >= WIDE_INT_MAX_PRECISION)
     return TREE_INT_CST_EXT_NUNITS (m_t);
   else
     /* This class is designed to be used for specific output precisions
Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h	(revision 205726)
+++ gcc/wide-int.h	(working copy)
@@ -228,15 +228,17 @@ along with GCC; see the file COPYING3.
 #endif
 
 /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
-   early examination of the target's mode file.  Thus it is safe that
-   some small multiple of this number is easily larger than any number
-   that that target could compute.  The place in the compiler that
-   currently needs the widest ints is the code that determines the
-   range of a multiply.  This code needs 2n + 2 bits.  */
-
+   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
+   can accomodate at least 1 more bit so that unsigned numbers of that
+   mode can be represented.  Note that it is still possible to create
+   fixed_wide_ints that have precisions greater than
+   MAX_BITSIZE_MODE_ANY_INT.  This can be useful when representing a
+   double-width multiplication result, for example.  */
 #define WIDE_INT_MAX_ELTS \
-  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
-   / HOST_BITS_PER_WIDE_INT)
+  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)	\
+    / HOST_BITS_PER_WIDE_INT))
+
+#define WIDE_INT_MAX_PRECISION (WIDE_INT_MAX_ELTS * HOST_BITS_PER_WIDE_INT)
 
 /* This is the max size of any pointer on any machine.  It does not
    seem to be as easy to sniff this out of the machine description as
@@ -294,7 +296,7 @@ struct wide_int_storage;
 
 typedef generic_wide_int <wide_int_storage> wide_int;
 typedef FIXED_WIDE_INT (ADDR_MAX_PRECISION) offset_int;
-typedef FIXED_WIDE_INT (MAX_BITSIZE_MODE_ANY_INT) widest_int;
+typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION) widest_int;
 
 template <bool SE>
 struct wide_int_ref_storage;

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-09 14:49                 ` Kenneth Zadeck
@ 2013-12-09 15:01                   ` Richard Biener
  2013-12-09 15:08                     ` Kenneth Zadeck
                                       ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Richard Biener @ 2013-12-09 15:01 UTC (permalink / raw)
  To: Kenneth Zadeck; +Cc: Richard Sandiford, gcc-patches, Mike Stump

On Mon, Dec 9, 2013 at 3:49 PM, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
> On 12/08/2013 05:35 AM, Richard Biener wrote:
>>
>> Richard Sandiford <rdsandiford@googlemail.com> wrote:
>>>
>>> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>>>>>>>
>>>>>>>>     #define WIDE_INT_MAX_ELTS \
>>>>>>>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>>>>>>> -   / HOST_BITS_PER_WIDE_INT)
>>>>>>>> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)    \
>>>>>>>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>>>>>>>
>>>>>>> I think this should be:
>>>>>>>
>>>>>>>      (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>>>>>>>
>>>>>>> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact
>>>
>>> multiple
>>>>>>>
>>>>>>> of HOST_BITS_PER_WIDE_INT.
>>>>>>
>>>>>> we will do this later when some other issues that Eric B raised are
>>>
>>> settled.
>>>>>
>>>>> I think you're talking about defining MAX_BITSIZE_MODE_ANY_INT as
>>>>> MAX_SIZE_MODE_ANY_INT * BITS_PER_UNIT, but that's orthogonal to the
>>>>> change above.  IMO it doesn't make sense to both round up the
>>>
>>> division
>>>>>
>>>>> and also add 1 to the result.  So I think we should make this change
>>>>> regardless of whatever follows.
>>>>>
>>>>> Looks good to me otherwise, thanks.
>>>>>
>>>>> Richard
>>>>>
>>>> so this one works the way you want.    While it is true the the
>>>
>>> problems
>>>>
>>>> are disjoint, the solution will likely evolving change the same lines
>>>
>>> of
>>>>
>>>> source in two different ways.
>>>
>>> Well, if we're changing the definition of WIDE_INT_MAX_ELTS now (and we
>>> are)
>>> I think we should change it to something that makes sense.  All we want
>>> is
>>> 1 extra bit.  We don't need to round up the size and then add a whole
>>> HWI on top.  Doing that will potentially pessimise targets that don't
>>> need anything beyond SImode.
>>
>> I agree.
>>
>> Richard.
>
> Done, ok to commit?

+   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
+   can accomodate at least 1 more bit so that unsigned numbers of that
+   mode can be represented.  Note that it is still possible to create
+   fixed_wide_ints that have precisions greater than
+   MAX_BITSIZE_MODE_ANY_INT.  This can be useful when representing a
+   double-width multiplication result, for example.  */
 #define WIDE_INT_MAX_ELTS \
-  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
-   / HOST_BITS_PER_WIDE_INT)
+  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)    \
+    / HOST_BITS_PER_WIDE_INT))
+

that doesn't reserve 1 more bit, it just rounds up.  For one more bit you
need to add 1, so

 #define WIDE_INT_MAX_ELTS \
+  (((MAX_BITSIZE_MODE_ANY_INT + 1 + HOST_BITS_PER_WIDE_INT - 1)    \
+    / HOST_BITS_PER_WIDE_INT))

no?

Otherwise ok.

Thanks,
Richard.

> kenny
>
>
>>> It's not like I can approve the patch anyway though, so I'll leave it
>>> there.
>>>
>>> Thanks,
>>> Richard
>>
>>
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-09 15:01                   ` Richard Biener
@ 2013-12-09 15:08                     ` Kenneth Zadeck
  2013-12-09 15:12                     ` Richard Sandiford
  2013-12-11 17:46                     ` Kenneth Zadeck
  2 siblings, 0 replies; 25+ messages in thread
From: Kenneth Zadeck @ 2013-12-09 15:08 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Sandiford, gcc-patches, Mike Stump

On 12/09/2013 10:01 AM, Richard Biener wrote:
> On Mon, Dec 9, 2013 at 3:49 PM, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
>> On 12/08/2013 05:35 AM, Richard Biener wrote:
>>> Richard Sandiford <rdsandiford@googlemail.com> wrote:
>>>> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>>>>>>>>      #define WIDE_INT_MAX_ELTS \
>>>>>>>>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>>>>>>>> -   / HOST_BITS_PER_WIDE_INT)
>>>>>>>>> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)    \
>>>>>>>>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>>>>>>>> I think this should be:
>>>>>>>>
>>>>>>>>       (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>>>>>>>>
>>>>>>>> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact
>>>> multiple
>>>>>>>> of HOST_BITS_PER_WIDE_INT.
>>>>>>> we will do this later when some other issues that Eric B raised are
>>>> settled.
>>>>>> I think you're talking about defining MAX_BITSIZE_MODE_ANY_INT as
>>>>>> MAX_SIZE_MODE_ANY_INT * BITS_PER_UNIT, but that's orthogonal to the
>>>>>> change above.  IMO it doesn't make sense to both round up the
>>>> division
>>>>>> and also add 1 to the result.  So I think we should make this change
>>>>>> regardless of whatever follows.
>>>>>>
>>>>>> Looks good to me otherwise, thanks.
>>>>>>
>>>>>> Richard
>>>>>>
>>>>> so this one works the way you want.    While it is true the the
>>>> problems
>>>>> are disjoint, the solution will likely evolving change the same lines
>>>> of
>>>>> source in two different ways.
>>>> Well, if we're changing the definition of WIDE_INT_MAX_ELTS now (and we
>>>> are)
>>>> I think we should change it to something that makes sense.  All we want
>>>> is
>>>> 1 extra bit.  We don't need to round up the size and then add a whole
>>>> HWI on top.  Doing that will potentially pessimise targets that don't
>>>> need anything beyond SImode.
>>> I agree.
>>>
>>> Richard.
>> Done, ok to commit?
> +   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
> +   can accomodate at least 1 more bit so that unsigned numbers of that
> +   mode can be represented.  Note that it is still possible to create
> +   fixed_wide_ints that have precisions greater than
> +   MAX_BITSIZE_MODE_ANY_INT.  This can be useful when representing a
> +   double-width multiplication result, for example.  */
>   #define WIDE_INT_MAX_ELTS \
> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
> -   / HOST_BITS_PER_WIDE_INT)
> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)    \
> +    / HOST_BITS_PER_WIDE_INT))
> +
>
> that doesn't reserve 1 more bit, it just rounds up.  For one more bit you
> need to add 1, so
>
>   #define WIDE_INT_MAX_ELTS \
> +  (((MAX_BITSIZE_MODE_ANY_INT + 1 + HOST_BITS_PER_WIDE_INT - 1)    \
> +    / HOST_BITS_PER_WIDE_INT))
>
> no?

oops?   i am testing it correctly this time.   Will report back in an hour.


kenny
>
> Otherwise ok.
>
> Thanks,
> Richard.
>
>> kenny
>>
>>
>>>> It's not like I can approve the patch anyway though, so I'll leave it
>>>> there.
>>>>
>>>> Thanks,
>>>> Richard
>>>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-09 15:01                   ` Richard Biener
  2013-12-09 15:08                     ` Kenneth Zadeck
@ 2013-12-09 15:12                     ` Richard Sandiford
  2013-12-09 17:22                       ` Kenneth Zadeck
  2013-12-11 17:46                     ` Kenneth Zadeck
  2 siblings, 1 reply; 25+ messages in thread
From: Richard Sandiford @ 2013-12-09 15:12 UTC (permalink / raw)
  To: Richard Biener; +Cc: Kenneth Zadeck, gcc-patches, Mike Stump

Richard Biener <richard.guenther@gmail.com> writes:
> On Mon, Dec 9, 2013 at 3:49 PM, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
>> On 12/08/2013 05:35 AM, Richard Biener wrote:
>>>
>>> Richard Sandiford <rdsandiford@googlemail.com> wrote:
>>>>
>>>> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>>>>>>>>
>>>>>>>>>     #define WIDE_INT_MAX_ELTS \
>>>>>>>>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>>>>>>>> -   / HOST_BITS_PER_WIDE_INT)
>>>>>>>>> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)    \
>>>>>>>>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>>>>>>>>
>>>>>>>> I think this should be:
>>>>>>>>
>>>>>>>>      (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>>>>>>>>
>>>>>>>> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact
>>>>
>>>> multiple
>>>>>>>>
>>>>>>>> of HOST_BITS_PER_WIDE_INT.
>>>>>>>
>>>>>>> we will do this later when some other issues that Eric B raised are
>>>>
>>>> settled.
>>>>>>
>>>>>> I think you're talking about defining MAX_BITSIZE_MODE_ANY_INT as
>>>>>> MAX_SIZE_MODE_ANY_INT * BITS_PER_UNIT, but that's orthogonal to the
>>>>>> change above.  IMO it doesn't make sense to both round up the
>>>>
>>>> division
>>>>>>
>>>>>> and also add 1 to the result.  So I think we should make this change
>>>>>> regardless of whatever follows.
>>>>>>
>>>>>> Looks good to me otherwise, thanks.
>>>>>>
>>>>>> Richard
>>>>>>
>>>>> so this one works the way you want.    While it is true the the
>>>>
>>>> problems
>>>>>
>>>>> are disjoint, the solution will likely evolving change the same lines
>>>>
>>>> of
>>>>>
>>>>> source in two different ways.
>>>>
>>>> Well, if we're changing the definition of WIDE_INT_MAX_ELTS now (and we
>>>> are)
>>>> I think we should change it to something that makes sense.  All we want
>>>> is
>>>> 1 extra bit.  We don't need to round up the size and then add a whole
>>>> HWI on top.  Doing that will potentially pessimise targets that don't
>>>> need anything beyond SImode.
>>>
>>> I agree.
>>>
>>> Richard.
>>
>> Done, ok to commit?
>
> +   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
> +   can accomodate at least 1 more bit so that unsigned numbers of that
> +   mode can be represented.  Note that it is still possible to create
> +   fixed_wide_ints that have precisions greater than
> +   MAX_BITSIZE_MODE_ANY_INT.  This can be useful when representing a
> +   double-width multiplication result, for example.  */
>  #define WIDE_INT_MAX_ELTS \
> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
> -   / HOST_BITS_PER_WIDE_INT)
> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)    \
> +    / HOST_BITS_PER_WIDE_INT))
> +
>
> that doesn't reserve 1 more bit, it just rounds up.  For one more bit you
> need to add 1, so
>
>  #define WIDE_INT_MAX_ELTS \
> +  (((MAX_BITSIZE_MODE_ANY_INT + 1 + HOST_BITS_PER_WIDE_INT - 1)    \
> +    / HOST_BITS_PER_WIDE_INT))
>
> no?

Or just:

   (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)

as above.

Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-09 15:12                     ` Richard Sandiford
@ 2013-12-09 17:22                       ` Kenneth Zadeck
  0 siblings, 0 replies; 25+ messages in thread
From: Kenneth Zadeck @ 2013-12-09 17:22 UTC (permalink / raw)
  To: Richard Biener, gcc-patches, Mike Stump, rsandifo

[-- Attachment #1: Type: text/plain, Size: 3111 bytes --]

On 12/09/2013 10:12 AM, Richard Sandiford wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Mon, Dec 9, 2013 at 3:49 PM, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
>>> On 12/08/2013 05:35 AM, Richard Biener wrote:
>>>> Richard Sandiford <rdsandiford@googlemail.com> wrote:
>>>>> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>>>>>>>>>      #define WIDE_INT_MAX_ELTS \
>>>>>>>>>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>>>>>>>>> -   / HOST_BITS_PER_WIDE_INT)
>>>>>>>>>> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)    \
>>>>>>>>>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>>>>>>>>> I think this should be:
>>>>>>>>>
>>>>>>>>>       (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>>>>>>>>>
>>>>>>>>> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact
>>>>> multiple
>>>>>>>>> of HOST_BITS_PER_WIDE_INT.
>>>>>>>> we will do this later when some other issues that Eric B raised are
>>>>> settled.
>>>>>>> I think you're talking about defining MAX_BITSIZE_MODE_ANY_INT as
>>>>>>> MAX_SIZE_MODE_ANY_INT * BITS_PER_UNIT, but that's orthogonal to the
>>>>>>> change above.  IMO it doesn't make sense to both round up the
>>>>> division
>>>>>>> and also add 1 to the result.  So I think we should make this change
>>>>>>> regardless of whatever follows.
>>>>>>>
>>>>>>> Looks good to me otherwise, thanks.
>>>>>>>
>>>>>>> Richard
>>>>>>>
>>>>>> so this one works the way you want.    While it is true the the
>>>>> problems
>>>>>> are disjoint, the solution will likely evolving change the same lines
>>>>> of
>>>>>> source in two different ways.
>>>>> Well, if we're changing the definition of WIDE_INT_MAX_ELTS now (and we
>>>>> are)
>>>>> I think we should change it to something that makes sense.  All we want
>>>>> is
>>>>> 1 extra bit.  We don't need to round up the size and then add a whole
>>>>> HWI on top.  Doing that will potentially pessimise targets that don't
>>>>> need anything beyond SImode.
>>>> I agree.
>>>>
>>>> Richard.
>>> Done, ok to commit?
>> +   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
>> +   can accomodate at least 1 more bit so that unsigned numbers of that
>> +   mode can be represented.  Note that it is still possible to create
>> +   fixed_wide_ints that have precisions greater than
>> +   MAX_BITSIZE_MODE_ANY_INT.  This can be useful when representing a
>> +   double-width multiplication result, for example.  */
>>   #define WIDE_INT_MAX_ELTS \
>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>> -   / HOST_BITS_PER_WIDE_INT)
>> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)    \
>> +    / HOST_BITS_PER_WIDE_INT))
>> +
>>
>> that doesn't reserve 1 more bit, it just rounds up.  For one more bit you
>> need to add 1, so
>>
>>   #define WIDE_INT_MAX_ELTS \
>> +  (((MAX_BITSIZE_MODE_ANY_INT + 1 + HOST_BITS_PER_WIDE_INT - 1)    \
>> +    / HOST_BITS_PER_WIDE_INT))
>>
>> no?
> Or just:
>
>     (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>
> as above.
>
> Richard
>
This time for sure.   Tested on x86-64

kenny

[-- Attachment #2: MulVRP4.diff --]
[-- Type: text/x-patch, Size: 5601 bytes --]

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 205726)
+++ gcc/tree-vrp.c	(working copy)
@@ -2620,23 +2620,24 @@ extract_range_from_binary_expr_1 (value_
 
       signop sign = TYPE_SIGN (expr_type);
       unsigned int prec = TYPE_PRECISION (expr_type);
-      unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
 
       if (range_int_cst_p (&vr0)
 	  && range_int_cst_p (&vr1)
 	  && TYPE_OVERFLOW_WRAPS (expr_type))
 	{
-	  wide_int sizem1 = wi::mask (prec, false, prec2);
-	  wide_int size = sizem1 + 1;
+	  typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int;
+	  typedef generic_wide_int
+             <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
+	  vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
+	  vrp_int size = sizem1 + 1;
 
 	  /* Extend the values using the sign of the result to PREC2.
 	     From here on out, everthing is just signed math no matter
 	     what the input types were.  */
-	  wide_int min0 = wide_int::from (vr0.min, prec2, sign);
-	  wide_int max0 = wide_int::from (vr0.max, prec2, sign);
-	  wide_int min1 = wide_int::from (vr1.min, prec2, sign);
-	  wide_int max1 = wide_int::from (vr1.max, prec2, sign);
-
+          vrp_int min0 = vrp_int_cst (vr0.min);
+          vrp_int max0 = vrp_int_cst (vr0.max);
+          vrp_int min1 = vrp_int_cst (vr1.min);
+          vrp_int max1 = vrp_int_cst (vr1.max);
 	  /* Canonicalize the intervals.  */
 	  if (sign == UNSIGNED)
 	    {
@@ -2653,17 +2654,17 @@ extract_range_from_binary_expr_1 (value_
 		}
 	    }
 
-	  wide_int prod0 = min0 * min1;
-	  wide_int prod1 = min0 * max1;
-	  wide_int prod2 = max0 * min1;
-	  wide_int prod3 = max0 * max1;
+	  vrp_int prod0 = min0 * min1;
+	  vrp_int prod1 = min0 * max1;
+	  vrp_int prod2 = max0 * min1;
+	  vrp_int prod3 = max0 * max1;
 
 	  /* Sort the 4 products so that min is in prod0 and max is in
 	     prod3.  */
 	  /* min0min1 > max0max1 */
 	  if (wi::gts_p (prod0, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod0;
 	      prod0 = tmp;
 	    }
@@ -2671,21 +2672,21 @@ extract_range_from_binary_expr_1 (value_
 	  /* min0max1 > max0min1 */
 	  if (wi::gts_p (prod1, prod2))
 	    {
-	      wide_int tmp = prod2;
+	      vrp_int tmp = prod2;
 	      prod2 = prod1;
 	      prod1 = tmp;
 	    }
 
 	  if (wi::gts_p (prod0, prod1))
 	    {
-	      wide_int tmp = prod1;
+	      vrp_int tmp = prod1;
 	      prod1 = prod0;
 	      prod0 = tmp;
 	    }
 
 	  if (wi::gts_p (prod2, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod2;
 	      prod2 = tmp;
 	    }
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 205726)
+++ gcc/tree.h	(working copy)
@@ -4549,7 +4549,7 @@ namespace wi
     static const unsigned int precision = N;
   };
 
-  generic_wide_int <extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+  generic_wide_int <extended_tree <WIDE_INT_MAX_PRECISION> >
   to_widest (const_tree);
 
   generic_wide_int <extended_tree <ADDR_MAX_PRECISION> > to_offset (const_tree);
@@ -4570,7 +4570,7 @@ wi::int_traits <const_tree>::decompose (
 			  precision);
 }
 
-inline generic_wide_int <wi::extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+inline generic_wide_int <wi::extended_tree <WIDE_INT_MAX_PRECISION> >
 wi::to_widest (const_tree t)
 {
   return t;
@@ -4609,7 +4609,7 @@ wi::extended_tree <N>::get_len () const
 {
   if (N == ADDR_MAX_PRECISION)
     return TREE_INT_CST_OFFSET_NUNITS (m_t);
-  else if (N == MAX_BITSIZE_MODE_ANY_INT)
+  else if (N >= WIDE_INT_MAX_PRECISION)
     return TREE_INT_CST_EXT_NUNITS (m_t);
   else
     /* This class is designed to be used for specific output precisions
Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h	(revision 205726)
+++ gcc/wide-int.h	(working copy)
@@ -228,15 +228,16 @@ along with GCC; see the file COPYING3.
 #endif
 
 /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
-   early examination of the target's mode file.  Thus it is safe that
-   some small multiple of this number is easily larger than any number
-   that that target could compute.  The place in the compiler that
-   currently needs the widest ints is the code that determines the
-   range of a multiply.  This code needs 2n + 2 bits.  */
-
+   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
+   can accomodate at least 1 more bit so that unsigned numbers of that
+   mode can be represented.  Note that it is still possible to create
+   fixed_wide_ints that have precisions greater than
+   MAX_BITSIZE_MODE_ANY_INT.  This can be useful when representing a
+   double-width multiplication result, for example.  */
 #define WIDE_INT_MAX_ELTS \
-  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
-   / HOST_BITS_PER_WIDE_INT)
+  ((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT) / HOST_BITS_PER_WIDE_INT)
+
+#define WIDE_INT_MAX_PRECISION (WIDE_INT_MAX_ELTS * HOST_BITS_PER_WIDE_INT)
 
 /* This is the max size of any pointer on any machine.  It does not
    seem to be as easy to sniff this out of the machine description as
@@ -294,7 +295,7 @@ struct wide_int_storage;
 
 typedef generic_wide_int <wide_int_storage> wide_int;
 typedef FIXED_WIDE_INT (ADDR_MAX_PRECISION) offset_int;
-typedef FIXED_WIDE_INT (MAX_BITSIZE_MODE_ANY_INT) widest_int;
+typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION) widest_int;
 
 template <bool SE>
 struct wide_int_ref_storage;

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [wide-int] small cleanup in wide-int.*
  2013-12-09 15:01                   ` Richard Biener
  2013-12-09 15:08                     ` Kenneth Zadeck
  2013-12-09 15:12                     ` Richard Sandiford
@ 2013-12-11 17:46                     ` Kenneth Zadeck
  2 siblings, 0 replies; 25+ messages in thread
From: Kenneth Zadeck @ 2013-12-11 17:46 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Sandiford, gcc-patches, Mike Stump

[-- Attachment #1: Type: text/plain, Size: 3074 bytes --]

On 12/09/2013 10:01 AM, Richard Biener wrote:
> On Mon, Dec 9, 2013 at 3:49 PM, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
>> On 12/08/2013 05:35 AM, Richard Biener wrote:
>>> Richard Sandiford <rdsandiford@googlemail.com> wrote:
>>>> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>>>>>>>>      #define WIDE_INT_MAX_ELTS \
>>>>>>>>> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
>>>>>>>>> -   / HOST_BITS_PER_WIDE_INT)
>>>>>>>>> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)    \
>>>>>>>>> +    / HOST_BITS_PER_WIDE_INT) + 1)
>>>>>>>> I think this should be:
>>>>>>>>
>>>>>>>>       (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1)
>>>>>>>>
>>>>>>>> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact
>>>> multiple
>>>>>>>> of HOST_BITS_PER_WIDE_INT.
>>>>>>> we will do this later when some other issues that Eric B raised are
>>>> settled.
>>>>>> I think you're talking about defining MAX_BITSIZE_MODE_ANY_INT as
>>>>>> MAX_SIZE_MODE_ANY_INT * BITS_PER_UNIT, but that's orthogonal to the
>>>>>> change above.  IMO it doesn't make sense to both round up the
>>>> division
>>>>>> and also add 1 to the result.  So I think we should make this change
>>>>>> regardless of whatever follows.
>>>>>>
>>>>>> Looks good to me otherwise, thanks.
>>>>>>
>>>>>> Richard
>>>>>>
>>>>> so this one works the way you want.    While it is true the the
>>>> problems
>>>>> are disjoint, the solution will likely evolving change the same lines
>>>> of
>>>>> source in two different ways.
>>>> Well, if we're changing the definition of WIDE_INT_MAX_ELTS now (and we
>>>> are)
>>>> I think we should change it to something that makes sense.  All we want
>>>> is
>>>> 1 extra bit.  We don't need to round up the size and then add a whole
>>>> HWI on top.  Doing that will potentially pessimise targets that don't
>>>> need anything beyond SImode.
>>> I agree.
>>>
>>> Richard.
>> Done, ok to commit?
> +   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
> +   can accomodate at least 1 more bit so that unsigned numbers of that
> +   mode can be represented.  Note that it is still possible to create
> +   fixed_wide_ints that have precisions greater than
> +   MAX_BITSIZE_MODE_ANY_INT.  This can be useful when representing a
> +   double-width multiplication result, for example.  */
>   #define WIDE_INT_MAX_ELTS \
> -  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
> -   / HOST_BITS_PER_WIDE_INT)
> +  (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1)    \
> +    / HOST_BITS_PER_WIDE_INT))
> +
>
> that doesn't reserve 1 more bit, it just rounds up.  For one more bit you
> need to add 1, so
>
>   #define WIDE_INT_MAX_ELTS \
> +  (((MAX_BITSIZE_MODE_ANY_INT + 1 + HOST_BITS_PER_WIDE_INT - 1)    \
> +    / HOST_BITS_PER_WIDE_INT))
>
> no?
>
> Otherwise ok.
>
> Thanks,
> Richard.
>
>> kenny
>>
>>
>>>> It's not like I can approve the patch anyway though, so I'll leave it
>>>> there.
>>>>
>>>> Thanks,
>>>> Richard
>>>
committed as revision 205900 after rebootstrap.

kenny

[-- Attachment #2: MulVRP5.diff --]
[-- Type: text/x-patch, Size: 5601 bytes --]

Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 205897)
+++ gcc/tree.h	(working copy)
@@ -4545,7 +4545,7 @@ namespace wi
     static const unsigned int precision = N;
   };
 
-  generic_wide_int <extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+  generic_wide_int <extended_tree <WIDE_INT_MAX_PRECISION> >
   to_widest (const_tree);
 
   generic_wide_int <extended_tree <ADDR_MAX_PRECISION> > to_offset (const_tree);
@@ -4566,7 +4566,7 @@ wi::int_traits <const_tree>::decompose (
 			  precision);
 }
 
-inline generic_wide_int <wi::extended_tree <MAX_BITSIZE_MODE_ANY_INT> >
+inline generic_wide_int <wi::extended_tree <WIDE_INT_MAX_PRECISION> >
 wi::to_widest (const_tree t)
 {
   return t;
@@ -4605,7 +4605,7 @@ wi::extended_tree <N>::get_len () const
 {
   if (N == ADDR_MAX_PRECISION)
     return TREE_INT_CST_OFFSET_NUNITS (m_t);
-  else if (N == MAX_BITSIZE_MODE_ANY_INT)
+  else if (N >= WIDE_INT_MAX_PRECISION)
     return TREE_INT_CST_EXT_NUNITS (m_t);
   else
     /* This class is designed to be used for specific output precisions
Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 205897)
+++ gcc/tree-vrp.c	(working copy)
@@ -2620,23 +2620,24 @@ extract_range_from_binary_expr_1 (value_
 
       signop sign = TYPE_SIGN (expr_type);
       unsigned int prec = TYPE_PRECISION (expr_type);
-      unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0);
 
       if (range_int_cst_p (&vr0)
 	  && range_int_cst_p (&vr1)
 	  && TYPE_OVERFLOW_WRAPS (expr_type))
 	{
-	  wide_int sizem1 = wi::mask (prec, false, prec2);
-	  wide_int size = sizem1 + 1;
+	  typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int;
+	  typedef generic_wide_int
+             <wi::extended_tree <WIDE_INT_MAX_PRECISION * 2> > vrp_int_cst;
+	  vrp_int sizem1 = wi::mask <vrp_int> (prec, false);
+	  vrp_int size = sizem1 + 1;
 
 	  /* Extend the values using the sign of the result to PREC2.
 	     From here on out, everthing is just signed math no matter
 	     what the input types were.  */
-	  wide_int min0 = wide_int::from (vr0.min, prec2, sign);
-	  wide_int max0 = wide_int::from (vr0.max, prec2, sign);
-	  wide_int min1 = wide_int::from (vr1.min, prec2, sign);
-	  wide_int max1 = wide_int::from (vr1.max, prec2, sign);
-
+          vrp_int min0 = vrp_int_cst (vr0.min);
+          vrp_int max0 = vrp_int_cst (vr0.max);
+          vrp_int min1 = vrp_int_cst (vr1.min);
+          vrp_int max1 = vrp_int_cst (vr1.max);
 	  /* Canonicalize the intervals.  */
 	  if (sign == UNSIGNED)
 	    {
@@ -2653,17 +2654,17 @@ extract_range_from_binary_expr_1 (value_
 		}
 	    }
 
-	  wide_int prod0 = min0 * min1;
-	  wide_int prod1 = min0 * max1;
-	  wide_int prod2 = max0 * min1;
-	  wide_int prod3 = max0 * max1;
+	  vrp_int prod0 = min0 * min1;
+	  vrp_int prod1 = min0 * max1;
+	  vrp_int prod2 = max0 * min1;
+	  vrp_int prod3 = max0 * max1;
 
 	  /* Sort the 4 products so that min is in prod0 and max is in
 	     prod3.  */
 	  /* min0min1 > max0max1 */
 	  if (wi::gts_p (prod0, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod0;
 	      prod0 = tmp;
 	    }
@@ -2671,21 +2672,21 @@ extract_range_from_binary_expr_1 (value_
 	  /* min0max1 > max0min1 */
 	  if (wi::gts_p (prod1, prod2))
 	    {
-	      wide_int tmp = prod2;
+	      vrp_int tmp = prod2;
 	      prod2 = prod1;
 	      prod1 = tmp;
 	    }
 
 	  if (wi::gts_p (prod0, prod1))
 	    {
-	      wide_int tmp = prod1;
+	      vrp_int tmp = prod1;
 	      prod1 = prod0;
 	      prod0 = tmp;
 	    }
 
 	  if (wi::gts_p (prod2, prod3))
 	    {
-	      wide_int tmp = prod3;
+	      vrp_int tmp = prod3;
 	      prod3 = prod2;
 	      prod2 = tmp;
 	    }
Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h	(revision 205897)
+++ gcc/wide-int.h	(working copy)
@@ -228,15 +228,16 @@ along with GCC; see the file COPYING3.
 #endif
 
 /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very
-   early examination of the target's mode file.  Thus it is safe that
-   some small multiple of this number is easily larger than any number
-   that that target could compute.  The place in the compiler that
-   currently needs the widest ints is the code that determines the
-   range of a multiply.  This code needs 2n + 2 bits.  */
-
+   early examination of the target's mode file.  The WIDE_INT_MAX_ELTS
+   can accomodate at least 1 more bit so that unsigned numbers of that
+   mode can be represented.  Note that it is still possible to create
+   fixed_wide_ints that have precisions greater than
+   MAX_BITSIZE_MODE_ANY_INT.  This can be useful when representing a
+   double-width multiplication result, for example.  */
 #define WIDE_INT_MAX_ELTS \
-  ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \
-   / HOST_BITS_PER_WIDE_INT)
+  ((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT) / HOST_BITS_PER_WIDE_INT)
+
+#define WIDE_INT_MAX_PRECISION (WIDE_INT_MAX_ELTS * HOST_BITS_PER_WIDE_INT)
 
 /* This is the max size of any pointer on any machine.  It does not
    seem to be as easy to sniff this out of the machine description as
@@ -294,7 +295,7 @@ struct wide_int_storage;
 
 typedef generic_wide_int <wide_int_storage> wide_int;
 typedef FIXED_WIDE_INT (ADDR_MAX_PRECISION) offset_int;
-typedef FIXED_WIDE_INT (MAX_BITSIZE_MODE_ANY_INT) widest_int;
+typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION) widest_int;
 
 template <bool SE>
 struct wide_int_ref_storage;

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2013-12-11 17:46 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-28 23:04 [wide-int] small cleanup in wide-int.* Kenneth Zadeck
2013-11-29 11:58 ` Richard Biener
2013-11-29 12:51   ` Richard Sandiford
2013-11-29 13:23     ` Richard Biener
2013-11-29 17:06   ` Kenneth Zadeck
2013-11-30 18:06   ` Kenneth Zadeck
2013-12-02 10:50     ` Richard Biener
2013-12-02 19:57       ` Kenneth Zadeck
2013-12-03 16:20   ` Kenneth Zadeck
2013-12-03 16:52     ` Richard Sandiford
2013-12-03 18:32       ` Mike Stump
2013-12-06 16:45       ` Kenneth Zadeck
2013-12-06 18:14         ` Kenneth Zadeck
2013-12-06 18:32         ` Richard Sandiford
2013-12-06 22:27           ` Kenneth Zadeck
2013-12-07 10:40             ` Richard Sandiford
2013-12-08 10:33               ` Richard Biener
2013-12-09 14:49                 ` Kenneth Zadeck
2013-12-09 15:01                   ` Richard Biener
2013-12-09 15:08                     ` Kenneth Zadeck
2013-12-09 15:12                     ` Richard Sandiford
2013-12-09 17:22                       ` Kenneth Zadeck
2013-12-11 17:46                     ` Kenneth Zadeck
2013-11-29 13:39 ` Richard Sandiford
2013-11-29 19:32   ` Kenneth Zadeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).