diff mbox series

lightweight class to wide int ranges in VRP and friends

Message ID dd9e25da-847c-e13a-f5cc-280ec2b152da@redhat.com
State New
Headers show
Series lightweight class to wide int ranges in VRP and friends | expand

Commit Message

Aldy Hernandez Aug. 15, 2018, 5:31 p.m. UTC 
I kept seeing the same patterns over and over in all this re-factoring:

1. extract value_range constants into pairs of wide ints

2. mimic symbolics as [-MIN, +MAX] (most of the time :))

3. perform some wide int operation on the wide int range

4. convert back to a value range

So I've decided to clean this up even more.  Instead of passing a pair 
of wide ints plus sign, precision, and god knows what to every 
wide_int_range_* function, I've implemented a lighweight class 
(wi_range) for a pair of wide ints.  It is not meant for long term 
storage, but even so its footprint is minimal.

The only extra bits are another 64-bit word per pair for keeping 
attributes such as precision, sign, overflow_wraps, and a few other 
things we'll need shortly.  In reality we only need one set of 
attributes for both sets of pairs, so we waste one 64-bit word.  I don't 
care.  They're short lived, and the resulting code looks an awful lot 
nicer.  For example, the shift code gets rewritten from this:

       if (range_int_cst_p (&vr1)
	  && !vrp_shift_undefined_p (vr1, prec))
	{
	  if (code == RSHIFT_EXPR)
	    {
	      if (vr0.type != VR_RANGE || symbolic_range_p (&vr0))
		{
		  vr0.type = type = VR_RANGE;
		  vr0.min = vrp_val_min (expr_type);
		  vr0.max = vrp_val_max (expr_type);
		}
	      extract_range_from_multiplicative_op (vr, code, &vr0, &vr1);
	      return;
	    }
	  else if (code == LSHIFT_EXPR
		   && range_int_cst_p (&vr0))
	    {
	      wide_int res_lb, res_ub;
	      if (wide_int_range_lshift (res_lb, res_ub, sign, prec,
					 wi::to_wide (vr0.min),
					 wi::to_wide (vr0.max),
					 wi::to_wide (vr1.min),
					 wi::to_wide (vr1.max),
					 TYPE_OVERFLOW_UNDEFINED (expr_type),
					 TYPE_OVERFLOW_WRAPS (expr_type)))
		{
		  min = wide_int_to_tree (expr_type, res_lb);
		  max = wide_int_to_tree (expr_type, res_ub);
		  set_and_canonicalize_value_range (vr, VR_RANGE,
						    min, max, NULL);
		  return;
		}
	    }
	}
       set_value_range_to_varying (vr);

into this:

       wi_range w0 (&vr0, expr_type);
       wi_range w1 (&vr1, expr_type);
       if (!range_int_cst_p (&vr1)
	  || wi_range_shift_undefined_p (w1)
	  || (code == LSHIFT_EXPR
	      && !range_int_cst_p (&vr0)))
	{
	  vr->set_varying ();
	  return;
	}
       bool success;
       if (code == RSHIFT_EXPR)
	success = wi_range_multiplicative_op (res, code, w0, w1);
       else
	success = wi_range_lshift (res, w0, w1);

       if (success)
	vr->set_and_canonicalize (res, expr_type);
       else
	vr->set_varying ();
       return;

I also munged together the maybe/mustbe nonzero_bits into one structure.

Finally, I've pontificated that wide_int_range_blah is too much typing. 
Everyone's RSI will thank me for rewriting the methods as wi_range_blah.

I've tried to keep the functionality changes to a minimum.  However, 
there are some slight changes where I treat symbolics and VR_VARYING as 
[MIN, MAX] and let the constant code do its thing.  It is considerably 
easier to read.

I am finally happy with the overall direction of this.  If this and the 
last one are acceptable, I think I only need to clean MIN/MAX_EXPR and 
ABS_EXPR and I'm basically done with what I need going forward.

Phew...

How does this look?

Aldy

Comments

Richard Biener Aug. 24, 2018, 10:32 a.m. UTC | #1 
On Wed, Aug 15, 2018 at 7:31 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> I kept seeing the same patterns over and over in all this re-factoring:
>
> 1. extract value_range constants into pairs of wide ints
>
> 2. mimic symbolics as [-MIN, +MAX] (most of the time :))
>
> 3. perform some wide int operation on the wide int range
>
> 4. convert back to a value range
>
> So I've decided to clean this up even more.  Instead of passing a pair
> of wide ints plus sign, precision, and god knows what to every
> wide_int_range_* function, I've implemented a lighweight class
> (wi_range) for a pair of wide ints.  It is not meant for long term
> storage, but even so its footprint is minimal.
>
> The only extra bits are another 64-bit word per pair for keeping
> attributes such as precision, sign, overflow_wraps, and a few other
> things we'll need shortly.  In reality we only need one set of
> attributes for both sets of pairs, so we waste one 64-bit word.  I don't
> care.  They're short lived, and the resulting code looks an awful lot
> nicer.  For example, the shift code gets rewritten from this:
>
>        if (range_int_cst_p (&vr1)
>           && !vrp_shift_undefined_p (vr1, prec))
>         {
>           if (code == RSHIFT_EXPR)
>             {
>               if (vr0.type != VR_RANGE || symbolic_range_p (&vr0))
>                 {
>                   vr0.type = type = VR_RANGE;
>                   vr0.min = vrp_val_min (expr_type);
>                   vr0.max = vrp_val_max (expr_type);
>                 }
>               extract_range_from_multiplicative_op (vr, code, &vr0, &vr1);
>               return;
>             }
>           else if (code == LSHIFT_EXPR
>                    && range_int_cst_p (&vr0))
>             {
>               wide_int res_lb, res_ub;
>               if (wide_int_range_lshift (res_lb, res_ub, sign, prec,
>                                          wi::to_wide (vr0.min),
>                                          wi::to_wide (vr0.max),
>                                          wi::to_wide (vr1.min),
>                                          wi::to_wide (vr1.max),
>                                          TYPE_OVERFLOW_UNDEFINED (expr_type),
>                                          TYPE_OVERFLOW_WRAPS (expr_type)))
>                 {
>                   min = wide_int_to_tree (expr_type, res_lb);
>                   max = wide_int_to_tree (expr_type, res_ub);
>                   set_and_canonicalize_value_range (vr, VR_RANGE,
>                                                     min, max, NULL);
>                   return;
>                 }
>             }
>         }
>        set_value_range_to_varying (vr);
>
> into this:
>
>        wi_range w0 (&vr0, expr_type);
>        wi_range w1 (&vr1, expr_type);
>        if (!range_int_cst_p (&vr1)
>           || wi_range_shift_undefined_p (w1)
>           || (code == LSHIFT_EXPR
>               && !range_int_cst_p (&vr0)))
>         {
>           vr->set_varying ();
>           return;
>         }
>        bool success;
>        if (code == RSHIFT_EXPR)
>         success = wi_range_multiplicative_op (res, code, w0, w1);
>        else
>         success = wi_range_lshift (res, w0, w1);
>
>        if (success)
>         vr->set_and_canonicalize (res, expr_type);
>        else
>         vr->set_varying ();
>        return;

not sure whether I like the munging of lshift and right shift (what exactly gets
done is less clear in your version ...).  Having a light-weigth class for
the wi_range_ parameters is nice though.

> I also munged together the maybe/mustbe nonzero_bits into one structure.
>
> Finally, I've pontificated that wide_int_range_blah is too much typing.
> Everyone's RSI will thank me for rewriting the methods as wi_range_blah.
>
> I've tried to keep the functionality changes to a minimum.  However,
> there are some slight changes where I treat symbolics and VR_VARYING as
> [MIN, MAX] and let the constant code do its thing.  It is considerably
> easier to read.
>
> I am finally happy with the overall direction of this.  If this and the
> last one are acceptable, I think I only need to clean MIN/MAX_EXPR and
> ABS_EXPR and I'm basically done with what I need going forward.
>
> Phew...
>
> How does this look?

+struct wi_range
+{
+  wide_int min;
+  wide_int max;
+  /* This structure takes one additional 64-bit word apart from the
+     min/max bounds above.  It is laid out so that PRECISION and SIGN
+     can be accessed without any bit twiddling, since they're the most
+     commonly accessed fields.  */
+  unsigned short precision;
+  bool empty_p:1;
+  bool pointer_type_p:1;
+  bool overflow_wraps:1;
+  bool overflow_undefined:1;
+  signop sign;

isn't precision already available in min.get_precision () which is
required to be equal to max.get_precision ()?

+  wi_range () { empty_p = false; }

true I suppose?  Better not allow default construction?

empty_p doesn't seem to be documented, nor is pointer_type_p
or why that is necessary - maybe simply store a tree type
instead all of the bits?

+/* A structure to pass maybe and mustbe nonzero bitmasks.  This is for
+   use in wi_range_set_zero_nonzero_bits and friends.  */
+
+struct maybe_nonzero_bits
+{
+  /* If some bit is unset, it means that for all numbers in the range
+   the bit is 0, otherwise it might be 0 or 1.  */
+  wide_int may_be;
+
+  /* If some bit is set, it means that for all numbers in the range
+   the bit is 1, otherwise it might be 0 or 1.  */
+  wide_int must_be;
 };

eh - multiple changes in one patch ...

@@ -52,6 +54,58 @@ struct GTY((for_user)) value_range

   /* Dump value range to stderr.  */
   void dump ();
+
+  void set (const wi_range &r, tree type);
+  void set_and_canonicalize (const wi_range &r, tree type);
+  void set_undefined ();
+  void set_varying ();
+};

likewise ... you introduce those methods but not use them consistently.
Please leave out this change (I would object to this kind of change
if carried out through).

+/* Construct a new range from the contents of a value_range.  Varying
+   or symbolic ranges are normalized to the entire possible range.  */
+
+inline
+wi_range::wi_range (value_range *vr, tree type)
+{
+  precision = TYPE_PRECISION (type);
+  pointer_type_p = POINTER_TYPE_P (type);
+  overflow_wraps = TYPE_OVERFLOW_WRAPS (type);
+  overflow_undefined = TYPE_OVERFLOW_UNDEFINED (type);
+  sign = TYPE_SIGN (type);
+  empty_p = false;
+  if (vr->type == VR_VARYING || symbolic_range_p (vr))
+    {
+      min = wi::min_value (precision, sign);
+      max = wi::max_value (precision, sign);
+    }
+  else if (vr->type == VR_RANGE)
+    {
+      min = wi::to_wide (vr->min);
+      max = wi::to_wide (vr->max);
+    }
+  else
+    gcc_unreachable ();
+}

What about VR_ANTI_RANGE?  Note the above doesn't exactly look
"light-weight".

-/* Calculate TRUNC_MOD_EXPR on two ranges and store the result in
-   [WMIN,WMAX].  */
+/* Calculate TRUNC_MOD_EXPR on two ranges and store the result in RES.
+   Return TRUE if operation succeeded.  */

did I say multiple changes...?  I think this isn't a good one given "succeed"
doesn't imply that if not the result is UNDEFINED ...

As said above the shift handlign structrue is now unreadable :/  Given we do
different things for lshift vs. rshift it makes sense to simply split

   else if (code == RSHIFT_EXPR
           || code == LSHIFT_EXPR)
     {

even if that means duplicating a few lines.

Just to remind you review (and approval) isn't easier if you put unrelated stuff
into a patch ;)

Richard.

>
> Aldy
Aldy Hernandez Aug. 24, 2018, 5:40 p.m. UTC | #2 
On 08/24/2018 06:32 AM, Richard Biener wrote:
> On Wed, Aug 15, 2018 at 7:31 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>>
>> I kept seeing the same patterns over and over in all this re-factoring:
>>
>> 1. extract value_range constants into pairs of wide ints
>>
>> 2. mimic symbolics as [-MIN, +MAX] (most of the time :))
>>
>> 3. perform some wide int operation on the wide int range
>>
>> 4. convert back to a value range
>>
>> So I've decided to clean this up even more.  Instead of passing a pair
>> of wide ints plus sign, precision, and god knows what to every
>> wide_int_range_* function, I've implemented a lighweight class
>> (wi_range) for a pair of wide ints.  It is not meant for long term
>> storage, but even so its footprint is minimal.
>>
>> The only extra bits are another 64-bit word per pair for keeping
>> attributes such as precision, sign, overflow_wraps, and a few other
>> things we'll need shortly.  In reality we only need one set of
>> attributes for both sets of pairs, so we waste one 64-bit word.  I don't
>> care.  They're short lived, and the resulting code looks an awful lot
>> nicer.  For example, the shift code gets rewritten from this:
>>
>>         if (range_int_cst_p (&vr1)
>>            && !vrp_shift_undefined_p (vr1, prec))
>>          {
>>            if (code == RSHIFT_EXPR)
>>              {
>>                if (vr0.type != VR_RANGE || symbolic_range_p (&vr0))
>>                  {
>>                    vr0.type = type = VR_RANGE;
>>                    vr0.min = vrp_val_min (expr_type);
>>                    vr0.max = vrp_val_max (expr_type);
>>                  }
>>                extract_range_from_multiplicative_op (vr, code, &vr0, &vr1);
>>                return;
>>              }
>>            else if (code == LSHIFT_EXPR
>>                     && range_int_cst_p (&vr0))
>>              {
>>                wide_int res_lb, res_ub;
>>                if (wide_int_range_lshift (res_lb, res_ub, sign, prec,
>>                                           wi::to_wide (vr0.min),
>>                                           wi::to_wide (vr0.max),
>>                                           wi::to_wide (vr1.min),
>>                                           wi::to_wide (vr1.max),
>>                                           TYPE_OVERFLOW_UNDEFINED (expr_type),
>>                                           TYPE_OVERFLOW_WRAPS (expr_type)))
>>                  {
>>                    min = wide_int_to_tree (expr_type, res_lb);
>>                    max = wide_int_to_tree (expr_type, res_ub);
>>                    set_and_canonicalize_value_range (vr, VR_RANGE,
>>                                                      min, max, NULL);
>>                    return;
>>                  }
>>              }
>>          }
>>         set_value_range_to_varying (vr);
>>
>> into this:
>>
>>         wi_range w0 (&vr0, expr_type);
>>         wi_range w1 (&vr1, expr_type);
>>         if (!range_int_cst_p (&vr1)
>>            || wi_range_shift_undefined_p (w1)
>>            || (code == LSHIFT_EXPR
>>                && !range_int_cst_p (&vr0)))
>>          {
>>            vr->set_varying ();
>>            return;
>>          }
>>         bool success;
>>         if (code == RSHIFT_EXPR)
>>          success = wi_range_multiplicative_op (res, code, w0, w1);
>>         else
>>          success = wi_range_lshift (res, w0, w1);
>>
>>         if (success)
>>          vr->set_and_canonicalize (res, expr_type);
>>         else
>>          vr->set_varying ();
>>         return;
> 
> not sure whether I like the munging of lshift and right shift (what exactly gets
> done is less clear in your version ...).  Having a light-weigth class for
> the wi_range_ parameters is nice though.

No worries.  I'm not emotionally attached to munging them :).

> 
>> I also munged together the maybe/mustbe nonzero_bits into one structure.
>>
>> Finally, I've pontificated that wide_int_range_blah is too much typing.
>> Everyone's RSI will thank me for rewriting the methods as wi_range_blah.
>>
>> I've tried to keep the functionality changes to a minimum.  However,
>> there are some slight changes where I treat symbolics and VR_VARYING as
>> [MIN, MAX] and let the constant code do its thing.  It is considerably
>> easier to read.
>>
>> I am finally happy with the overall direction of this.  If this and the
>> last one are acceptable, I think I only need to clean MIN/MAX_EXPR and
>> ABS_EXPR and I'm basically done with what I need going forward.
>>
>> Phew...
>>
>> How does this look?
> 
> +struct wi_range
> +{
> +  wide_int min;
> +  wide_int max;
> +  /* This structure takes one additional 64-bit word apart from the
> +     min/max bounds above.  It is laid out so that PRECISION and SIGN
> +     can be accessed without any bit twiddling, since they're the most
> +     commonly accessed fields.  */
> +  unsigned short precision;
> +  bool empty_p:1;
> +  bool pointer_type_p:1;
> +  bool overflow_wraps:1;
> +  bool overflow_undefined:1;
> +  signop sign;
> 
> isn't precision already available in min.get_precision () which is
> required to be equal to max.get_precision ()?

Indeed.  Good point.

> 
> +  wi_range () { empty_p = false; }
> 
> true I suppose?  Better not allow default construction?

OK.

> 
> empty_p doesn't seem to be documented, nor is pointer_type_p
> or why that is necessary - maybe simply store a tree type
> instead all of the bits?

I was trying to avoid dereferencing another pointer by perhaps 
flattening out the needed bits, but again-- not emotionally attached. 
Easier for me...

> 
> +/* A structure to pass maybe and mustbe nonzero bitmasks.  This is for
> +   use in wi_range_set_zero_nonzero_bits and friends.  */
> +
> +struct maybe_nonzero_bits
> +{
> +  /* If some bit is unset, it means that for all numbers in the range
> +   the bit is 0, otherwise it might be 0 or 1.  */
> +  wide_int may_be;
> +
> +  /* If some bit is set, it means that for all numbers in the range
> +   the bit is 1, otherwise it might be 0 or 1.  */
> +  wide_int must_be;
>   };
> 
> eh - multiple changes in one patch ...

Well, is it?  I'm trying to avoid passing tons of things to the 
wide_int_range_* functions, and that includes wide ints and the nonzero 
bit pairs.  Could I please leave these in the patch?

> 
> @@ -52,6 +54,58 @@ struct GTY((for_user)) value_range
> 
>     /* Dump value range to stderr.  */
>     void dump ();
> +
> +  void set (const wi_range &r, tree type);
> +  void set_and_canonicalize (const wi_range &r, tree type);
> +  void set_undefined ();
> +  void set_varying ();
> +};
> 
> likewise ... you introduce those methods but not use them consistently.
> Please leave out this change (I would object to this kind of change
> if carried out through).

I was really avoiding having to convert every caller.  But really, why 
would you object even if carried out through?

> 
> +/* Construct a new range from the contents of a value_range.  Varying
> +   or symbolic ranges are normalized to the entire possible range.  */
> +
> +inline
> +wi_range::wi_range (value_range *vr, tree type)
> +{
> +  precision = TYPE_PRECISION (type);
> +  pointer_type_p = POINTER_TYPE_P (type);
> +  overflow_wraps = TYPE_OVERFLOW_WRAPS (type);
> +  overflow_undefined = TYPE_OVERFLOW_UNDEFINED (type);
> +  sign = TYPE_SIGN (type);
> +  empty_p = false;
> +  if (vr->type == VR_VARYING || symbolic_range_p (vr))
> +    {
> +      min = wi::min_value (precision, sign);
> +      max = wi::max_value (precision, sign);
> +    }
> +  else if (vr->type == VR_RANGE)
> +    {
> +      min = wi::to_wide (vr->min);
> +      max = wi::to_wide (vr->max);
> +    }
> +  else
> +    gcc_unreachable ();
> +}
> 
> What about VR_ANTI_RANGE?  Note the above doesn't exactly look
> "light-weight".

Yes, I should handle anti range, or ICE or something, considering the 
pain it was to find the anti range problem with the POINTER_PLUS_EXPR 
code recently.

Is "not exactly light-weight" an objection, or just a btw type of 
comment :)?  Would it make it less painful if we stored the entire type 
in the wi_range as mentioned earlier?

> 
> -/* Calculate TRUNC_MOD_EXPR on two ranges and store the result in
> -   [WMIN,WMAX].  */
> +/* Calculate TRUNC_MOD_EXPR on two ranges and store the result in RES.
> +   Return TRUE if operation succeeded.  */
> 
> did I say multiple changes...?  I think this isn't a good one given "succeed"
> doesn't imply that if not the result is UNDEFINED ...

Point taken, but hey I can't read your responses as you're typing them ;-).

> 
> As said above the shift handlign structrue is now unreadable :/  Given we do
> different things for lshift vs. rshift it makes sense to simply split

Unreadable?  Surely you jest!  As opposed to the super-readable stuff in 
VRP before?  Come on!  But I can split the shifts.  That's not a problem :).

> 
>     else if (code == RSHIFT_EXPR
>             || code == LSHIFT_EXPR)
>       {
> 
> even if that means duplicating a few lines.
> 
> Just to remind you review (and approval) isn't easier if you put unrelated stuff
> into a patch ;)

If you mention it three times in a review and I apologize three times, 
are we at peace now? :-P

Aldy
Aldy Hernandez Sept. 3, 2018, 6:58 a.m. UTC | #3 
-------- Forwarded Message --------
Subject: Re: lightweight class to wide int ranges in VRP and friends
Date: Fri, 24 Aug 2018 13:40:29 -0400
From: Aldy Hernandez <aldyh@redhat.com>
To: Richard Biener <richard.guenther@gmail.com>
CC: GCC Patches <gcc-patches@gcc.gnu.org>

On 08/24/2018 06:32 AM, Richard Biener wrote:
> On Wed, Aug 15, 2018 at 7:31 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>>
>> I kept seeing the same patterns over and over in all this re-factoring:
>>
>> 1. extract value_range constants into pairs of wide ints
>>
>> 2. mimic symbolics as [-MIN, +MAX] (most of the time :))
>>
>> 3. perform some wide int operation on the wide int range
>>
>> 4. convert back to a value range
>>
>> So I've decided to clean this up even more.  Instead of passing a pair
>> of wide ints plus sign, precision, and god knows what to every
>> wide_int_range_* function, I've implemented a lighweight class
>> (wi_range) for a pair of wide ints.  It is not meant for long term
>> storage, but even so its footprint is minimal.
>>
>> The only extra bits are another 64-bit word per pair for keeping
>> attributes such as precision, sign, overflow_wraps, and a few other
>> things we'll need shortly.  In reality we only need one set of
>> attributes for both sets of pairs, so we waste one 64-bit word.  I don't
>> care.  They're short lived, and the resulting code looks an awful lot
>> nicer.  For example, the shift code gets rewritten from this:
>>
>>         if (range_int_cst_p (&vr1)
>>            && !vrp_shift_undefined_p (vr1, prec))
>>          {
>>            if (code == RSHIFT_EXPR)
>>              {
>>                if (vr0.type != VR_RANGE || symbolic_range_p (&vr0))
>>                  {
>>                    vr0.type = type = VR_RANGE;
>>                    vr0.min = vrp_val_min (expr_type);
>>                    vr0.max = vrp_val_max (expr_type);
>>                  }
>>                extract_range_from_multiplicative_op (vr, code, &vr0, &vr1);
>>                return;
>>              }
>>            else if (code == LSHIFT_EXPR
>>                     && range_int_cst_p (&vr0))
>>              {
>>                wide_int res_lb, res_ub;
>>                if (wide_int_range_lshift (res_lb, res_ub, sign, prec,
>>                                           wi::to_wide (vr0.min),
>>                                           wi::to_wide (vr0.max),
>>                                           wi::to_wide (vr1.min),
>>                                           wi::to_wide (vr1.max),
>>                                           TYPE_OVERFLOW_UNDEFINED (expr_type),
>>                                           TYPE_OVERFLOW_WRAPS (expr_type)))
>>                  {
>>                    min = wide_int_to_tree (expr_type, res_lb);
>>                    max = wide_int_to_tree (expr_type, res_ub);
>>                    set_and_canonicalize_value_range (vr, VR_RANGE,
>>                                                      min, max, NULL);
>>                    return;
>>                  }
>>              }
>>          }
>>         set_value_range_to_varying (vr);
>>
>> into this:
>>
>>         wi_range w0 (&vr0, expr_type);
>>         wi_range w1 (&vr1, expr_type);
>>         if (!range_int_cst_p (&vr1)
>>            || wi_range_shift_undefined_p (w1)
>>            || (code == LSHIFT_EXPR
>>                && !range_int_cst_p (&vr0)))
>>          {
>>            vr->set_varying ();
>>            return;
>>          }
>>         bool success;
>>         if (code == RSHIFT_EXPR)
>>          success = wi_range_multiplicative_op (res, code, w0, w1);
>>         else
>>          success = wi_range_lshift (res, w0, w1);
>>
>>         if (success)
>>          vr->set_and_canonicalize (res, expr_type);
>>         else
>>          vr->set_varying ();
>>         return;
> 
> not sure whether I like the munging of lshift and right shift (what exactly gets
> done is less clear in your version ...).  Having a light-weigth class for
> the wi_range_ parameters is nice though.

No worries.  I'm not emotionally attached to munging them :).

> 
>> I also munged together the maybe/mustbe nonzero_bits into one structure.
>>
>> Finally, I've pontificated that wide_int_range_blah is too much typing.
>> Everyone's RSI will thank me for rewriting the methods as wi_range_blah.
>>
>> I've tried to keep the functionality changes to a minimum.  However,
>> there are some slight changes where I treat symbolics and VR_VARYING as
>> [MIN, MAX] and let the constant code do its thing.  It is considerably
>> easier to read.
>>
>> I am finally happy with the overall direction of this.  If this and the
>> last one are acceptable, I think I only need to clean MIN/MAX_EXPR and
>> ABS_EXPR and I'm basically done with what I need going forward.
>>
>> Phew...
>>
>> How does this look?
> 
> +struct wi_range
> +{
> +  wide_int min;
> +  wide_int max;
> +  /* This structure takes one additional 64-bit word apart from the
> +     min/max bounds above.  It is laid out so that PRECISION and SIGN
> +     can be accessed without any bit twiddling, since they're the most
> +     commonly accessed fields.  */
> +  unsigned short precision;
> +  bool empty_p:1;
> +  bool pointer_type_p:1;
> +  bool overflow_wraps:1;
> +  bool overflow_undefined:1;
> +  signop sign;
> 
> isn't precision already available in min.get_precision () which is
> required to be equal to max.get_precision ()?

Indeed.  Good point.

> 
> +  wi_range () { empty_p = false; }
> 
> true I suppose?  Better not allow default construction?

OK.

> 
> empty_p doesn't seem to be documented, nor is pointer_type_p
> or why that is necessary - maybe simply store a tree type
> instead all of the bits?

I was trying to avoid dereferencing another pointer by perhaps 
flattening out the needed bits, but again-- not emotionally attached. 
Easier for me...

> 
> +/* A structure to pass maybe and mustbe nonzero bitmasks.  This is for
> +   use in wi_range_set_zero_nonzero_bits and friends.  */
> +
> +struct maybe_nonzero_bits
> +{
> +  /* If some bit is unset, it means that for all numbers in the range
> +   the bit is 0, otherwise it might be 0 or 1.  */
> +  wide_int may_be;
> +
> +  /* If some bit is set, it means that for all numbers in the range
> +   the bit is 1, otherwise it might be 0 or 1.  */
> +  wide_int must_be;
>   };
> 
> eh - multiple changes in one patch ...

Well, is it?  I'm trying to avoid passing tons of things to the 
wide_int_range_* functions, and that includes wide ints and the nonzero 
bit pairs.  Could I please leave these in the patch?

> 
> @@ -52,6 +54,58 @@ struct GTY((for_user)) value_range
> 
>     /* Dump value range to stderr.  */
>     void dump ();
> +
> +  void set (const wi_range &r, tree type);
> +  void set_and_canonicalize (const wi_range &r, tree type);
> +  void set_undefined ();
> +  void set_varying ();
> +};
> 
> likewise ... you introduce those methods but not use them consistently.
> Please leave out this change (I would object to this kind of change
> if carried out through).

I was really avoiding having to convert every caller.  But really, why 
would you object even if carried out through?

> 
> +/* Construct a new range from the contents of a value_range.  Varying
> +   or symbolic ranges are normalized to the entire possible range.  */
> +
> +inline
> +wi_range::wi_range (value_range *vr, tree type)
> +{
> +  precision = TYPE_PRECISION (type);
> +  pointer_type_p = POINTER_TYPE_P (type);
> +  overflow_wraps = TYPE_OVERFLOW_WRAPS (type);
> +  overflow_undefined = TYPE_OVERFLOW_UNDEFINED (type);
> +  sign = TYPE_SIGN (type);
> +  empty_p = false;
> +  if (vr->type == VR_VARYING || symbolic_range_p (vr))
> +    {
> +      min = wi::min_value (precision, sign);
> +      max = wi::max_value (precision, sign);
> +    }
> +  else if (vr->type == VR_RANGE)
> +    {
> +      min = wi::to_wide (vr->min);
> +      max = wi::to_wide (vr->max);
> +    }
> +  else
> +    gcc_unreachable ();
> +}
> 
> What about VR_ANTI_RANGE?  Note the above doesn't exactly look
> "light-weight".

Yes, I should handle anti range, or ICE or something, considering the 
pain it was to find the anti range problem with the POINTER_PLUS_EXPR 
code recently.

Is "not exactly light-weight" an objection, or just a btw type of 
comment :)?  Would it make it less painful if we stored the entire type 
in the wi_range as mentioned earlier?

> 
> -/* Calculate TRUNC_MOD_EXPR on two ranges and store the result in
> -   [WMIN,WMAX].  */
> +/* Calculate TRUNC_MOD_EXPR on two ranges and store the result in RES.
> +   Return TRUE if operation succeeded.  */
> 
> did I say multiple changes...?  I think this isn't a good one given "succeed"
> doesn't imply that if not the result is UNDEFINED ...

Point taken, but hey I can't read your responses as you're typing them ;-).

> 
> As said above the shift handlign structrue is now unreadable :/  Given we do
> different things for lshift vs. rshift it makes sense to simply split

Unreadable?  Surely you jest!  As opposed to the super-readable stuff in 
VRP before?  Come on!  But I can split the shifts.  That's not a problem :).

> 
>     else if (code == RSHIFT_EXPR
>             || code == LSHIFT_EXPR)
>       {
> 
> even if that means duplicating a few lines.
> 
> Just to remind you review (and approval) isn't easier if you put unrelated stuff
> into a patch ;)

If you mention it three times in a review and I apologize three times, 
are we at peace now? :-P

Aldy
Richard Biener Sept. 14, 2018, 7:06 a.m. UTC | #4 
On Mon, Sep 3, 2018 at 8:58 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>
>

Can you re-base and re-post please?

Sorry for being late here.

Richard.

>
> -------- Forwarded Message --------
> Subject: Re: lightweight class to wide int ranges in VRP and friends
> Date: Fri, 24 Aug 2018 13:40:29 -0400
> From: Aldy Hernandez <aldyh@redhat.com>
> To: Richard Biener <richard.guenther@gmail.com>
> CC: GCC Patches <gcc-patches@gcc.gnu.org>
>
> On 08/24/2018 06:32 AM, Richard Biener wrote:
> > On Wed, Aug 15, 2018 at 7:31 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> >>
> >> I kept seeing the same patterns over and over in all this re-factoring:
> >>
> >> 1. extract value_range constants into pairs of wide ints
> >>
> >> 2. mimic symbolics as [-MIN, +MAX] (most of the time :))
> >>
> >> 3. perform some wide int operation on the wide int range
> >>
> >> 4. convert back to a value range
> >>
> >> So I've decided to clean this up even more.  Instead of passing a pair
> >> of wide ints plus sign, precision, and god knows what to every
> >> wide_int_range_* function, I've implemented a lighweight class
> >> (wi_range) for a pair of wide ints.  It is not meant for long term
> >> storage, but even so its footprint is minimal.
> >>
> >> The only extra bits are another 64-bit word per pair for keeping
> >> attributes such as precision, sign, overflow_wraps, and a few other
> >> things we'll need shortly.  In reality we only need one set of
> >> attributes for both sets of pairs, so we waste one 64-bit word.  I don't
> >> care.  They're short lived, and the resulting code looks an awful lot
> >> nicer.  For example, the shift code gets rewritten from this:
> >>
> >>         if (range_int_cst_p (&vr1)
> >>            && !vrp_shift_undefined_p (vr1, prec))
> >>          {
> >>            if (code == RSHIFT_EXPR)
> >>              {
> >>                if (vr0.type != VR_RANGE || symbolic_range_p (&vr0))
> >>                  {
> >>                    vr0.type = type = VR_RANGE;
> >>                    vr0.min = vrp_val_min (expr_type);
> >>                    vr0.max = vrp_val_max (expr_type);
> >>                  }
> >>                extract_range_from_multiplicative_op (vr, code, &vr0, &vr1);
> >>                return;
> >>              }
> >>            else if (code == LSHIFT_EXPR
> >>                     && range_int_cst_p (&vr0))
> >>              {
> >>                wide_int res_lb, res_ub;
> >>                if (wide_int_range_lshift (res_lb, res_ub, sign, prec,
> >>                                           wi::to_wide (vr0.min),
> >>                                           wi::to_wide (vr0.max),
> >>                                           wi::to_wide (vr1.min),
> >>                                           wi::to_wide (vr1.max),
> >>                                           TYPE_OVERFLOW_UNDEFINED (expr_type),
> >>                                           TYPE_OVERFLOW_WRAPS (expr_type)))
> >>                  {
> >>                    min = wide_int_to_tree (expr_type, res_lb);
> >>                    max = wide_int_to_tree (expr_type, res_ub);
> >>                    set_and_canonicalize_value_range (vr, VR_RANGE,
> >>                                                      min, max, NULL);
> >>                    return;
> >>                  }
> >>              }
> >>          }
> >>         set_value_range_to_varying (vr);
> >>
> >> into this:
> >>
> >>         wi_range w0 (&vr0, expr_type);
> >>         wi_range w1 (&vr1, expr_type);
> >>         if (!range_int_cst_p (&vr1)
> >>            || wi_range_shift_undefined_p (w1)
> >>            || (code == LSHIFT_EXPR
> >>                && !range_int_cst_p (&vr0)))
> >>          {
> >>            vr->set_varying ();
> >>            return;
> >>          }
> >>         bool success;
> >>         if (code == RSHIFT_EXPR)
> >>          success = wi_range_multiplicative_op (res, code, w0, w1);
> >>         else
> >>          success = wi_range_lshift (res, w0, w1);
> >>
> >>         if (success)
> >>          vr->set_and_canonicalize (res, expr_type);
> >>         else
> >>          vr->set_varying ();
> >>         return;
> >
> > not sure whether I like the munging of lshift and right shift (what exactly gets
> > done is less clear in your version ...).  Having a light-weigth class for
> > the wi_range_ parameters is nice though.
>
> No worries.  I'm not emotionally attached to munging them :).
>
> >
> >> I also munged together the maybe/mustbe nonzero_bits into one structure.
> >>
> >> Finally, I've pontificated that wide_int_range_blah is too much typing.
> >> Everyone's RSI will thank me for rewriting the methods as wi_range_blah.
> >>
> >> I've tried to keep the functionality changes to a minimum.  However,
> >> there are some slight changes where I treat symbolics and VR_VARYING as
> >> [MIN, MAX] and let the constant code do its thing.  It is considerably
> >> easier to read.
> >>
> >> I am finally happy with the overall direction of this.  If this and the
> >> last one are acceptable, I think I only need to clean MIN/MAX_EXPR and
> >> ABS_EXPR and I'm basically done with what I need going forward.
> >>
> >> Phew...
> >>
> >> How does this look?
> >
> > +struct wi_range
> > +{
> > +  wide_int min;
> > +  wide_int max;
> > +  /* This structure takes one additional 64-bit word apart from the
> > +     min/max bounds above.  It is laid out so that PRECISION and SIGN
> > +     can be accessed without any bit twiddling, since they're the most
> > +     commonly accessed fields.  */
> > +  unsigned short precision;
> > +  bool empty_p:1;
> > +  bool pointer_type_p:1;
> > +  bool overflow_wraps:1;
> > +  bool overflow_undefined:1;
> > +  signop sign;
> >
> > isn't precision already available in min.get_precision () which is
> > required to be equal to max.get_precision ()?
>
> Indeed.  Good point.
>
> >
> > +  wi_range () { empty_p = false; }
> >
> > true I suppose?  Better not allow default construction?
>
> OK.
>
> >
> > empty_p doesn't seem to be documented, nor is pointer_type_p
> > or why that is necessary - maybe simply store a tree type
> > instead all of the bits?
>
> I was trying to avoid dereferencing another pointer by perhaps
> flattening out the needed bits, but again-- not emotionally attached.
> Easier for me...
>
> >
> > +/* A structure to pass maybe and mustbe nonzero bitmasks.  This is for
> > +   use in wi_range_set_zero_nonzero_bits and friends.  */
> > +
> > +struct maybe_nonzero_bits
> > +{
> > +  /* If some bit is unset, it means that for all numbers in the range
> > +   the bit is 0, otherwise it might be 0 or 1.  */
> > +  wide_int may_be;
> > +
> > +  /* If some bit is set, it means that for all numbers in the range
> > +   the bit is 1, otherwise it might be 0 or 1.  */
> > +  wide_int must_be;
> >   };
> >
> > eh - multiple changes in one patch ...
>
> Well, is it?  I'm trying to avoid passing tons of things to the
> wide_int_range_* functions, and that includes wide ints and the nonzero
> bit pairs.  Could I please leave these in the patch?
>
> >
> > @@ -52,6 +54,58 @@ struct GTY((for_user)) value_range
> >
> >     /* Dump value range to stderr.  */
> >     void dump ();
> > +
> > +  void set (const wi_range &r, tree type);
> > +  void set_and_canonicalize (const wi_range &r, tree type);
> > +  void set_undefined ();
> > +  void set_varying ();
> > +};
> >
> > likewise ... you introduce those methods but not use them consistently.
> > Please leave out this change (I would object to this kind of change
> > if carried out through).
>
> I was really avoiding having to convert every caller.  But really, why
> would you object even if carried out through?
>
> >
> > +/* Construct a new range from the contents of a value_range.  Varying
> > +   or symbolic ranges are normalized to the entire possible range.  */
> > +
> > +inline
> > +wi_range::wi_range (value_range *vr, tree type)
> > +{
> > +  precision = TYPE_PRECISION (type);
> > +  pointer_type_p = POINTER_TYPE_P (type);
> > +  overflow_wraps = TYPE_OVERFLOW_WRAPS (type);
> > +  overflow_undefined = TYPE_OVERFLOW_UNDEFINED (type);
> > +  sign = TYPE_SIGN (type);
> > +  empty_p = false;
> > +  if (vr->type == VR_VARYING || symbolic_range_p (vr))
> > +    {
> > +      min = wi::min_value (precision, sign);
> > +      max = wi::max_value (precision, sign);
> > +    }
> > +  else if (vr->type == VR_RANGE)
> > +    {
> > +      min = wi::to_wide (vr->min);
> > +      max = wi::to_wide (vr->max);
> > +    }
> > +  else
> > +    gcc_unreachable ();
> > +}
> >
> > What about VR_ANTI_RANGE?  Note the above doesn't exactly look
> > "light-weight".
>
> Yes, I should handle anti range, or ICE or something, considering the
> pain it was to find the anti range problem with the POINTER_PLUS_EXPR
> code recently.
>
> Is "not exactly light-weight" an objection, or just a btw type of
> comment :)?  Would it make it less painful if we stored the entire type
> in the wi_range as mentioned earlier?
>
> >
> > -/* Calculate TRUNC_MOD_EXPR on two ranges and store the result in
> > -   [WMIN,WMAX].  */
> > +/* Calculate TRUNC_MOD_EXPR on two ranges and store the result in RES.
> > +   Return TRUE if operation succeeded.  */
> >
> > did I say multiple changes...?  I think this isn't a good one given "succeed"
> > doesn't imply that if not the result is UNDEFINED ...
>
> Point taken, but hey I can't read your responses as you're typing them ;-).
>
> >
> > As said above the shift handlign structrue is now unreadable :/  Given we do
> > different things for lshift vs. rshift it makes sense to simply split
>
> Unreadable?  Surely you jest!  As opposed to the super-readable stuff in
> VRP before?  Come on!  But I can split the shifts.  That's not a problem :).
>
> >
> >     else if (code == RSHIFT_EXPR
> >             || code == LSHIFT_EXPR)
> >       {
> >
> > even if that means duplicating a few lines.
> >
> > Just to remind you review (and approval) isn't easier if you put unrelated stuff
> > into a patch ;)
>
> If you mention it three times in a review and I apologize three times,
> are we at peace now? :-P
>
> Aldy
diff mbox series

Patch

gcc/

	* tree-vrp.c (set_value_range_to_undefined): Remove static inline.
	(vrp_set_zero_nonzero_bits): Pass nonzero bits as a structure.
	(vrp_shift_undefined_p): Remove.
	(extract_range_from_multiplicative_op): Use wi_range class.
	(vrp_can_optimize_bit_op): Rename
	wide_int_range_can_optimize_bit_op to
	wi_range_can_optimize_bit_op.
	(extract_range_from_binary_expr_1): Use wi_range class throughout.
	Rename uses of wide_int_range_* to wi_range_*.
	(wi_range::dump): New.
	* tree-vrp.h (struct value_range): Add set, set_and_canonicalize,
	set_undefined, and set_varying.
	(struct wi_range): New.
	(struct maybe_nonzero_bits): New.
	* vr-values.c (simplify_bit_ops_using_ranges): Pass nonzero bits
	to vrp_set_zero_nonzero_bits in a structure.
	* wide-int-range.cc (wi_range_set_zero_nonzero_bits): Rename from
	wide_int_range_*.  Pass nonzero bits as a structure.  Use wi_range
	for arguments and adjust.
	(wi_range_min_max): Same.
	(wi_range_cross_product): Same.
	(wi_range_mult_wrapping): Same.
	(wi_range_multiplicative_op): Same.
	(wi_range_lshift): Same.
	(wi_range_can_optimize_bit_op): Same.
	(wi_range_bit_xor): Same.
	(wi_range_bit_ior): Same.
	(wi_range_bit_and): Same.
	(wi_range_trunc_mod): Same.
	* wide-int-range.h: Rename all wide_int_range_* to wi_range_*.
	(wi_range_shift_undefined_p): Adjust for wi_range.

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index d553a254878..7fe68177704 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -246,7 +246,7 @@  intersect_range_with_nonzero_bits (enum value_range_type vr_type,
 
 /* Set value range VR to VR_UNDEFINED.  */
 
-static inline void
+void
 set_value_range_to_undefined (value_range *vr)
 {
   vr->type = VR_UNDEFINED;
@@ -957,9 +957,9 @@  value_range_constant_singleton (value_range *vr)
   return NULL_TREE;
 }
 
-/* Value range wrapper for wide_int_range_set_zero_nonzero_bits.
+/* Value range wrapper for wi_range_set_zero_nonzero_bits.
 
-   Compute MAY_BE_NONZERO and MUST_BE_NONZERO bit masks for range in VR.
+   Compute maybe_nonzero_bits for range in VR.
 
    Return TRUE if VR was a constant range and we were able to compute
    the bit masks.  */
@@ -967,19 +967,16 @@  value_range_constant_singleton (value_range *vr)
 bool
 vrp_set_zero_nonzero_bits (const tree expr_type,
 			   value_range *vr,
-			   wide_int *may_be_nonzero,
-			   wide_int *must_be_nonzero)
+			   maybe_nonzero_bits *nonzero)
 {
   if (!range_int_cst_p (vr))
     {
-      *may_be_nonzero = wi::minus_one (TYPE_PRECISION (expr_type));
-      *must_be_nonzero = wi::zero (TYPE_PRECISION (expr_type));
+      nonzero->may_be = wi::minus_one (TYPE_PRECISION (expr_type));
+      nonzero->must_be = wi::zero (TYPE_PRECISION (expr_type));
       return false;
     }
-  wide_int_range_set_zero_nonzero_bits (TYPE_SIGN (expr_type),
-					wi::to_wide (vr->min),
-					wi::to_wide (vr->max),
-					*may_be_nonzero, *must_be_nonzero);
+  wi_range r (vr, expr_type);
+  wi_range_set_zero_nonzero_bits (r, nonzero);
   return true;
 }
 
@@ -1048,17 +1045,6 @@  extract_range_into_wide_ints (value_range *vr,
     }
 }
 
-/* Value range wrapper for wide_int_range_shift_undefined_p.  */
-
-static inline bool
-vrp_shift_undefined_p (const value_range &shifter, unsigned prec)
-{
-  tree type = TREE_TYPE (shifter.min);
-  return wide_int_range_shift_undefined_p (TYPE_SIGN (type), prec,
-					   wi::to_wide (shifter.min),
-					   wi::to_wide (shifter.max));
-}
-
 /* Value range wrapper for wide_int_range_multiplicative_op:
 
      *VR = *VR0 .CODE. *VR1.  */
@@ -1079,24 +1065,13 @@  extract_range_from_multiplicative_op (value_range *vr,
   gcc_assert (vr0->type == VR_RANGE && vr0->type == vr1->type);
 
   tree type = TREE_TYPE (vr0->min);
-  wide_int res_lb, res_ub;
-  wide_int vr0_lb = wi::to_wide (vr0->min);
-  wide_int vr0_ub = wi::to_wide (vr0->max);
-  wide_int vr1_lb = wi::to_wide (vr1->min);
-  wide_int vr1_ub = wi::to_wide (vr1->max);
-  bool overflow_undefined = TYPE_OVERFLOW_UNDEFINED (type);
-  bool overflow_wraps = TYPE_OVERFLOW_WRAPS (type);
-  unsigned prec = TYPE_PRECISION (type);
-
-  if (wide_int_range_multiplicative_op (res_lb, res_ub,
-					 code, TYPE_SIGN (type), prec,
-					 vr0_lb, vr0_ub, vr1_lb, vr1_ub,
-					 overflow_undefined, overflow_wraps))
-    set_and_canonicalize_value_range (vr, VR_RANGE,
-				      wide_int_to_tree (type, res_lb),
-				      wide_int_to_tree (type, res_ub), NULL);
+  wi_range res;
+  wi_range w0 (vr0, type);
+  wi_range w1 (vr1, type);
+  if (wi_range_multiplicative_op (res, code, w0, w1))
+    vr->set_and_canonicalize (res, type);
   else
-    set_value_range_to_varying (vr);
+    vr->set_varying ();
 }
 
 /* Value range wrapper for wide_int_range_can_optimize_bit_op.
@@ -1129,10 +1104,10 @@  vrp_can_optimize_bit_op (value_range *vr, enum tree_code code,
     }
   else
     return false;
-  if (wide_int_range_can_optimize_bit_op (code,
-					  wi::to_wide (lower_bound),
-					  wi::to_wide (upper_bound),
-					  wi::to_wide (mask)))
+  if (wi_range_can_optimize_bit_op (code,
+				    wi::to_wide (lower_bound),
+				    wi::to_wide (upper_bound),
+				    wi::to_wide (mask)))
     {
       tree min = int_const_binop (code, lower_bound, mask);
       tree max = int_const_binop (code, upper_bound, mask);
@@ -1317,13 +1292,12 @@  extract_range_from_binary_expr_1 (value_range *vr,
 				  enum tree_code code, tree expr_type,
 				  value_range *vr0_, value_range *vr1_)
 {
-  signop sign = TYPE_SIGN (expr_type);
-  unsigned int prec = TYPE_PRECISION (expr_type);
   value_range vr0 = *vr0_, vr1 = *vr1_;
   value_range vrtem0 = VR_INITIALIZER, vrtem1 = VR_INITIALIZER;
   enum value_range_type type;
   tree min = NULL_TREE, max = NULL_TREE;
   int cmp;
+  wi_range res;
 
   if (!INTEGRAL_TYPE_P (expr_type)
       && !POINTER_TYPE_P (expr_type))
@@ -1636,56 +1610,49 @@  extract_range_from_binary_expr_1 (value_range *vr,
     }
   else if (code == MULT_EXPR)
     {
-      if (!range_int_cst_p (&vr0)
-	  || !range_int_cst_p (&vr1))
+      if (!range_int_cst_p (&vr0) || !range_int_cst_p (&vr1))
 	{
-	  set_value_range_to_varying (vr);
+	  vr->set_varying ();
 	  return;
 	}
-      extract_range_from_multiplicative_op (vr, code, &vr0, &vr1);
+      wi_range w0 (&vr0, expr_type);
+      wi_range w1 (&vr1, expr_type);
+      if (wi_range_multiplicative_op (res, code, w0, w1))
+	vr->set_and_canonicalize (res, expr_type);
+      else
+	vr->set_varying ();
       return;
     }
   else if (code == RSHIFT_EXPR
 	   || code == LSHIFT_EXPR)
     {
-      if (range_int_cst_p (&vr1)
-	  && !vrp_shift_undefined_p (vr1, prec))
+      /* Avoid calculating the obviously impossible stuff.
+
+	 However, we allow right shifting an unknown vr0 if the shift
+	 count is known, because we may still calculate a useful
+	 range.  For example:
+
+	 x >> 63 for signed 64-bit x is always [-1, 0].  */
+      wi_range w0 (&vr0, expr_type);
+      wi_range w1 (&vr1, expr_type);
+      if (!range_int_cst_p (&vr1)
+	  || wi_range_shift_undefined_p (w1)
+	  || (code == LSHIFT_EXPR
+	      && !range_int_cst_p (&vr0)))
 	{
-	  if (code == RSHIFT_EXPR)
-	    {
-	      /* Even if vr0 is VARYING or otherwise not usable, we can derive
-		 useful ranges just from the shift count.  E.g.
-		 x >> 63 for signed 64-bit x is always [-1, 0].  */
-	      if (vr0.type != VR_RANGE || symbolic_range_p (&vr0))
-		{
-		  vr0.type = type = VR_RANGE;
-		  vr0.min = vrp_val_min (expr_type);
-		  vr0.max = vrp_val_max (expr_type);
-		}
-	      extract_range_from_multiplicative_op (vr, code, &vr0, &vr1);
-	      return;
-	    }
-	  else if (code == LSHIFT_EXPR
-		   && range_int_cst_p (&vr0))
-	    {
-	      wide_int res_lb, res_ub;
-	      if (wide_int_range_lshift (res_lb, res_ub, sign, prec,
-					 wi::to_wide (vr0.min),
-					 wi::to_wide (vr0.max),
-					 wi::to_wide (vr1.min),
-					 wi::to_wide (vr1.max),
-					 TYPE_OVERFLOW_UNDEFINED (expr_type),
-					 TYPE_OVERFLOW_WRAPS (expr_type)))
-		{
-		  min = wide_int_to_tree (expr_type, res_lb);
-		  max = wide_int_to_tree (expr_type, res_ub);
-		  set_and_canonicalize_value_range (vr, VR_RANGE,
-						    min, max, NULL);
-		  return;
-		}
-	    }
+	  vr->set_varying ();
+	  return;
 	}
-      set_value_range_to_varying (vr);
+      bool success;
+      if (code == RSHIFT_EXPR)
+	success = wi_range_multiplicative_op (res, code, w0, w1);
+      else
+	success = wi_range_lshift (res, w0, w1);
+
+      if (success)
+	vr->set_and_canonicalize (res, expr_type);
+      else
+	vr->set_varying ();
       return;
     }
   else if (code == TRUNC_DIV_EXPR
@@ -1800,20 +1767,12 @@  extract_range_from_binary_expr_1 (value_range *vr,
     }
   else if (code == TRUNC_MOD_EXPR)
     {
-      if (range_is_null (&vr1))
-	{
-	  set_value_range_to_undefined (vr);
-	  return;
-	}
-      wide_int wmin, wmax, tmp;
-      wide_int vr0_min, vr0_max, vr1_min, vr1_max;
-      extract_range_into_wide_ints (&vr0, sign, prec, &vr0_min, &vr0_max);
-      extract_range_into_wide_ints (&vr1, sign, prec, &vr1_min, &vr1_max);
-      wide_int_range_trunc_mod (wmin, wmax, sign, prec,
-				vr0_min, vr0_max, vr1_min, vr1_max);
-      min = wide_int_to_tree (expr_type, wmin);
-      max = wide_int_to_tree (expr_type, wmax);
-      set_value_range (vr, VR_RANGE, min, max, NULL);
+      wi_range w0 (&vr0, expr_type);
+      wi_range w1 (&vr1, expr_type);
+      if (wi_range_trunc_mod (res, w0, w1))
+	vr->set (res, expr_type);
+      else
+	vr->set_undefined ();
       return;
     }
   else if (code == BIT_AND_EXPR || code == BIT_IOR_EXPR || code == BIT_XOR_EXPR)
@@ -1821,68 +1780,30 @@  extract_range_from_binary_expr_1 (value_range *vr,
       if (vrp_can_optimize_bit_op (vr, code, &vr0, &vr1))
 	return;
 
-      wide_int may_be_nonzero0, may_be_nonzero1;
-      wide_int must_be_nonzero0, must_be_nonzero1;
-      wide_int wmin, wmax;
-      wide_int vr0_min, vr0_max, vr1_min, vr1_max;
-      vrp_set_zero_nonzero_bits (expr_type, &vr0,
-				 &may_be_nonzero0, &must_be_nonzero0);
-      vrp_set_zero_nonzero_bits (expr_type, &vr1,
-				 &may_be_nonzero1, &must_be_nonzero1);
-      extract_range_into_wide_ints (&vr0, sign, prec, &vr0_min, &vr0_max);
-      extract_range_into_wide_ints (&vr1, sign, prec, &vr1_min, &vr1_max);
+      wi_range w0 (&vr0, expr_type);
+      wi_range w1 (&vr1, expr_type);
+      maybe_nonzero_bits nonzero0, nonzero1;
+      /* ?? Previously we were calling vrp_set_zero_nonzero_bits,
+	 which would immediately set the masks if the range was not a
+	 constant.  Let's avoid this special casing for maximum
+	 share-ability.  */
+      wi_range_set_zero_nonzero_bits (w0, &nonzero0);
+      wi_range_set_zero_nonzero_bits (w1, &nonzero1);
+      bool success;
       if (code == BIT_AND_EXPR)
-	{
-	  if (wide_int_range_bit_and (wmin, wmax, sign, prec,
-				      vr0_min, vr0_max,
-				      vr1_min, vr1_max,
-				      must_be_nonzero0,
-				      may_be_nonzero0,
-				      must_be_nonzero1,
-				      may_be_nonzero1))
-	    {
-	      min = wide_int_to_tree (expr_type, wmin);
-	      max = wide_int_to_tree (expr_type, wmax);
-	      set_value_range (vr, VR_RANGE, min, max, NULL);
-	    }
-	  else
-	    set_value_range_to_varying (vr);
-	  return;
-	}
+	success = wi_range_bit_and (res, w0, w1, nonzero0, nonzero1);
       else if (code == BIT_IOR_EXPR)
-	{
-	  if (wide_int_range_bit_ior (wmin, wmax, sign,
-				      vr0_min, vr0_max,
-				      vr1_min, vr1_max,
-				      must_be_nonzero0,
-				      may_be_nonzero0,
-				      must_be_nonzero1,
-				      may_be_nonzero1))
-	    {
-	      min = wide_int_to_tree (expr_type, wmin);
-	      max = wide_int_to_tree (expr_type, wmax);
-	      set_value_range (vr, VR_RANGE, min, max, NULL);
-	    }
-	  else
-	    set_value_range_to_varying (vr);
-	  return;
-	}
+	success = wi_range_bit_ior (res, w0, w1, nonzero0, nonzero1);
       else if (code == BIT_XOR_EXPR)
-	{
-	  if (wide_int_range_bit_xor (wmin, wmax, sign, prec,
-				      must_be_nonzero0,
-				      may_be_nonzero0,
-				      must_be_nonzero1,
-				      may_be_nonzero1))
-	    {
-	      min = wide_int_to_tree (expr_type, wmin);
-	      max = wide_int_to_tree (expr_type, wmax);
-	      set_value_range (vr, VR_RANGE, min, max, NULL);
-	    }
-	  else
-	    set_value_range_to_varying (vr);
-	  return;
-	}
+	success = wi_range_bit_xor (res, w0, w1, nonzero0, nonzero1);
+      else
+	gcc_unreachable ();
+
+      if (success)
+	vr->set (res, expr_type);
+      else
+	vr->set_varying ();
+      return;
     }
   else
     gcc_unreachable ();
@@ -2215,6 +2136,14 @@  value_range::dump ()
   debug_value_range (this);
 }
 
+void
+wi_range::dump ()
+{
+  fprintf (stderr, "[");
+  min.dump ();
+  max.dump ();
+  fprintf (stderr, "], precision = %d\n", precision);
+}
 
 /* Given a COND_EXPR COND of the form 'V OP W', and an SSA name V,
    create a new SSA name N and return the assertion assignment
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index 0c1fb3637cf..ca873be6843 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -25,6 +25,8 @@  along with GCC; see the file COPYING3.  If not see
 enum value_range_type { VR_UNDEFINED, VR_RANGE,
 			VR_ANTI_RANGE, VR_VARYING, VR_LAST };
 
+struct wi_range;
+
 /* Range of values that can be associated with an SSA_NAME after VRP
    has executed.  */
 struct GTY((for_user)) value_range
@@ -52,6 +54,58 @@  struct GTY((for_user)) value_range
 
   /* Dump value range to stderr.  */
   void dump ();
+
+  void set (const wi_range &r, tree type);
+  void set_and_canonicalize (const wi_range &r, tree type);
+  void set_undefined ();
+  void set_varying ();
+};
+
+/* Similar to value_range, but only for straight up VR_RANGEs held as
+   a pair of wide ints.  This is used internally to pass wide int
+   ranges to wi_range_* supporting routines, and is not meant for long
+   term storage.  */
+
+struct wi_range
+{
+  wide_int min;
+  wide_int max;
+  /* This structure takes one additional 64-bit word apart from the
+     min/max bounds above.  It is laid out so that PRECISION and SIGN
+     can be accessed without any bit twiddling, since they're the most
+     commonly accessed fields.  */
+  unsigned short precision;
+  bool empty_p:1;
+  bool pointer_type_p:1;
+  bool overflow_wraps:1;
+  bool overflow_undefined:1;
+  signop sign;
+
+  wi_range () { empty_p = false; }
+  /* Construct a new range from the contents of a value_range.  */
+  wi_range (value_range *vr, tree type);
+  /* Initialize a range with [MIN_, MAX_], but using the attributes
+     (precision, sign, etc) from SRC.  */
+  void set (const wi_range &src, const wide_int &min_, const wide_int &max_);
+  /* Return TRUE if range is a singleton zero.  */
+  bool zero_p () const { return (wi::eq_p (min, max)
+				 && wi::eq_p (min, wi::zero (precision))); }
+  /* We all need a little help sometimes.  */
+  void dump ();
+};
+
+/* A structure to pass maybe and mustbe nonzero bitmasks.  This is for
+   use in wi_range_set_zero_nonzero_bits and friends.  */
+
+struct maybe_nonzero_bits
+{
+  /* If some bit is unset, it means that for all numbers in the range
+   the bit is 0, otherwise it might be 0 or 1.  */
+  wide_int may_be;
+
+  /* If some bit is set, it means that for all numbers in the range
+   the bit is 1, otherwise it might be 0 or 1.  */
+  wide_int must_be;
 };
 
 extern void vrp_intersect_ranges (value_range *vr0, value_range *vr1);
@@ -86,6 +140,7 @@  extern void register_edge_assert_for (tree, edge, enum tree_code,
 				      tree, tree, vec<assert_info> &);
 extern bool stmt_interesting_for_vrp (gimple *);
 extern void set_value_range_to_varying (value_range *);
+extern void set_value_range_to_undefined (value_range *);
 extern int range_includes_zero_p (tree, tree);
 extern bool infer_value_range (gimple *, tree, tree_code *, tree *);
 
@@ -116,7 +171,7 @@  extern int operand_less_p (tree, tree);
 extern bool find_case_label_range (gswitch *, tree, tree, size_t *, size_t *);
 extern bool find_case_label_index (gswitch *, size_t, tree, size_t *);
 extern bool vrp_set_zero_nonzero_bits (const tree, value_range *,
-				       wide_int *, wide_int *);
+				       maybe_nonzero_bits *);
 extern bool overflow_comparison_p (tree_code, tree, tree, bool, tree *);
 extern bool range_int_cst_singleton_p (value_range *);
 extern int value_inside_range (tree, tree, tree);
@@ -132,4 +187,82 @@  struct switch_update {
 extern vec<edge> to_remove_edges;
 extern vec<switch_update> to_update_switch_stmts;
 
+/* Construct a new range from the contents of a value_range.  Varying
+   or symbolic ranges are normalized to the entire possible range.  */
+
+inline
+wi_range::wi_range (value_range *vr, tree type)
+{
+  precision = TYPE_PRECISION (type);
+  pointer_type_p = POINTER_TYPE_P (type);
+  overflow_wraps = TYPE_OVERFLOW_WRAPS (type);
+  overflow_undefined = TYPE_OVERFLOW_UNDEFINED (type);
+  sign = TYPE_SIGN (type);
+  empty_p = false;
+  if (vr->type == VR_VARYING || symbolic_range_p (vr))
+    {
+      min = wi::min_value (precision, sign);
+      max = wi::max_value (precision, sign);
+    }
+  else if (vr->type == VR_RANGE)
+    {
+      min = wi::to_wide (vr->min);
+      max = wi::to_wide (vr->max);
+    }
+  else
+    gcc_unreachable ();
+}
+
+/* Set current wi_range to [MIN_, MAX_] but with the attributes from
+   SRC.  */
+
+inline void
+wi_range::set (const wi_range &src, const wide_int &min_, const wide_int &max_)
+{
+  min = min_;
+  max = max_;
+  precision = src.precision;
+  empty_p = src.empty_p;
+  pointer_type_p = src.pointer_type_p;
+  overflow_wraps = src.overflow_wraps;
+  overflow_undefined = src.overflow_undefined;
+  sign = src.sign;
+}
+
+/* Set current value_range to the range in R.  */
+
+inline void
+value_range::set (const wi_range &r, tree type)
+{
+  set_value_range (this, VR_RANGE,
+		   wide_int_to_tree (type, r.min),
+		   wide_int_to_tree (type, r.max), NULL);
+}
+
+/* Set and canonicalize current value_range to the range in R.  */
+
+inline void
+value_range::set_and_canonicalize (const wi_range &r, tree type)
+{
+  set_and_canonicalize_value_range (this, VR_RANGE,
+				    wide_int_to_tree (type, r.min),
+				    wide_int_to_tree (type, r.max), NULL);
+}
+
+/* Set current value range to VR_UNDEFINED.  */
+
+inline void
+value_range::set_undefined ()
+{
+  set_value_range_to_undefined (this);
+}
+
+/* Set current value range to VR_VARYING.  */
+
+inline void
+value_range::set_varying ()
+{
+  set_value_range_to_varying (this);
+}
+
 #endif /* GCC_TREE_VRP_H */
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 33335f3da31..f8b849a2b94 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -3300,23 +3300,22 @@  vr_values::simplify_bit_ops_using_ranges (gimple_stmt_iterator *gsi,
   else
     return false;
 
-  if (!vrp_set_zero_nonzero_bits (TREE_TYPE (op0), &vr0, &may_be_nonzero0,
-				  &must_be_nonzero0))
+  maybe_nonzero_bits nz0, nz1;
+  if (!vrp_set_zero_nonzero_bits (TREE_TYPE (op0), &vr0, &nz0))
     return false;
-  if (!vrp_set_zero_nonzero_bits (TREE_TYPE (op1), &vr1, &may_be_nonzero1,
-				  &must_be_nonzero1))
+  if (!vrp_set_zero_nonzero_bits (TREE_TYPE (op1), &vr1, &nz1))
     return false;
 
   switch (gimple_assign_rhs_code (stmt))
     {
     case BIT_AND_EXPR:
-      mask = wi::bit_and_not (may_be_nonzero0, must_be_nonzero1);
+      mask = wi::bit_and_not (nz0.may_be, nz1.must_be);
       if (mask == 0)
 	{
 	  op = op0;
 	  break;
 	}
-      mask = wi::bit_and_not (may_be_nonzero1, must_be_nonzero0);
+      mask = wi::bit_and_not (nz1.may_be, nz0.must_be);
       if (mask == 0)
 	{
 	  op = op1;
@@ -3324,13 +3323,13 @@  vr_values::simplify_bit_ops_using_ranges (gimple_stmt_iterator *gsi,
 	}
       break;
     case BIT_IOR_EXPR:
-      mask = wi::bit_and_not (may_be_nonzero0, must_be_nonzero1);
+      mask = wi::bit_and_not (nz0.may_be, nz1.must_be);
       if (mask == 0)
 	{
 	  op = op1;
 	  break;
 	}
-      mask = wi::bit_and_not (may_be_nonzero1, must_be_nonzero0);
+      mask = wi::bit_and_not (nz1.may_be, nz0.must_be);
       if (mask == 0)
 	{
 	  op = op0;
diff --git a/gcc/wide-int-range.cc b/gcc/wide-int-range.cc
index 3491d89664d..401bee4c946 100644
--- a/gcc/wide-int-range.cc
+++ b/gcc/wide-int-range.cc
@@ -22,6 +22,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "coretypes.h"
 #include "tree.h"
 #include "fold-const.h"
+#include "options.h"
+#include "tree-vrp.h"
 #include "wide-int-range.h"
 
 /* Wrapper around wide_int_binop that adjusts for overflow.
@@ -76,53 +78,46 @@  wide_int_binop_overflow (wide_int &res,
   return !overflow;
 }
 
-/* For range [LB, UB] compute two wide_int bit masks.
-
-   In the MAY_BE_NONZERO bit mask, if some bit is unset, it means that
-   for all numbers in the range the bit is 0, otherwise it might be 0
-   or 1.
-
-   In the MUST_BE_NONZERO bit mask, if some bit is set, it means that
-   for all numbers in the range the bit is 1, otherwise it might be 0
-   or 1.  */
+/* For range R compute the maybe and mustbe nonzero bits as described
+   in the MAYBE_NONZERO_BITS structure.  */
 
 void
-wide_int_range_set_zero_nonzero_bits (signop sign,
-				      const wide_int &lb, const wide_int &ub,
-				      wide_int &may_be_nonzero,
-				      wide_int &must_be_nonzero)
+wi_range_set_zero_nonzero_bits (const wi_range &r,
+				maybe_nonzero_bits *nonzero)
 {
-  may_be_nonzero = wi::minus_one (lb.get_precision ());
-  must_be_nonzero = wi::zero (lb.get_precision ());
+  signop sign = r.sign;
+  nonzero->may_be = wi::minus_one (r.min.get_precision ());
+  nonzero->must_be = wi::zero (r.min.get_precision ());
 
-  if (wi::eq_p (lb, ub))
+  if (wi::eq_p (r.min, r.max))
     {
-      may_be_nonzero = lb;
-      must_be_nonzero = may_be_nonzero;
+      nonzero->may_be = r.min;
+      nonzero->must_be = r.min;
     }
-  else if (wi::ge_p (lb, 0, sign) || wi::lt_p (ub, 0, sign))
+  else if (wi::ge_p (r.min, 0, sign) || wi::lt_p (r.max, 0, sign))
     {
-      wide_int xor_mask = lb ^ ub;
-      may_be_nonzero = lb | ub;
-      must_be_nonzero = lb & ub;
+      wide_int xor_mask = r.min ^ r.max;
+      nonzero->may_be = r.min | r.max;
+      nonzero->must_be = r.min & r.max;
       if (xor_mask != 0)
 	{
 	  wide_int mask = wi::mask (wi::floor_log2 (xor_mask), false,
-				    may_be_nonzero.get_precision ());
-	  may_be_nonzero = may_be_nonzero | mask;
-	  must_be_nonzero = wi::bit_and_not (must_be_nonzero, mask);
+				    nonzero->may_be.get_precision ());
+	  nonzero->may_be = nonzero->may_be | mask;
+	  nonzero->must_be = wi::bit_and_not (nonzero->must_be, mask);
 	}
     }
 }
 
-/* Order 2 sets of wide int ranges (w0/w1, w2/w3) and set MIN/MAX
-   accordingly.  */
+/* Order 2 sets of wide int ranges (w0/w1, w2/w3) and set the
+   resulting [MIN, MAX] in RES.  */
 
 static void
-wide_int_range_min_max (wide_int &min, wide_int &max,
-			wide_int &w0, wide_int &w1, wide_int &w2, wide_int &w3,
-			signop sign)
+wi_range_min_max (wi_range &res,
+		  wide_int &w0, wide_int &w1, wide_int &w2, wide_int &w3,
+		  const wi_range &vr0)
 {
+  signop sign = vr0.sign;
   /* Order pairs w0,w1 and w2,w3.  */
   if (wi::gt_p (w0, w1, sign))
     std::swap (w0, w1);
@@ -130,60 +125,55 @@  wide_int_range_min_max (wide_int &min, wide_int &max,
     std::swap (w2, w3);
 
   /* Choose min and max from the ordered pairs.  */
-  min = wi::min (w0, w2, sign);
-  max = wi::max (w1, w3, sign);
+  res.set (vr0, wi::min (w0, w2, sign), wi::max (w1, w3, sign));
 }
 
 /* Calculate the cross product of two sets of ranges (VR0 and VR1) and
-   store the result in [RES_LB, RES_UB].
-
-   CODE is the operation to perform with sign SIGN.
-
-   OVERFLOW_UNDEFINED is set if overflow is undefined for the operation type.
+   store the result in RES.
 
    Return TRUE if we were able to calculate the cross product.  */
 
 bool
-wide_int_range_cross_product (wide_int &res_lb, wide_int &res_ub,
-			      enum tree_code code, signop sign,
-			      const wide_int &vr0_lb, const wide_int &vr0_ub,
-			      const wide_int &vr1_lb, const wide_int &vr1_ub,
-			      bool overflow_undefined)
+wi_range_cross_product (wi_range &res,
+			enum tree_code code,
+			const wi_range &vr0, const wi_range &vr1)
 {
+  bool overflow_undefined = vr0.overflow_undefined;
+  signop sign = vr0.sign;
   wide_int cp1, cp2, cp3, cp4;
 
   /* Compute the 4 cross operations, bailing if we get an overflow we
      can't handle.  */
 
-  if (!wide_int_binop_overflow (cp1, code, vr0_lb, vr1_lb, sign,
+  if (!wide_int_binop_overflow (cp1, code, vr0.min, vr1.min, sign,
 				overflow_undefined))
     return false;
 
-  if (wi::eq_p (vr0_lb, vr0_ub))
+  if (wi::eq_p (vr0.min, vr0.max))
     cp3 = cp1;
-  else if (!wide_int_binop_overflow (cp3, code, vr0_ub, vr1_lb, sign,
+  else if (!wide_int_binop_overflow (cp3, code, vr0.max, vr1.min, sign,
 				     overflow_undefined))
     return false;
 
-  if (wi::eq_p (vr1_lb, vr1_ub))
+  if (wi::eq_p (vr1.min, vr1.max))
     cp2 = cp1;
-  else if (!wide_int_binop_overflow (cp2, code, vr0_lb, vr1_ub, sign,
-				     overflow_undefined))
+  else if (!wide_int_binop_overflow (cp2, code, vr0.min, vr1.max,
+				     sign, overflow_undefined))
     return false;
 
-  if (wi::eq_p (vr0_lb, vr0_ub))
+  if (wi::eq_p (vr0.min, vr0.max))
     cp4 = cp2;
-  else if (!wide_int_binop_overflow (cp4, code, vr0_ub, vr1_ub, sign,
+  else if (!wide_int_binop_overflow (cp4, code, vr0.max, vr1.max, sign,
 				     overflow_undefined))
     return false;
 
-  wide_int_range_min_max (res_lb, res_ub, cp1, cp2, cp3, cp4, sign);
+  wi_range_min_max (res, cp1, cp2, cp3, cp4, vr0);
   return true;
 }
 
 /* Multiply two ranges when TYPE_OVERFLOW_WRAPS:
 
-     [RES_LB, RES_UB] = [MIN0, MAX0] * [MIN1, MAX1]
+     RES = VR0 * VR1
 
    This is basically fancy code so we don't drop to varying with an
    unsigned [-3,-1]*[-3,-1].
@@ -191,24 +181,20 @@  wide_int_range_cross_product (wide_int &res_lb, wide_int &res_ub,
    Return TRUE if we were able to perform the operation.  */
 
 bool
-wide_int_range_mult_wrapping (wide_int &res_lb,
-			      wide_int &res_ub,
-			      signop sign,
-			      unsigned prec,
-			      const wide_int &min0_,
-			      const wide_int &max0_,
-			      const wide_int &min1_,
-			      const wide_int &max1_)
+wi_range_mult_wrapping (wi_range &res,
+			const wi_range &vr0, const wi_range &vr1)
 {
+  signop sign = vr0.sign;
+  unsigned prec = vr0.precision;
   /* This test requires 2*prec bits if both operands are signed and
      2*prec + 2 bits if either is not.  Therefore, 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.  */
-  widest2_int min0 = widest2_int::from (min0_, sign);
-  widest2_int max0 = widest2_int::from (max0_, sign);
-  widest2_int min1 = widest2_int::from (min1_, sign);
-  widest2_int max1 = widest2_int::from (max1_, sign);
+  widest2_int min0 = widest2_int::from (vr0.min, sign);
+  widest2_int max0 = widest2_int::from (vr0.max, sign);
+  widest2_int min1 = widest2_int::from (vr1.min, sign);
+  widest2_int max1 = widest2_int::from (vr1.max, sign);
   widest2_int sizem1 = wi::mask <widest2_int> (prec, false);
   widest2_int size = sizem1 + 1;
 
@@ -255,14 +241,15 @@  wide_int_range_mult_wrapping (wide_int &res_lb,
     /* The range covers all values.  */
     return false;
 
-  res_lb = wide_int::from (prod0, prec, sign);
-  res_ub = wide_int::from (prod3, prec, sign);
+  res = vr0;
+  res.min = wide_int::from (prod0, prec, sign);
+  res.max = wide_int::from (prod3, prec, sign);
   return true;
 }
 
 /* Perform multiplicative operation CODE on two ranges:
 
-     [RES_LB, RES_UB] = [VR0_LB, VR0_UB] .CODE. [VR1_LB, VR1_LB]
+	RES = VR0 .CODE. VR1
 
    Return TRUE if we were able to perform the operation.
 
@@ -271,16 +258,8 @@  wide_int_range_mult_wrapping (wide_int &res_lb,
    may be swapped.  */
 
 bool
-wide_int_range_multiplicative_op (wide_int &res_lb, wide_int &res_ub,
-				  enum tree_code code,
-				  signop sign,
-				  unsigned prec,
-				  const wide_int &vr0_lb,
-				  const wide_int &vr0_ub,
-				  const wide_int &vr1_lb,
-				  const wide_int &vr1_ub,
-				  bool overflow_undefined,
-				  bool overflow_wraps)
+wi_range_multiplicative_op (wi_range &res, enum tree_code code,
+			    const wi_range &vr0, const wi_range &vr1)
 {
   /* Multiplications, divisions and shifts are a bit tricky to handle,
      depending on the mix of signs we have in the two ranges, we
@@ -294,19 +273,15 @@  wide_int_range_multiplicative_op (wide_int &res_lb, wide_int &res_ub,
      (MIN0 OP MIN1, MIN0 OP MAX1, MAX0 OP MIN1 and MAX0 OP MAX0 OP
      MAX1) and then figure the smallest and largest values to form
      the new range.  */
-  if (code == MULT_EXPR && overflow_wraps)
-    return wide_int_range_mult_wrapping (res_lb, res_ub,
-					 sign, prec,
-					 vr0_lb, vr0_ub, vr1_lb, vr1_ub);
-  return wide_int_range_cross_product (res_lb, res_ub,
-				       code, sign,
-				       vr0_lb, vr0_ub, vr1_lb, vr1_ub,
-				       overflow_undefined);
+  /* ?? Document that overflow wraps comes from vr1.  */
+  if (code == MULT_EXPR && vr1.overflow_wraps)
+    return wi_range_mult_wrapping (res, vr0, vr1);
+  return wi_range_cross_product (res, code, vr0, vr1);
 }
 
 /* Perform a left shift operation on two ranges:
 
-     [RES_LB, RES_UB] = [VR0_LB, VR0_UB] << [VR1_LB, VR1_LB]
+	RES = VR0 << VR1
 
    Return TRUE if we were able to perform the operation.
 
@@ -314,28 +289,27 @@  wide_int_range_multiplicative_op (wide_int &res_lb, wide_int &res_ub,
    because its contents components may be swapped.  */
 
 bool
-wide_int_range_lshift (wide_int &res_lb, wide_int &res_ub,
-		       signop sign, unsigned prec,
-		       const wide_int &vr0_lb, const wide_int &vr0_ub,
-		       const wide_int &vr1_lb, const wide_int &vr1_ub,
-		       bool overflow_undefined, bool overflow_wraps)
+wi_range_lshift (wi_range &res, const wi_range &vr0, const wi_range &vr1)
 {
+  unsigned prec = vr0.precision;
+  signop sign = vr0.sign;
+
   /* Transform left shifts by constants into multiplies.  */
-  if (wi::eq_p (vr1_lb, vr1_ub))
+  if (wi::eq_p (vr1.min, vr1.max))
     {
-      int shift = wi::extract_uhwi (vr1_ub, 0, vr1_ub.get_precision ());
-      wide_int tmp = wi::set_bit_in_zero (shift, prec);
-      return wide_int_range_multiplicative_op (res_lb, res_ub,
-					       MULT_EXPR, sign, prec,
-					       vr0_lb, vr0_ub, tmp, tmp,
-					       overflow_undefined,
-					       /*overflow_wraps=*/true);
+      int shift = wi::extract_uhwi (vr1.max, 0, vr1.min.get_precision ());
+      wide_int tmp = wi::set_bit_in_zero (shift, vr0.precision);
+      wi_range tmp_range = vr0;
+      tmp_range.min = tmp;
+      tmp_range.max = tmp;
+      tmp_range.overflow_wraps = true;
+      return wi_range_multiplicative_op (res, MULT_EXPR, vr0, tmp_range);
     }
 
   int overflow_pos = prec;
   if (sign == SIGNED)
     overflow_pos -= 1;
-  int bound_shift = overflow_pos - vr1_ub.to_shwi ();
+  int bound_shift = overflow_pos - vr1.max.to_shwi ();
   /* If bound_shift == HOST_BITS_PER_WIDE_INT, the llshift can
      overflow.  However, for that to happen, vr1.max needs to be
      zero, which means vr1 is a singleton range of zero, which
@@ -349,14 +323,14 @@  wide_int_range_lshift (wide_int &res_lb, wide_int &res_ub,
     {
       low_bound = bound;
       high_bound = complement;
-      if (wi::ltu_p (vr0_ub, low_bound))
+      if (wi::ltu_p (vr0.max, low_bound))
 	{
 	  /* [5, 6] << [1, 2] == [10, 24].  */
 	  /* We're shifting out only zeroes, the value increases
 	     monotonically.  */
 	  in_bounds = true;
 	}
-      else if (wi::ltu_p (high_bound, vr0_lb))
+      else if (wi::ltu_p (high_bound, vr0.min))
 	{
 	  /* [0xffffff00, 0xffffffff] << [1, 2]
 	     == [0xfffffc00, 0xfffffffe].  */
@@ -370,8 +344,8 @@  wide_int_range_lshift (wide_int &res_lb, wide_int &res_ub,
       /* [-1, 1] << [1, 2] == [-4, 4].  */
       low_bound = complement;
       high_bound = bound;
-      if (wi::lts_p (vr0_ub, high_bound)
-	  && wi::lts_p (low_bound, vr0_lb))
+      if (wi::lts_p (vr0.max, high_bound)
+	  && wi::lts_p (low_bound, vr0.min))
 	{
 	  /* For non-negative numbers, we're shifting out only
 	     zeroes, the value increases monotonically.
@@ -381,12 +355,7 @@  wide_int_range_lshift (wide_int &res_lb, wide_int &res_ub,
 	}
     }
   if (in_bounds)
-    return wide_int_range_multiplicative_op (res_lb, res_ub,
-					     LSHIFT_EXPR, sign, prec,
-					     vr0_lb, vr0_ub,
-					     vr1_lb, vr1_ub,
-					     overflow_undefined,
-					     overflow_wraps);
+    return wi_range_multiplicative_op (res, LSHIFT_EXPR, vr0, vr1);
   return false;
 }
 
@@ -402,9 +371,9 @@  wide_int_range_lshift (wide_int &res_lb, wide_int &res_ub,
    It is up to the caller to perform the actual folding above.  */
 
 bool
-wide_int_range_can_optimize_bit_op (tree_code code,
-				    const wide_int &lb, const wide_int &ub,
-				    const wide_int &mask)
+wi_range_can_optimize_bit_op (tree_code code,
+			      const wide_int &lb, const wide_int &ub,
+			      const wide_int &mask)
 
 {
   if (code != BIT_AND_EXPR && code != BIT_IOR_EXPR)
@@ -441,167 +410,157 @@  wide_int_range_can_optimize_bit_op (tree_code code,
   return false;
 }
 
-/* Calculate the XOR of two ranges and store the result in [WMIN,WMAX].
-   The two input ranges are described by their MUST_BE_NONZERO and
-   MAY_BE_NONZERO bit masks.
+/* Calculate the XOR of two ranges and store the result in RES.  The
+   two input ranges are described by their MUST_BE and MAY_BE nonzero
+   bit masks.
 
    Return TRUE if we were able to successfully calculate the new range.  */
 
 bool
-wide_int_range_bit_xor (wide_int &wmin, wide_int &wmax,
-			signop sign,
-			unsigned prec,
-			const wide_int &must_be_nonzero0,
-			const wide_int &may_be_nonzero0,
-			const wide_int &must_be_nonzero1,
-			const wide_int &may_be_nonzero1)
+wi_range_bit_xor (wi_range &res,
+		  const wi_range &vr0,
+		  const wi_range &vr1 ATTRIBUTE_UNUSED,
+		  const maybe_nonzero_bits &nz0,
+		  const maybe_nonzero_bits &nz1)
 {
-  wide_int result_zero_bits = ((must_be_nonzero0 & must_be_nonzero1)
-			       | ~(may_be_nonzero0 | may_be_nonzero1));
+  unsigned prec = vr0.precision;
+  signop sign = vr0.sign;
+  wide_int result_zero_bits = ((nz0.must_be & nz1.must_be)
+			       | ~(nz0.may_be | nz1.may_be));
   wide_int result_one_bits
-    = (wi::bit_and_not (must_be_nonzero0, may_be_nonzero1)
-       | wi::bit_and_not (must_be_nonzero1, may_be_nonzero0));
-  wmax = ~result_zero_bits;
-  wmin = result_one_bits;
+    = (wi::bit_and_not (nz0.must_be, nz1.may_be)
+       | wi::bit_and_not (nz1.must_be, nz0.may_be));
+  res.set (vr0, result_one_bits, ~result_zero_bits);
   /* If the range has all positive or all negative values, the result
      is better than VARYING.  */
-  if (wi::lt_p (wmin, 0, sign) || wi::ge_p (wmax, 0, sign))
+  if (wi::lt_p (res.min, 0, sign) || wi::ge_p (res.max, 0, sign))
     return true;
-  wmin = wi::min_value (prec, sign);
-  wmax = wi::max_value (prec, sign);
+  res.min = wi::min_value (prec, sign);
+  res.max = wi::max_value (prec, sign);
   return false;
 }
 
-/* Calculate the IOR of two ranges and store the result in [WMIN,WMAX].
+/* Calculate the IOR of two ranges and store the result in RES.
    Return TRUE if we were able to successfully calculate the new range.  */
 
 bool
-wide_int_range_bit_ior (wide_int &wmin, wide_int &wmax,
-			signop sign,
-			const wide_int &vr0_min,
-			const wide_int &vr0_max,
-			const wide_int &vr1_min,
-			const wide_int &vr1_max,
-			const wide_int &must_be_nonzero0,
-			const wide_int &may_be_nonzero0,
-			const wide_int &must_be_nonzero1,
-			const wide_int &may_be_nonzero1)
+wi_range_bit_ior (wi_range &res,
+		  const wi_range &vr0, const wi_range &vr1,
+		  const maybe_nonzero_bits &nz0,
+		  const maybe_nonzero_bits &nz1)
 {
-  wmin = must_be_nonzero0 | must_be_nonzero1;
-  wmax = may_be_nonzero0 | may_be_nonzero1;
+  signop sign = vr0.sign;
+  res.set (vr0, nz0.must_be | nz1.must_be, nz0.may_be | nz1.may_be);
   /* If the input ranges contain only positive values we can
      truncate the minimum of the result range to the maximum
      of the input range minima.  */
-  if (wi::ge_p (vr0_min, 0, sign)
-      && wi::ge_p (vr1_min, 0, sign))
+  if (wi::ge_p (vr0.min, 0, sign)
+      && wi::ge_p (vr1.min, 0, sign))
     {
-      wmin = wi::max (wmin, vr0_min, sign);
-      wmin = wi::max (wmin, vr1_min, sign);
+      res.min = wi::max (res.min, vr0.min, sign);
+      res.min = wi::max (res.min, vr1.min, sign);
     }
   /* If either input range contains only negative values
      we can truncate the minimum of the result range to the
      respective minimum range.  */
-  if (wi::lt_p (vr0_max, 0, sign))
-    wmin = wi::max (wmin, vr0_min, sign);
-  if (wi::lt_p (vr1_max, 0, sign))
-    wmin = wi::max (wmin, vr1_min, sign);
+  if (wi::lt_p (vr0.max, 0, sign))
+    res.min = wi::max (res.min, vr0.min, sign);
+  if (wi::lt_p (vr1.max, 0, sign))
+    res.min = wi::max (res.min, vr1.min, sign);
   /* If the limits got swapped around, indicate error so we can adjust
      the range to VARYING.  */
-  if (wi::gt_p (wmin, wmax,sign))
+  if (wi::gt_p (res.min, res.max, sign))
     return false;
   return true;
 }
 
-/* Calculate the bitwise AND of two ranges and store the result in [WMIN,WMAX].
+/* Calculate the bitwise AND of two ranges and store the result in RES.
    Return TRUE if we were able to successfully calculate the new range.  */
 
 bool
-wide_int_range_bit_and (wide_int &wmin, wide_int &wmax,
-			signop sign,
-			unsigned prec,
-			const wide_int &vr0_min,
-			const wide_int &vr0_max,
-			const wide_int &vr1_min,
-			const wide_int &vr1_max,
-			const wide_int &must_be_nonzero0,
-			const wide_int &may_be_nonzero0,
-			const wide_int &must_be_nonzero1,
-			const wide_int &may_be_nonzero1)
+wi_range_bit_and (wi_range &res,
+		  const wi_range &vr0, const wi_range &vr1,
+		  const maybe_nonzero_bits &nz0,
+		  const maybe_nonzero_bits &nz1)
 {
-  wmin = must_be_nonzero0 & must_be_nonzero1;
-  wmax = may_be_nonzero0 & may_be_nonzero1;
+  signop sign = vr0.sign;
+  res.set (vr0, nz0.must_be & nz1.must_be, nz0.may_be & nz1.may_be);
   /* If both input ranges contain only negative values we can
      truncate the result range maximum to the minimum of the
      input range maxima.  */
-  if (wi::lt_p (vr0_max, 0, sign) && wi::lt_p (vr1_max, 0, sign))
+  if (wi::lt_p (vr0.max, 0, sign) && wi::lt_p (vr1.max, 0, sign))
     {
-      wmax = wi::min (wmax, vr0_max, sign);
-      wmax = wi::min (wmax, vr1_max, sign);
+      res.max = wi::min (res.max, vr0.max, sign);
+      res.max = wi::min (res.max, vr1.max, sign);
     }
   /* If either input range contains only non-negative values
      we can truncate the result range maximum to the respective
      maximum of the input range.  */
-  if (wi::ge_p (vr0_min, 0, sign))
-    wmax = wi::min (wmax, vr0_max, sign);
-  if (wi::ge_p (vr1_min, 0, sign))
-    wmax = wi::min (wmax, vr1_max, sign);
+  if (wi::ge_p (vr0.min, 0, sign))
+    res.max = wi::min (res.max, vr0.max, sign);
+  if (wi::ge_p (vr1.min, 0, sign))
+    res.max = wi::min (res.max, vr1.max, sign);
   /* PR68217: In case of signed & sign-bit-CST should
      result in [-INF, 0] instead of [-INF, INF].  */
-  if (wi::gt_p (wmin, wmax, sign))
+  if (wi::gt_p (res.min, res.max, sign))
     {
+      unsigned prec = vr0.precision;
       wide_int sign_bit = wi::set_bit_in_zero (prec - 1, prec);
       if (sign == SIGNED
-	  && ((wi::eq_p (vr0_min, vr0_max)
-	       && !wi::cmps (vr0_min, sign_bit))
-	      || (wi::eq_p (vr1_min, vr1_max)
-		  && !wi::cmps (vr1_min, sign_bit))))
+	  && ((wi::eq_p (vr0.min, vr0.max)
+	       && !wi::cmps (vr0.min, sign_bit))
+	      || (wi::eq_p (vr1.min, vr1.max)
+		  && !wi::cmps (vr1.min, sign_bit))))
 	{
-	  wmin = wi::min_value (prec, sign);
-	  wmax = wi::zero (prec);
+	  res.min = wi::min_value (prec, sign);
+	  res.max = wi::zero (prec);
 	}
     }
   /* If the limits got swapped around, indicate error so we can adjust
      the range to VARYING.  */
-  if (wi::gt_p (wmin, wmax,sign))
+  if (wi::gt_p (res.min, res.max, sign))
     return false;
   return true;
 }
 
-/* Calculate TRUNC_MOD_EXPR on two ranges and store the result in
-   [WMIN,WMAX].  */
+/* Calculate TRUNC_MOD_EXPR on two ranges and store the result in RES.
+   Return TRUE if operation succeeded.  */
 
-void
-wide_int_range_trunc_mod (wide_int &wmin, wide_int &wmax,
-			  signop sign,
-			  unsigned prec,
-			  const wide_int &vr0_min,
-			  const wide_int &vr0_max,
-			  const wide_int &vr1_min,
-			  const wide_int &vr1_max)
+bool
+wi_range_trunc_mod (wi_range &res, const wi_range &vr0, const wi_range &vr1)
 {
   wide_int tmp;
+  signop sign = vr0.sign;
+  unsigned prec = vr0.precision;
+
+  if (vr1.zero_p ())
+    return false;
+
+  /* Copy vr0'0 sign, precision, etc attributes into a fresh result.  */
+  res = vr0;
 
   /* ABS (A % B) < ABS (B) and either
      0 <= A % B <= A or A <= A % B <= 0.  */
-  wmax = vr1_max - 1;
+  res.max = vr1.max - 1;
   if (sign == SIGNED)
     {
-      tmp = -1 - vr1_min;
-      wmax = wi::smax (wmax, tmp);
+      tmp = -1 - vr1.min;
+      res.max = wi::smax (res.max, tmp);
     }
 
   if (sign == UNSIGNED)
-    wmin = wi::zero (prec);
+    res.min = wi::zero (prec);
   else
     {
-      wmin = -wmax;
-      tmp = vr0_min;
+      res.min = -res.max;
+      tmp = vr0.min;
       if (wi::gts_p (tmp, 0))
 	tmp = wi::zero (prec);
-      wmin = wi::smax (wmin, tmp);
+      res.min = wi::smax (res.min, tmp);
     }
-  tmp = vr0_max;
+  tmp = vr0.max;
   if (sign == SIGNED && wi::neg_p (tmp))
     tmp = wi::zero (prec);
-  wmax = wi::min (wmax, tmp, sign);
+  res.max = wi::min (res.max, tmp, sign);
+  return true;
 }
diff --git a/gcc/wide-int-range.h b/gcc/wide-int-range.h
index 4421bc8aeca..987352b7d6e 100644
--- a/gcc/wide-int-range.h
+++ b/gcc/wide-int-range.h
@@ -20,86 +20,47 @@  along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_WIDE_INT_RANGE_H
 #define GCC_WIDE_INT_RANGE_H
 
-extern bool wide_int_range_cross_product (wide_int &res_lb, wide_int &res_ub,
-					  enum tree_code code, signop sign,
-					  const wide_int &, const wide_int &,
-					  const wide_int &, const wide_int &,
-					  bool overflow_undefined);
-extern bool wide_int_range_mult_wrapping (wide_int &res_lb,
-					  wide_int &res_ub,
-					  signop sign,
-					  unsigned prec,
-					  const wide_int &min0_,
-					  const wide_int &max0_,
-					  const wide_int &min1_,
-					  const wide_int &max1_);
-extern bool wide_int_range_multiplicative_op (wide_int &res_lb,
-					      wide_int &res_ub,
-					      enum tree_code code,
-					      signop sign,
-					      unsigned prec,
-					      const wide_int &vr0_lb,
-					      const wide_int &vr0_ub,
-					      const wide_int &vr1_lb,
-					      const wide_int &vr1_ub,
-					      bool overflow_undefined,
-					      bool overflow_wraps);
-extern bool wide_int_range_lshift (wide_int &res_lb, wide_int &res_ub,
-				   signop sign, unsigned prec,
-				   const wide_int &, const wide_int &,
-				   const wide_int &, const wide_int &,
-				   bool overflow_undefined,
-				   bool overflow_wraps);
-extern void wide_int_range_set_zero_nonzero_bits (signop,
-						  const wide_int &lb,
-						  const wide_int &ub,
-						  wide_int &may_be_nonzero,
-						  wide_int &must_be_nonzero);
-extern bool wide_int_range_can_optimize_bit_op (tree_code,
-						const wide_int &lb,
-						const wide_int &ub,
-						const wide_int &mask);
-extern bool wide_int_range_bit_xor (wide_int &wmin, wide_int &wmax,
-				    signop sign,
-				    unsigned prec,
-				    const wide_int &must_be_nonzero0,
-				    const wide_int &may_be_nonzero0,
-				    const wide_int &must_be_nonzero1,
-				    const wide_int &may_be_nonzero1);
-extern bool wide_int_range_bit_ior (wide_int &wmin, wide_int &wmax,
-				    signop sign,
-				    const wide_int &vr0_min,
-				    const wide_int &vr0_max,
-				    const wide_int &vr1_min,
-				    const wide_int &vr1_max,
-				    const wide_int &must_be_nonzero0,
-				    const wide_int &may_be_nonzero0,
-				    const wide_int &must_be_nonzero1,
-				    const wide_int &may_be_nonzero1);
-extern bool wide_int_range_bit_and (wide_int &wmin, wide_int &wmax,
-				    signop sign,
-				    unsigned prec,
-				    const wide_int &vr0_min,
-				    const wide_int &vr0_max,
-				    const wide_int &vr1_min,
-				    const wide_int &vr1_max,
-				    const wide_int &must_be_nonzero0,
-				    const wide_int &may_be_nonzero0,
-				    const wide_int &must_be_nonzero1,
-				    const wide_int &may_be_nonzero1);
-extern void wide_int_range_trunc_mod (wide_int &wmin, wide_int &wmax,
-				      signop sign,
-				      unsigned prec,
-				      const wide_int &vr0_min,
-				      const wide_int &vr0_max,
-				      const wide_int &vr1_min,
-				      const wide_int &vr1_max);
+extern bool wi_range_cross_product (wi_range &res,
+				    enum tree_code code,
+				    const wi_range &vr0,
+				    const wi_range &vr1);
+extern bool wi_range_mult_wrapping (wi_range &res,
+				    const wi_range &vr0,
+				    const wi_range &vr1);
+extern bool wi_range_multiplicative_op (wi_range &res,
+					enum tree_code code,
+					const wi_range &vr0,
+					const wi_range &vr1);
+extern bool wi_range_lshift (wi_range &res,
+			     const wi_range &vr0, const wi_range &vr1);
+extern void wi_range_set_zero_nonzero_bits (const wi_range &r,
+					    maybe_nonzero_bits *nonzero);
+extern bool wi_range_can_optimize_bit_op (tree_code,
+					  const wide_int &lb,
+					  const wide_int &ub,
+					  const wide_int &mask);
+extern bool wi_range_bit_xor (wi_range &res,
+			      const wi_range &vr0,
+			      const wi_range &vr1,
+			      const maybe_nonzero_bits &nz0,
+			      const maybe_nonzero_bits &nz1);
+extern bool wi_range_bit_ior (wi_range &res,
+			      const wi_range &vr0,
+			      const wi_range &vr1,
+			      const maybe_nonzero_bits &nz0,
+			      const maybe_nonzero_bits &nz1);
+extern bool wi_range_bit_and (wi_range &res,
+			      const wi_range &vr0,
+			      const wi_range &vr1,
+			      const maybe_nonzero_bits &nz0,
+			      const maybe_nonzero_bits &nz1);
+extern bool wi_range_trunc_mod (wi_range &res,
+				const wi_range &vr0, const wi_range &vr1);
 
 /* Return TRUE if shifting by range [MIN, MAX] is undefined behavior.  */
 
 inline bool
-wide_int_range_shift_undefined_p (signop sign, unsigned prec,
-				  const wide_int &min, const wide_int &max)
+wi_range_shift_undefined_p (const wi_range &shifter)
 {
   /* ?? Note: The original comment said this only applied to
      RSHIFT_EXPR, but it was being applied to both left and right
@@ -109,7 +70,9 @@  wide_int_range_shift_undefined_p (signop sign, unsigned prec,
      behavior from the shift operation.  We cannot even trust
      SHIFT_COUNT_TRUNCATED at this stage, because that applies to rtl
      shifts, and the operation at the tree level may be widened.  */
-  return wi::lt_p (min, 0, sign) || wi::ge_p (max, prec, sign);
+  signop sign = shifter.sign;
+  unsigned prec = shifter.precision;
+  return wi::lt_p (shifter.min, 0, sign) || wi::ge_p (shifter.max, prec, sign);
 }
 
 #endif /* GCC_WIDE_INT_RANGE_H */