PR tree-optimization/96818 - cast label range to type of switch operand

As discussed in the PR, the type of a switch operand may be different 
than the type of the individual cases.  This is causing us to attempt to 
intersect incompatible ranges.

This inconsistent IL seems wrong to me, but it also seems pervasive 
throughout GCC.  Jason, as one of the original gimple designers, do you 
remember if this was an oversight, or was there a reason for this?

Richard, you mention in the PR that normalizing the IL would cause 
propagation of widening cast sources into switches harder.  Couldn't we 
just cast all labels to the switch operand type upon creation?  What 
would be the problem with that?  Would we generate sub-optimal code?

In either case, I'd hate to leave trunk in a broken case while this is 
being discussed.  The attached patch fixes the PRs.

OK?

Aldy

On Mon, Aug 31, 2020 at 10:20 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> As discussed in the PR, the type of a switch operand may be different
> than the type of the individual cases.  This is causing us to attempt to
> intersect incompatible ranges.
>
> This inconsistent IL seems wrong to me, but it also seems pervasive
> throughout GCC.  Jason, as one of the original gimple designers, do you
> remember if this was an oversight, or was there a reason for this?
>
> Richard, you mention in the PR that normalizing the IL would cause
> propagation of widening cast sources into switches harder.  Couldn't we
> just cast all labels to the switch operand type upon creation?  What
> would be the problem with that?  Would we generate sub-optimal code?
>
> In either case, I'd hate to leave trunk in a broken case while this is
> being discussed.  The attached patch fixes the PRs.
>
> OK?

       widest_irange label_range (CASE_LOW (min_label), case_high);
+      if (!types_compatible_p (label_range.type (), range_of_op->type ()))
+       range_cast (label_range, range_of_op->type ());
       label_range.intersect (range_of_op);

so label_range is of 'widest_int' type but then instead of casting
range_of_op to widest_irange you cast that widest_irange to the
type of the range op?  Why not build label_range in the type
of range_op immediately?

Using widest_int for both would have been obvious to me, you're
indicating that switch (type op) { case type2 lab: } is supposed to
match as (type)lab == op rather than (type2)op == lab.  I think
the IL will be consistent in a way that sign-extending both
op and lab to a common larger integer type is correct
(thus using widest_int), but no truncation should happen here
(such trucation should be applied by the frontend).

In any case type mismatches here are of course unfortunate
and both more verification and documentation would be
nice.  verify_gimple_switch only verifies all case labels
have the same type, the type of the switch argument is
not verified in any way against that type.

Richard.


> Aldy

On Mon, Aug 31, 2020 at 10:33:20AM +0200, Richard Biener wrote:
> In any case type mismatches here are of course unfortunate
> and both more verification and documentation would be
> nice.  verify_gimple_switch only verifies all case labels
> have the same type, the type of the switch argument is
> not verified in any way against that type.

When looking at the verification, I have noticed a bug in it.

The verification that CASE_HIGH (if present) has the same type as CASE_LOW
is only performed for the case label 2 and higher, case label 1 (the first
one after the default label) isn't checked.

The following patch fixes that, it will uselessly also compare
TREE_TYPE (CASE_LOW (elt)) != elt_type for the case label 1, but I think
that isn't that expensive and helps readability of the code.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2020-08-31  Jakub Jelinek  <jakub@redhat.com>

	* tree-cfg.c (verify_gimple_switch): If the first non-default case
	label has CASE_HIGH, verify it has the same type as CASE_LOW.

--- gcc/tree-cfg.c.jj	2020-08-27 18:42:35.660711349 +0200
+++ gcc/tree-cfg.c	2020-08-27 22:46:34.574154370 +0200
@@ -4809,17 +4809,7 @@ verify_gimple_switch (gswitch *stmt)
 	  return true;
 	}
 
-      if (elt_type)
-	{
-	  if (TREE_TYPE (CASE_LOW (elt)) != elt_type
-	      || (CASE_HIGH (elt) && TREE_TYPE (CASE_HIGH (elt)) != elt_type))
-	    {
-	      error ("type mismatch for case label in switch statement");
-	      debug_generic_expr (elt);
-	      return true;
-	    }
-	}
-      else
+      if (! elt_type)
 	{
 	  elt_type = TREE_TYPE (CASE_LOW (elt));
 	  if (TYPE_PRECISION (index_type) < TYPE_PRECISION (elt_type))
@@ -4828,6 +4818,13 @@ verify_gimple_switch (gswitch *stmt)
 	      return true;
 	    }
 	}
+      if (TREE_TYPE (CASE_LOW (elt)) != elt_type
+          || (CASE_HIGH (elt) && TREE_TYPE (CASE_HIGH (elt)) != elt_type))
+	{
+	  error ("type mismatch for case label in switch statement");
+	  debug_generic_expr (elt);
+	  return true;
+	}
 
       if (prev_upper_bound)
 	{


	Jakub

On Mon, Aug 31, 2020 at 10:39 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Mon, Aug 31, 2020 at 10:33:20AM +0200, Richard Biener wrote:
> > In any case type mismatches here are of course unfortunate
> > and both more verification and documentation would be
> > nice.  verify_gimple_switch only verifies all case labels
> > have the same type, the type of the switch argument is
> > not verified in any way against that type.
>
> When looking at the verification, I have noticed a bug in it.
>
> The verification that CASE_HIGH (if present) has the same type as CASE_LOW
> is only performed for the case label 2 and higher, case label 1 (the first
> one after the default label) isn't checked.
>
> The following patch fixes that, it will uselessly also compare
> TREE_TYPE (CASE_LOW (elt)) != elt_type for the case label 1, but I think
> that isn't that expensive and helps readability of the code.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> 2020-08-31  Jakub Jelinek  <jakub@redhat.com>
>
>         * tree-cfg.c (verify_gimple_switch): If the first non-default case
>         label has CASE_HIGH, verify it has the same type as CASE_LOW.
>
> --- gcc/tree-cfg.c.jj   2020-08-27 18:42:35.660711349 +0200
> +++ gcc/tree-cfg.c      2020-08-27 22:46:34.574154370 +0200
> @@ -4809,17 +4809,7 @@ verify_gimple_switch (gswitch *stmt)
>           return true;
>         }
>
> -      if (elt_type)
> -       {
> -         if (TREE_TYPE (CASE_LOW (elt)) != elt_type
> -             || (CASE_HIGH (elt) && TREE_TYPE (CASE_HIGH (elt)) != elt_type))
> -           {
> -             error ("type mismatch for case label in switch statement");
> -             debug_generic_expr (elt);
> -             return true;
> -           }
> -       }
> -      else
> +      if (! elt_type)
>         {
>           elt_type = TREE_TYPE (CASE_LOW (elt));
>           if (TYPE_PRECISION (index_type) < TYPE_PRECISION (elt_type))
> @@ -4828,6 +4818,13 @@ verify_gimple_switch (gswitch *stmt)
>               return true;
>             }
>         }
> +      if (TREE_TYPE (CASE_LOW (elt)) != elt_type
> +          || (CASE_HIGH (elt) && TREE_TYPE (CASE_HIGH (elt)) != elt_type))
> +       {
> +         error ("type mismatch for case label in switch statement");
> +         debug_generic_expr (elt);
> +         return true;
> +       }
>
>        if (prev_upper_bound)
>         {
>
>
>         Jakub
>

On 8/31/20 10:33 AM, Richard Biener wrote:
> On Mon, Aug 31, 2020 at 10:20 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>>
>> As discussed in the PR, the type of a switch operand may be different
>> than the type of the individual cases.  This is causing us to attempt to
>> intersect incompatible ranges.
>>
>> This inconsistent IL seems wrong to me, but it also seems pervasive
>> throughout GCC.  Jason, as one of the original gimple designers, do you
>> remember if this was an oversight, or was there a reason for this?
>>
>> Richard, you mention in the PR that normalizing the IL would cause
>> propagation of widening cast sources into switches harder.  Couldn't we
>> just cast all labels to the switch operand type upon creation?  What
>> would be the problem with that?  Would we generate sub-optimal code?
>>
>> In either case, I'd hate to leave trunk in a broken case while this is
>> being discussed.  The attached patch fixes the PRs.
>>
>> OK?
> 
>         widest_irange label_range (CASE_LOW (min_label), case_high);
> +      if (!types_compatible_p (label_range.type (), range_of_op->type ()))
> +       range_cast (label_range, range_of_op->type ());
>         label_range.intersect (range_of_op);
> 
> so label_range is of 'widest_int' type but then instead of casting
> range_of_op to widest_irange you cast that widest_irange to the
> type of the range op?  Why not build label_range in the type
> of range_op immediately?

The "widest" in widest_irange does not denote the type of the range, but 
the number of sub-ranges that can maximally fit.  It has nothing to do 
with the underlying type of its elements.  Instead, it is supposed to 
indicate that label_range can fit an infinite amount of sub-ranges.  In 
practice this is 255 sub-ranges, but we can adjust that if needed.

So, in the constructor:

widest_irange label_range (CASE_LOW (min_label), case_high);

...the type for the sub-ranges in label_range is the type of 
CASE_LOW(min_label) and case_high.  They must match IIRC.

I am working on an irange best practices document that should go into 
detail on all this, and will post it later this week.

> 
> Using widest_int for both would have been obvious to me, you're
> indicating that switch (type op) { case type2 lab: } is supposed to
> match as (type)lab == op rather than (type2)op == lab.  I think
> the IL will be consistent in a way that sign-extending both
> op and lab to a common larger integer type is correct
> (thus using widest_int), but no truncation should happen here
> (such trucation should be applied by the frontend).

I suppose we could convert both label_range and range_of_op to a wider 
type and do the intersection on these results.  Is there a tree type 
that indicates a widest possible int?

> 
> In any case type mismatches here are of course unfortunate
> and both more verification and documentation would be
> nice.  verify_gimple_switch only verifies all case labels
> have the same type, the type of the switch argument is
> not verified in any way against that type.

Is the verification good enough with what Jakub posted?  I can adjust 
the documentation, but first I'd like a nod from Jason that this was 
indeed expected behavior.

Thanks.
Aldy

On Mon, Aug 31, 2020 at 11:10 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>
>
>
> On 8/31/20 10:33 AM, Richard Biener wrote:
> > On Mon, Aug 31, 2020 at 10:20 AM Aldy Hernandez <aldyh@redhat.com> wrote:
> >>
> >> As discussed in the PR, the type of a switch operand may be different
> >> than the type of the individual cases.  This is causing us to attempt to
> >> intersect incompatible ranges.
> >>
> >> This inconsistent IL seems wrong to me, but it also seems pervasive
> >> throughout GCC.  Jason, as one of the original gimple designers, do you
> >> remember if this was an oversight, or was there a reason for this?
> >>
> >> Richard, you mention in the PR that normalizing the IL would cause
> >> propagation of widening cast sources into switches harder.  Couldn't we
> >> just cast all labels to the switch operand type upon creation?  What
> >> would be the problem with that?  Would we generate sub-optimal code?
> >>
> >> In either case, I'd hate to leave trunk in a broken case while this is
> >> being discussed.  The attached patch fixes the PRs.
> >>
> >> OK?
> >
> >         widest_irange label_range (CASE_LOW (min_label), case_high);
> > +      if (!types_compatible_p (label_range.type (), range_of_op->type ()))
> > +       range_cast (label_range, range_of_op->type ());
> >         label_range.intersect (range_of_op);
> >
> > so label_range is of 'widest_int' type but then instead of casting
> > range_of_op to widest_irange you cast that widest_irange to the
> > type of the range op?  Why not build label_range in the type
> > of range_op immediately?
>
> The "widest" in widest_irange does not denote the type of the range, but
> the number of sub-ranges that can maximally fit.  It has nothing to do
> with the underlying type of its elements.  Instead, it is supposed to
> indicate that label_range can fit an infinite amount of sub-ranges.  In
> practice this is 255 sub-ranges, but we can adjust that if needed.

That's definitely confusing naming then :/

> So, in the constructor:
>
> widest_irange label_range (CASE_LOW (min_label), case_high);
>
> ...the type for the sub-ranges in label_range is the type of
> CASE_LOW(min_label) and case_high.  They must match IIRC.
>
> I am working on an irange best practices document that should go into
> detail on all this, and will post it later this week.
>
> >
> > Using widest_int for both would have been obvious to me, you're
> > indicating that switch (type op) { case type2 lab: } is supposed to
> > match as (type)lab == op rather than (type2)op == lab.  I think
> > the IL will be consistent in a way that sign-extending both
> > op and lab to a common larger integer type is correct
> > (thus using widest_int), but no truncation should happen here
> > (such trucation should be applied by the frontend).
>
> I suppose we could convert both label_range and range_of_op to a wider
> type and do the intersection on these results.  Is there a tree type
> that indicates a widest possible int?

Btw, just looked up how we expand GIMPLE_SWITCH and there we
convert case label values to the type of the index expression
(what your patch did).

So the patch is OK.

Richard.



> >
> > In any case type mismatches here are of course unfortunate
> > and both more verification and documentation would be
> > nice.  verify_gimple_switch only verifies all case labels
> > have the same type, the type of the switch argument is
> > not verified in any way against that type.
>
> Is the verification good enough with what Jakub posted?  I can adjust
> the documentation, but first I'd like a nod from Jason that this was
> indeed expected behavior.
>
> Thanks.
> Aldy
>

On 8/31/20 5:10 AM, Aldy Hernandez wrote:
> On 8/31/20 10:33 AM, Richard Biener wrote:
>> On Mon, Aug 31, 2020 at 10:20 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>>>
>>> As discussed in the PR, the type of a switch operand may be different
>>> than the type of the individual cases.  This is causing us to attempt to
>>> intersect incompatible ranges.
>>>
>>> This inconsistent IL seems wrong to me, but it also seems pervasive
>>> throughout GCC.  Jason, as one of the original gimple designers, do you
>>> remember if this was an oversight, or was there a reason for this?
>>>
>>> Richard, you mention in the PR that normalizing the IL would cause
>>> propagation of widening cast sources into switches harder.  Couldn't we
>>> just cast all labels to the switch operand type upon creation?  What
>>> would be the problem with that?  Would we generate sub-optimal code?
>>>
>>> In either case, I'd hate to leave trunk in a broken case while this is
>>> being discussed.  The attached patch fixes the PRs.
>>>
>>> OK?
>>
>>         widest_irange label_range (CASE_LOW (min_label), case_high);
>> +      if (!types_compatible_p (label_range.type (), range_of_op->type 
>> ()))
>> +       range_cast (label_range, range_of_op->type ());
>>         label_range.intersect (range_of_op);
>>
>> so label_range is of 'widest_int' type but then instead of casting
>> range_of_op to widest_irange you cast that widest_irange to the
>> type of the range op?  Why not build label_range in the type
>> of range_op immediately?
> 
> The "widest" in widest_irange does not denote the type of the range, but 
> the number of sub-ranges that can maximally fit.  It has nothing to do 
> with the underlying type of its elements.  Instead, it is supposed to 
> indicate that label_range can fit an infinite amount of sub-ranges.  In 
> practice this is 255 sub-ranges, but we can adjust that if needed.
> 
> So, in the constructor:
> 
> widest_irange label_range (CASE_LOW (min_label), case_high);
> 
> ...the type for the sub-ranges in label_range is the type of 
> CASE_LOW(min_label) and case_high.  They must match IIRC.
> 
> I am working on an irange best practices document that should go into 
> detail on all this, and will post it later this week.
> 
>>
>> Using widest_int for both would have been obvious to me, you're
>> indicating that switch (type op) { case type2 lab: } is supposed to
>> match as (type)lab == op rather than (type2)op == lab.  I think
>> the IL will be consistent in a way that sign-extending both
>> op and lab to a common larger integer type is correct
>> (thus using widest_int), but no truncation should happen here
>> (such trucation should be applied by the frontend).
> 
> I suppose we could convert both label_range and range_of_op to a wider 
> type and do the intersection on these results.  Is there a tree type 
> that indicates a widest possible int?
> 
>> In any case type mismatches here are of course unfortunate
>> and both more verification and documentation would be
>> nice.  verify_gimple_switch only verifies all case labels
>> have the same type, the type of the switch argument is
>> not verified in any way against that type.
> 
> Is the verification good enough with what Jakub posted?  I can adjust 
> the documentation, but first I'd like a nod from Jason that this was 
> indeed expected behavior.

I don't remember any such decision.  In C++, the case values are 
converted to the type of the switch argument (in case_conversion).

Jason

On 8/31/20 2:55 PM, Richard Biener wrote:
> On Mon, Aug 31, 2020 at 11:10 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>>
>>
>>
>> On 8/31/20 10:33 AM, Richard Biener wrote:
>>> On Mon, Aug 31, 2020 at 10:20 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>>>>
>>>> As discussed in the PR, the type of a switch operand may be different
>>>> than the type of the individual cases.  This is causing us to attempt to
>>>> intersect incompatible ranges.
>>>>
>>>> This inconsistent IL seems wrong to me, but it also seems pervasive
>>>> throughout GCC.  Jason, as one of the original gimple designers, do you
>>>> remember if this was an oversight, or was there a reason for this?
>>>>
>>>> Richard, you mention in the PR that normalizing the IL would cause
>>>> propagation of widening cast sources into switches harder.  Couldn't we
>>>> just cast all labels to the switch operand type upon creation?  What
>>>> would be the problem with that?  Would we generate sub-optimal code?
>>>>
>>>> In either case, I'd hate to leave trunk in a broken case while this is
>>>> being discussed.  The attached patch fixes the PRs.
>>>>
>>>> OK?
>>>
>>>          widest_irange label_range (CASE_LOW (min_label), case_high);
>>> +      if (!types_compatible_p (label_range.type (), range_of_op->type ()))
>>> +       range_cast (label_range, range_of_op->type ());
>>>          label_range.intersect (range_of_op);
>>>
>>> so label_range is of 'widest_int' type but then instead of casting
>>> range_of_op to widest_irange you cast that widest_irange to the
>>> type of the range op?  Why not build label_range in the type
>>> of range_op immediately?
>>
>> The "widest" in widest_irange does not denote the type of the range, but
>> the number of sub-ranges that can maximally fit.  It has nothing to do
>> with the underlying type of its elements.  Instead, it is supposed to
>> indicate that label_range can fit an infinite amount of sub-ranges.  In
>> practice this is 255 sub-ranges, but we can adjust that if needed.
> 
> That's definitely confusing naming then :/

I've been mulling this over, and you're right.  The naming does imply 
some relation to widest_int.

Originally I batted some ideas on naming this, but came up short.  I'm 
still not sure what it should be.

max_irange?

biggest_irange?

Perhaps some name involving "int_range", since int_range<> is how ranges 
are declared (int_range_max??).

Suggestions welcome.
Aldy

On Thu, Sep 3, 2020 at 9:21 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>
>
>
> On 8/31/20 2:55 PM, Richard Biener wrote:
> > On Mon, Aug 31, 2020 at 11:10 AM Aldy Hernandez <aldyh@redhat.com> wrote:
> >>
> >>
> >>
> >> On 8/31/20 10:33 AM, Richard Biener wrote:
> >>> On Mon, Aug 31, 2020 at 10:20 AM Aldy Hernandez <aldyh@redhat.com> wrote:
> >>>>
> >>>> As discussed in the PR, the type of a switch operand may be different
> >>>> than the type of the individual cases.  This is causing us to attempt to
> >>>> intersect incompatible ranges.
> >>>>
> >>>> This inconsistent IL seems wrong to me, but it also seems pervasive
> >>>> throughout GCC.  Jason, as one of the original gimple designers, do you
> >>>> remember if this was an oversight, or was there a reason for this?
> >>>>
> >>>> Richard, you mention in the PR that normalizing the IL would cause
> >>>> propagation of widening cast sources into switches harder.  Couldn't we
> >>>> just cast all labels to the switch operand type upon creation?  What
> >>>> would be the problem with that?  Would we generate sub-optimal code?
> >>>>
> >>>> In either case, I'd hate to leave trunk in a broken case while this is
> >>>> being discussed.  The attached patch fixes the PRs.
> >>>>
> >>>> OK?
> >>>
> >>>          widest_irange label_range (CASE_LOW (min_label), case_high);
> >>> +      if (!types_compatible_p (label_range.type (), range_of_op->type ()))
> >>> +       range_cast (label_range, range_of_op->type ());
> >>>          label_range.intersect (range_of_op);
> >>>
> >>> so label_range is of 'widest_int' type but then instead of casting
> >>> range_of_op to widest_irange you cast that widest_irange to the
> >>> type of the range op?  Why not build label_range in the type
> >>> of range_op immediately?
> >>
> >> The "widest" in widest_irange does not denote the type of the range, but
> >> the number of sub-ranges that can maximally fit.  It has nothing to do
> >> with the underlying type of its elements.  Instead, it is supposed to
> >> indicate that label_range can fit an infinite amount of sub-ranges.  In
> >> practice this is 255 sub-ranges, but we can adjust that if needed.
> >
> > That's definitely confusing naming then :/
>
> I've been mulling this over, and you're right.  The naming does imply
> some relation to widest_int.
>
> Originally I batted some ideas on naming this, but came up short.  I'm
> still not sure what it should be.
>
> max_irange?
>
> biggest_irange?
>
> Perhaps some name involving "int_range", since int_range<> is how ranges
> are declared (int_range_max??).

The issue is that widest_irange has a connection to the type of the range
while it seems to constrain the number of subranges.  So if there's
a irange<1>, irange<2>, etc. then irange_max would be a typedef that
makes most sense (both <2> and _max are then suffixes).

Hmm, a simple grep reveals

value-range.h:typedef int_range<255> widest_irange;
value-range.h:class GTY((user)) int_range : public irange

so widest_irange is not an irange but an int_range?  But then
there's little use of 'irange' or 'int_range', most code uses
either irange & arguments (possibly accepting sth else than
int_range) and variables are widest_irange or irange3
(typedefed from int_range<3>).

IMHO a typedef irange* to int_range is confusing a bit.

Why isn't it int_range3 or widest_int_rage?  Why is there
irange and int_range?

Well, still stand with irange_max, or, preferably int_range_max.
Or ...

template<unsigned N = 255>
class GTY((user)) int_range : public irange
{

so people can use

int_range x;

and get the max range by default?

But back to the question of just renaming widest_irange.  Use irange_max.
The other confusion still stands of course.

Richard.

> Suggestions welcome.
> Aldy
>

On 9/3/20 3:43 AM, Richard Biener wrote:
> On Thu, Sep 3, 2020 at 9:21 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>>
>>
>> On 8/31/20 2:55 PM, Richard Biener wrote:
>>> On Mon, Aug 31, 2020 at 11:10 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>>>>
>>>>
>>>> On 8/31/20 10:33 AM, Richard Biener wrote:
>>>>> On Mon, Aug 31, 2020 at 10:20 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>>>>>> As discussed in the PR, the type of a switch operand may be different
>>>>>> than the type of the individual cases.  This is causing us to attempt to
>>>>>> intersect incompatible ranges.
>>>>>>
>>>>>> This inconsistent IL seems wrong to me, but it also seems pervasive
>>>>>> throughout GCC.  Jason, as one of the original gimple designers, do you
>>>>>> remember if this was an oversight, or was there a reason for this?
>>>>>>
>>>>>> Richard, you mention in the PR that normalizing the IL would cause
>>>>>> propagation of widening cast sources into switches harder.  Couldn't we
>>>>>> just cast all labels to the switch operand type upon creation?  What
>>>>>> would be the problem with that?  Would we generate sub-optimal code?
>>>>>>
>>>>>> In either case, I'd hate to leave trunk in a broken case while this is
>>>>>> being discussed.  The attached patch fixes the PRs.
>>>>>>
>>>>>> OK?
>>>>>           widest_irange label_range (CASE_LOW (min_label), case_high);
>>>>> +      if (!types_compatible_p (label_range.type (), range_of_op->type ()))
>>>>> +       range_cast (label_range, range_of_op->type ());
>>>>>           label_range.intersect (range_of_op);
>>>>>
>>>>> so label_range is of 'widest_int' type but then instead of casting
>>>>> range_of_op to widest_irange you cast that widest_irange to the
>>>>> type of the range op?  Why not build label_range in the type
>>>>> of range_op immediately?
>>>> The "widest" in widest_irange does not denote the type of the range, but
>>>> the number of sub-ranges that can maximally fit.  It has nothing to do
>>>> with the underlying type of its elements.  Instead, it is supposed to
>>>> indicate that label_range can fit an infinite amount of sub-ranges.  In
>>>> practice this is 255 sub-ranges, but we can adjust that if needed.
>>> That's definitely confusing naming then :/
>> I've been mulling this over, and you're right.  The naming does imply
>> some relation to widest_int.
>>
>> Originally I batted some ideas on naming this, but came up short.  I'm
>> still not sure what it should be.
>>
>> max_irange?
>>
>> biggest_irange?
>>
>> Perhaps some name involving "int_range", since int_range<> is how ranges
>> are declared (int_range_max??).
> The issue is that widest_irange has a connection to the type of the range
> while it seems to constrain the number of subranges.  So if there's
> a irange<1>, irange<2>, etc. then irange_max would be a typedef that
> makes most sense (both <2> and _max are then suffixes).
>
> Hmm, a simple grep reveals
>
> value-range.h:typedef int_range<255> widest_irange;
> value-range.h:class GTY((user)) int_range : public irange
>
> so widest_irange is not an irange but an int_range?  But then
> there's little use of 'irange' or 'int_range', most code uses
> either irange & arguments (possibly accepting sth else than
> int_range) and variables are widest_irange or irange3
> (typedefed from int_range<3>).
typedef int_range<3> irange3;

isnt a real thing.. its a typedef Aldy used in the range-ops self tests 
as shorthand.. I think its a leftover from a previous incarnation of 
irange.

>
> IMHO a typedef irange* to int_range is confusing a bit.
>
> Why isn't it int_range3 or widest_int_rage?  Why is there
> irange and int_range?

irange is the generic class that is used to operate on integral ranges.  
It implements all the range operations and provides the API. it is 
agnostic to the number of sub ranges.

int_range is the template that instantiates an irange object, and 
associates enough memory to represent the number of sub-ranges you want 
to work with.. so it is a bit more than a typedef, its a derived class 
which adds memory.

declaring a function
foo (int_range<3> &result, op1, op2)

wont allow you to pass an int_range<4> as the argument... it will take 
nothing but an int_range<3> as they are different types in c++... which 
is very limiting

foo (irange &result, op1, op2)

on the other hand allows you to pass any implementation/instantiation 
around you want. So we use irange everywhere we want to work with an 
generic integral range, and when you need to declare a variable to store 
a range, you use int_range<x>


>
> Well, still stand with irange_max, or, preferably int_range_max.
> Or ...
>
> template<unsigned N = 255>
> class GTY((user)) int_range : public irange
> {
>
> so people can use
>
> int_range x;
>
> and get the max range by default?
>
Indeed, I had forgotten about default template arguments, the only 
problem is

int_range x;

is valid in c++17, but previous to that we have to do

int_range<> x;

Its a bit ugly, and possibly a bit less natural, But it does show the 
intent.

So I think the 2 basic choices are:

int_range<> r;
int_range_max r;

I'm thinking 'int_range_max'?    but I really don't feel strongly one 
way or the other...   Eventually we'll probably end up with c++17 and we 
can revert to  'int_range' everywhere.
If we go with the _max suffix, then we should stick to the "int_range" 
pattern for consistency.. in hindsight, 'widest_irange' should have been 
'widest_int_range'.

Andrew

There is also a pool allocator coming which will give you a dynamically 
allocated irange object with just enough memory associated to represent 
a specified range object.
This will allow you to calculate a range using maximal sub-ranges, but 
then store the result away efficeiently using just the required number 
of sub-ranges.

On September 3, 2020 5:39:18 PM GMT+02:00, Andrew MacLeod <amacleod@redhat.com> wrote:
>On 9/3/20 3:43 AM, Richard Biener wrote:
>> On Thu, Sep 3, 2020 at 9:21 AM Aldy Hernandez <aldyh@redhat.com>
>wrote:
>>>
>>>
>>> On 8/31/20 2:55 PM, Richard Biener wrote:
>>>> On Mon, Aug 31, 2020 at 11:10 AM Aldy Hernandez <aldyh@redhat.com>
>wrote:
>>>>>
>>>>>
>>>>> On 8/31/20 10:33 AM, Richard Biener wrote:
>>>>>> On Mon, Aug 31, 2020 at 10:20 AM Aldy Hernandez
><aldyh@redhat.com> wrote:
>>>>>>> As discussed in the PR, the type of a switch operand may be
>different
>>>>>>> than the type of the individual cases.  This is causing us to
>attempt to
>>>>>>> intersect incompatible ranges.
>>>>>>>
>>>>>>> This inconsistent IL seems wrong to me, but it also seems
>pervasive
>>>>>>> throughout GCC.  Jason, as one of the original gimple designers,
>do you
>>>>>>> remember if this was an oversight, or was there a reason for
>this?
>>>>>>>
>>>>>>> Richard, you mention in the PR that normalizing the IL would
>cause
>>>>>>> propagation of widening cast sources into switches harder. 
>Couldn't we
>>>>>>> just cast all labels to the switch operand type upon creation? 
>What
>>>>>>> would be the problem with that?  Would we generate sub-optimal
>code?
>>>>>>>
>>>>>>> In either case, I'd hate to leave trunk in a broken case while
>this is
>>>>>>> being discussed.  The attached patch fixes the PRs.
>>>>>>>
>>>>>>> OK?
>>>>>>           widest_irange label_range (CASE_LOW (min_label),
>case_high);
>>>>>> +      if (!types_compatible_p (label_range.type (),
>range_of_op->type ()))
>>>>>> +       range_cast (label_range, range_of_op->type ());
>>>>>>           label_range.intersect (range_of_op);
>>>>>>
>>>>>> so label_range is of 'widest_int' type but then instead of
>casting
>>>>>> range_of_op to widest_irange you cast that widest_irange to the
>>>>>> type of the range op?  Why not build label_range in the type
>>>>>> of range_op immediately?
>>>>> The "widest" in widest_irange does not denote the type of the
>range, but
>>>>> the number of sub-ranges that can maximally fit.  It has nothing
>to do
>>>>> with the underlying type of its elements.  Instead, it is supposed
>to
>>>>> indicate that label_range can fit an infinite amount of
>sub-ranges.  In
>>>>> practice this is 255 sub-ranges, but we can adjust that if needed.
>>>> That's definitely confusing naming then :/
>>> I've been mulling this over, and you're right.  The naming does
>imply
>>> some relation to widest_int.
>>>
>>> Originally I batted some ideas on naming this, but came up short. 
>I'm
>>> still not sure what it should be.
>>>
>>> max_irange?
>>>
>>> biggest_irange?
>>>
>>> Perhaps some name involving "int_range", since int_range<> is how
>ranges
>>> are declared (int_range_max??).
>> The issue is that widest_irange has a connection to the type of the
>range
>> while it seems to constrain the number of subranges.  So if there's
>> a irange<1>, irange<2>, etc. then irange_max would be a typedef that
>> makes most sense (both <2> and _max are then suffixes).
>>
>> Hmm, a simple grep reveals
>>
>> value-range.h:typedef int_range<255> widest_irange;
>> value-range.h:class GTY((user)) int_range : public irange
>>
>> so widest_irange is not an irange but an int_range?  But then
>> there's little use of 'irange' or 'int_range', most code uses
>> either irange & arguments (possibly accepting sth else than
>> int_range) and variables are widest_irange or irange3
>> (typedefed from int_range<3>).
>typedef int_range<3> irange3;
>
>isnt a real thing.. its a typedef Aldy used in the range-ops self tests
>
>as shorthand.. I think its a leftover from a previous incarnation of 
>irange.
>
>>
>> IMHO a typedef irange* to int_range is confusing a bit.
>>
>> Why isn't it int_range3 or widest_int_rage?  Why is there
>> irange and int_range?
>
>irange is the generic class that is used to operate on integral
>ranges.  
>It implements all the range operations and provides the API. it is 
>agnostic to the number of sub ranges.
>
>int_range is the template that instantiates an irange object, and 
>associates enough memory to represent the number of sub-ranges you want
>
>to work with.. so it is a bit more than a typedef, its a derived class 
>which adds memory.
>
>declaring a function
>foo (int_range<3> &result, op1, op2)
>
>wont allow you to pass an int_range<4> as the argument... it will take 
>nothing but an int_range<3> as they are different types in c++... which
>
>is very limiting
>
>foo (irange &result, op1, op2)
>
>on the other hand allows you to pass any implementation/instantiation 
>around you want. So we use irange everywhere we want to work with an 
>generic integral range, and when you need to declare a variable to
>store 
>a range, you use int_range<x>
>
>
>>
>> Well, still stand with irange_max, or, preferably int_range_max.
>> Or ...
>>
>> template<unsigned N = 255>
>> class GTY((user)) int_range : public irange
>> {
>>
>> so people can use
>>
>> int_range x;
>>
>> and get the max range by default?
>>
>Indeed, I had forgotten about default template arguments, the only 
>problem is
>
>int_range x;
>
>is valid in c++17, but previous to that we have to do
>
>int_range<> x;
>
>Its a bit ugly, and possibly a bit less natural, But it does show the 
>intent.
>
>So I think the 2 basic choices are:
>
>int_range<> r;
>int_range_max r;
>
>I'm thinking 'int_range_max'?    but I really don't feel strongly one 
>way or the other...   Eventually we'll probably end up with c++17 and
>we 
>can revert to  'int_range' everywhere.
>If we go with the _max suffix, then we should stick to the "int_range" 
>pattern for consistency.. in hindsight, 'widest_irange' should have
>been 
>'widest_int_range'.

Int_range_max works for me. 

Richard. 

>Andrew
>
>There is also a pool allocator coming which will give you a dynamically
>
>allocated irange object with just enough memory associated to represent
>
>a specified range object.
>This will allow you to calculate a range using maximal sub-ranges, but 
>then store the result away efficeiently using just the required number 
>of sub-ranges.

>>> template<unsigned N = 255>
>>> class GTY((user)) int_range : public irange
>>> {
>>>
>>> so people can use
>>>
>>> int_range x;
>>>
>>> and get the max range by default?
>>>
>> Indeed, I had forgotten about default template arguments, the only
>> problem is
>>
>> int_range x;
>>
>> is valid in c++17, but previous to that we have to do
>>
>> int_range<> x;
>>
>> Its a bit ugly, and possibly a bit less natural, But it does show the
>> intent.

If we eventually go this route, I will note that we should decide 
whether we want to obscure the fact that an int_range<> or int_range is 
no longer a tiny object.

(gdb) p sizeof(int_range<1>)
$4 = 32
(gdb) p sizeof(int_range<2>)
$5 = 48
(gdb) p sizeof(widest_irange)
$6 = 4096

At least with int_range_max the intent is very clear.

However, it does seem like int_range_max is the more prevalent use 
except for small temporary ranges we know fit (a VARYING, or a small 
range with just two end-points).

>>
>> So I think the 2 basic choices are:
>>
>> int_range<> r;
>> int_range_max r;
>>
>> I'm thinking 'int_range_max'?    but I really don't feel strongly one
>> way or the other...   Eventually we'll probably end up with c++17 and
>> we
>> can revert to  'int_range' everywhere.
>> If we go with the _max suffix, then we should stick to the "int_range"
>> pattern for consistency.. in hindsight, 'widest_irange' should have
>> been
>> 'widest_int_range'.
>
> Int_range_max works for me.

Sounds good to me.  See attached patch.

I have also renamed the deceptive irange3 to int_range3.

Committed.

gcc/ChangeLog:

	* range-op.cc (range_operator::fold_range): Rename widest_irange
	to int_range_max.
	(operator_div::wi_fold): Same.
	(operator_lshift::op1_range): Same.
	(operator_rshift::op1_range): Same.
	(operator_cast::fold_range): Same.
	(operator_cast::op1_range): Same.
	(operator_bitwise_and::remove_impossible_ranges): Same.
	(operator_bitwise_and::op1_range): Same.
	(operator_abs::op1_range): Same.
	(range_cast): Same.
	(widest_irange_tests): Same.
	(range3_tests): Rename irange3 to int_range3.
	(int_range_max_tests): Rename from widest_irange_tests.
	Rename widest_irange to int_range_max.
	(operator_tests): Rename widest_irange to int_range_max.
	(range_tests): Same.
	* tree-vrp.c (find_case_label_range): Same.
	* value-range.cc (irange::irange_intersect): Same.
	(irange::invert): Same.
	* value-range.h: Same.
---
  gcc/range-op.cc    | 132 ++++++++++++++++++++++-----------------------
  gcc/tree-vrp.c     |   4 +-
  gcc/value-range.cc |   4 +-
  gcc/value-range.h  |   2 +-
  4 files changed, 71 insertions(+), 71 deletions(-)

diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index b7b708b488b..fbf78be0a3c 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -160,7 +160,7 @@ range_operator::fold_range (irange &r, tree type,
        return true;
      }

-  widest_irange tmp;
+  int_range_max tmp;
    r.set_undefined ();
    for (unsigned x = 0; x < num_lh; ++x)
      for (unsigned y = 0; y < num_rh; ++y)
@@ -1367,7 +1367,7 @@ operator_div::wi_fold (irange &r, tree type,
    // Then divide by the non-zero positive numbers, if any.
    if (wi::gt_p (divisor_max, wi::zero (prec), sign))
      {
-      widest_irange tmp;
+      int_range_max tmp;
        wi_cross_product (tmp, type, dividend_min, dividend_max,
  			wi::one (prec), divisor_max);
        r.union_ (tmp);
@@ -1567,7 +1567,7 @@ operator_lshift::op1_range (irange &r,
        //
        // Perform a logical rshift by doing the rshift as unsigned.
        tree unsigned_type = unsigned_type_for (type);
-      widest_irange unsigned_lhs = lhs;
+      int_range_max unsigned_lhs = lhs;
        range_cast (unsigned_lhs, unsigned_type);
        rshift_op = range_op_handler (RSHIFT_EXPR, unsigned_type);
        rshift_op->fold_range (lb, unsigned_type, unsigned_lhs, shifted);
@@ -1611,7 +1611,7 @@ operator_rshift::op1_range (irange &r,
      {
        // Folding the original operation may discard some impossible
        // ranges from the LHS.
-      widest_irange lhs_refined;
+      int_range_max lhs_refined;
        op_rshift.fold_range (lhs_refined, type, int_range<1> (type), op2);
        lhs_refined.intersect (lhs);
        if (lhs_refined.undefined_p ())
@@ -1619,8 +1619,8 @@ operator_rshift::op1_range (irange &r,
  	  r.set_undefined ();
  	  return true;
  	}
-      widest_irange shift_range (shift, shift);
-      widest_irange lb, ub;
+      int_range_max shift_range (shift, shift);
+      int_range_max lb, ub;
        op_lshift.fold_range (lb, type, lhs_refined, shift_range);
        //    LHS
        // 0000 0111 = OP1 >> 3
@@ -1632,7 +1632,7 @@ operator_rshift::op1_range (irange &r,
  			       fold_build2 (LSHIFT_EXPR, type,
  					    build_minus_one_cst (type),
  					    shift));
-      widest_irange mask_range (build_zero_cst (type), mask);
+      int_range_max mask_range (build_zero_cst (type), mask);
        op_plus.fold_range (ub, type, lb, mask_range);
        r = lb;
        r.union_ (ub);
@@ -1786,7 +1786,7 @@ operator_cast::fold_range (irange &r, tree type 
ATTRIBUTE_UNUSED,
    // Then process any additonal pairs by unioning with their results.
    for (unsigned x = 1; x < inner.num_pairs (); ++x)
      {
-      widest_irange tmp;
+      int_range_max tmp;
        fold_pair (tmp, x, inner, outer);
        r.union_ (tmp);
        if (r.varying_p ())
@@ -1811,7 +1811,7 @@ operator_cast::op1_range (irange &r, tree type,
          {
  	  // We want to insert the LHS as an unsigned value since it
  	  // would not trigger the signed bit of the larger type.
-	  widest_irange converted_lhs = lhs;
+	  int_range_max converted_lhs = lhs;
  	  range_cast (converted_lhs, unsigned_type_for (lhs_type));
  	  range_cast (converted_lhs, type);
  	  // Start by building the positive signed outer range for the type.
@@ -1826,14 +1826,14 @@ operator_cast::op1_range (irange &r, tree type,
  	  lim = wi::mask (TYPE_PRECISION (lhs_type), true,
  			  TYPE_PRECISION (type));
  	  // Add this to the unsigned LHS range(s).
-	  widest_irange lim_range (type, lim, lim);
-	  widest_irange lhs_neg;
+	  int_range_max lim_range (type, lim, lim);
+	  int_range_max lhs_neg;
  	  range_op_handler (PLUS_EXPR, type)->fold_range (lhs_neg,
  							  type,
  							  converted_lhs,
  							  lim_range);
  	  // And union this with the entire outer types negative range.
-	  widest_irange neg (type,
+	  int_range_max neg (type,
  			     wi::min_value (TYPE_PRECISION (type),
  					    SIGNED),
  			     lim - 1);
@@ -1846,7 +1846,7 @@ operator_cast::op1_range (irange &r, tree type,
        return true;
      }

-  widest_irange tmp;
+  int_range_max tmp;
    if (TYPE_PRECISION (lhs_type) == TYPE_PRECISION (type))
      tmp = lhs;
    else
@@ -1984,7 +1984,7 @@ operator_bitwise_and::remove_impossible_ranges 
(irange &r,
    tree type = r.type ();
    int prec = TYPE_PRECISION (type);
    int leading_zeros = wi::clz (mask);
-  widest_irange impossible_ranges;
+  int_range_max impossible_ranges;

    /* We know that starting at the most significant bit, any 0 in the
       mask means the resulting range cannot contain a 1 in that same
@@ -2315,10 +2315,10 @@ operator_bitwise_and::op1_range (irange &r, tree 
type,
    r.set_undefined ();
    for (unsigned i = 0; i < lhs.num_pairs (); ++i)
      {
-      widest_irange chunk (lhs.type (),
+      int_range_max chunk (lhs.type (),
  			   lhs.lower_bound (i),
  			   lhs.upper_bound (i));
-      widest_irange res;
+      int_range_max res;
        simple_op1_range_solver (res, type, chunk, op2);
        r.union_ (res);
      }
@@ -2872,7 +2872,7 @@ operator_abs::op1_range (irange &r, tree type,
        return true;
      }
    // Start with the positives because negatives are an impossible result.
-  widest_irange positives = range_positives (type);
+  int_range_max positives = range_positives (type);
    positives.intersect (lhs);
    r = positives;
    // Then add the negative of each pair:
@@ -3284,7 +3284,7 @@ range_op_handler (enum tree_code code, tree type)
  void
  range_cast (irange &r, tree type)
  {
-  widest_irange tmp = r;
+  int_range_max tmp = r;
    range_operator *op = range_op_handler (CONVERT_EXPR, type);
    // Call op_convert, if it fails, the result is varying.
    if (!op->fold_range (r, type, tmp, int_range<1> (type)))
@@ -3321,84 +3321,84 @@ build_range3 (int a, int b, int c, int d, int e, 
int f)
  static void
  range3_tests ()
  {
-  typedef int_range<3> irange3;
-  irange3 r0, r1, r2;
-  irange3 i1, i2, i3;
+  typedef int_range<3> int_range3;
+  int_range3 r0, r1, r2;
+  int_range3 i1, i2, i3;

    // ([10,20] U [5,8]) U [1,3] ==> [1,3][5,8][10,20].
-  r0 = irange3 (INT (10), INT (20));
-  r1 = irange3 (INT (5), INT (8));
+  r0 = int_range3 (INT (10), INT (20));
+  r1 = int_range3 (INT (5), INT (8));
    r0.union_ (r1);
-  r1 = irange3 (INT (1), INT (3));
+  r1 = int_range3 (INT (1), INT (3));
    r0.union_ (r1);
    ASSERT_TRUE (r0 == build_range3 (1, 3, 5, 8, 10, 20));

    // [1,3][5,8][10,20] U [-5,0] => [-5,3][5,8][10,20].
-  r1 = irange3 (INT (-5), INT (0));
+  r1 = int_range3 (INT (-5), INT (0));
    r0.union_ (r1);
    ASSERT_TRUE (r0 == build_range3 (-5, 3, 5, 8, 10, 20));

    // [10,20][30,40] U [50,60] ==> [10,20][30,40][50,60].
-  r1 = irange3 (INT (50), INT (60));
-  r0 = irange3 (INT (10), INT (20));
-  r0.union_ (irange3 (INT (30), INT (40)));
+  r1 = int_range3 (INT (50), INT (60));
+  r0 = int_range3 (INT (10), INT (20));
+  r0.union_ (int_range3 (INT (30), INT (40)));
    r0.union_ (r1);
    ASSERT_TRUE (r0 == build_range3 (10, 20, 30, 40, 50, 60));
    // [10,20][30,40][50,60] U [70, 80] ==> [10,20][30,40][50,60][70,80].
-  r1 = irange3 (INT (70), INT (80));
+  r1 = int_range3 (INT (70), INT (80));
    r0.union_ (r1);

    r2 = build_range3 (10, 20, 30, 40, 50, 60);
-  r2.union_ (irange3 (INT (70), INT (80)));
+  r2.union_ (int_range3 (INT (70), INT (80)));
    ASSERT_TRUE (r0 == r2);

    // [10,20][30,40][50,60] U [6,35] => [6,40][50,60].
    r0 = build_range3 (10, 20, 30, 40, 50, 60);
-  r1 = irange3 (INT (6), INT (35));
+  r1 = int_range3 (INT (6), INT (35));
    r0.union_ (r1);
-  r1 = irange3 (INT (6), INT (40));
-  r1.union_ (irange3 (INT (50), INT (60)));
+  r1 = int_range3 (INT (6), INT (40));
+  r1.union_ (int_range3 (INT (50), INT (60)));
    ASSERT_TRUE (r0 == r1);

    // [10,20][30,40][50,60] U [6,60] => [6,60].
    r0 = build_range3 (10, 20, 30, 40, 50, 60);
-  r1 = irange3 (INT (6), INT (60));
+  r1 = int_range3 (INT (6), INT (60));
    r0.union_ (r1);
-  ASSERT_TRUE (r0 == irange3 (INT (6), INT (60)));
+  ASSERT_TRUE (r0 == int_range3 (INT (6), INT (60)));

    // [10,20][30,40][50,60] U [6,70] => [6,70].
    r0 = build_range3 (10, 20, 30, 40, 50, 60);
-  r1 = irange3 (INT (6), INT (70));
+  r1 = int_range3 (INT (6), INT (70));
    r0.union_ (r1);
-  ASSERT_TRUE (r0 == irange3 (INT (6), INT (70)));
+  ASSERT_TRUE (r0 == int_range3 (INT (6), INT (70)));

    // [10,20][30,40][50,60] U [35,70] => [10,20][30,70].
    r0 = build_range3 (10, 20, 30, 40, 50, 60);
-  r1 = irange3 (INT (35), INT (70));
+  r1 = int_range3 (INT (35), INT (70));
    r0.union_ (r1);
-  r1 = irange3 (INT (10), INT (20));
-  r1.union_ (irange3 (INT (30), INT (70)));
+  r1 = int_range3 (INT (10), INT (20));
+  r1.union_ (int_range3 (INT (30), INT (70)));
    ASSERT_TRUE (r0 == r1);

    // [10,20][30,40][50,60] U [15,35] => [10,40][50,60].
    r0 = build_range3 (10, 20, 30, 40, 50, 60);
-  r1 = irange3 (INT (15), INT (35));
+  r1 = int_range3 (INT (15), INT (35));
    r0.union_ (r1);
-  r1 = irange3 (INT (10), INT (40));
-  r1.union_ (irange3 (INT (50), INT (60)));
+  r1 = int_range3 (INT (10), INT (40));
+  r1.union_ (int_range3 (INT (50), INT (60)));
    ASSERT_TRUE (r0 == r1);

    // [10,20][30,40][50,60] U [35,35] => [10,20][30,40][50,60].
    r0 = build_range3 (10, 20, 30, 40, 50, 60);
-  r1 = irange3 (INT (35), INT (35));
+  r1 = int_range3 (INT (35), INT (35));
    r0.union_ (r1);
    ASSERT_TRUE (r0 == build_range3 (10, 20, 30, 40, 50, 60));
  }

  static void
-widest_irange_tests ()
+int_range_max_tests ()
  {
-  widest_irange big;
+  int_range_max big;
    unsigned int nrange;

    // Build a huge multi-range range.
@@ -3410,7 +3410,7 @@ widest_irange_tests ()
    ASSERT_TRUE (big.num_pairs () == nrange);

    // Verify that we can copy it without loosing precision.
-  widest_irange copy (big);
+  int_range_max copy (big);
    ASSERT_TRUE (copy.num_pairs () == nrange);

    // Inverting it should produce one more sub-range.
@@ -3423,9 +3423,9 @@ widest_irange_tests ()

    // Test that [10,10][20,20] does NOT contain 15.
    {
-    widest_irange i1 (build_int_cst (integer_type_node, 10),
+    int_range_max i1 (build_int_cst (integer_type_node, 10),
  		      build_int_cst (integer_type_node, 10));
-    widest_irange i2 (build_int_cst (integer_type_node, 20),
+    int_range_max i2 (build_int_cst (integer_type_node, 20),
  		      build_int_cst (integer_type_node, 20));
      i1.union_ (i2);
      ASSERT_FALSE (i1.contains_p (build_int_cst (integer_type_node, 15)));
@@ -3463,9 +3463,9 @@ operator_tests ()
    tree max = vrp_val_max (integer_type_node);
    tree tiny = fold_build2 (PLUS_EXPR, integer_type_node, min,
  			   build_one_cst (integer_type_node));
-  widest_irange res;
-  widest_irange i1 (tiny, max);
-  widest_irange i2 (build_int_cst (integer_type_node, 255),
+  int_range_max res;
+  int_range_max i1 (tiny, max);
+  int_range_max i2 (build_int_cst (integer_type_node, 255),
  		    build_int_cst (integer_type_node, 255));

    // [MIN+1, MAX] = OP1 & 255: OP1 is VARYING
@@ -3497,20 +3497,20 @@ operator_tests ()

    // unsigned: [3, MAX] = OP1 >> 1
    {
-    widest_irange lhs (build_int_cst (unsigned_type_node, 3),
+    int_range_max lhs (build_int_cst (unsigned_type_node, 3),
  		       TYPE_MAX_VALUE (unsigned_type_node));
-    widest_irange one (build_one_cst (unsigned_type_node),
+    int_range_max one (build_one_cst (unsigned_type_node),
  		       build_one_cst (unsigned_type_node));
-    widest_irange op1;
+    int_range_max op1;
      op_rshift.op1_range (op1, unsigned_type_node, lhs, one);
      ASSERT_FALSE (op1.contains_p (UINT (3)));
    }

    // signed: [3, MAX] = OP1 >> 1
    {
-    widest_irange lhs (INT (3), TYPE_MAX_VALUE (integer_type_node));
-    widest_irange one (INT (1), INT (1));
-    widest_irange op1;
+    int_range_max lhs (INT (3), TYPE_MAX_VALUE (integer_type_node));
+    int_range_max one (INT (1), INT (1));
+    int_range_max op1;
      op_rshift.op1_range (op1, integer_type_node, lhs, one);
      ASSERT_FALSE (op1.contains_p (INT (-2)));
    }
@@ -3518,10 +3518,10 @@ operator_tests ()
    // This is impossible, so OP1 should be [].
    // signed: [MIN, MIN] = OP1 >> 1
    {
-    widest_irange lhs (TYPE_MIN_VALUE (integer_type_node),
+    int_range_max lhs (TYPE_MIN_VALUE (integer_type_node),
  		       TYPE_MIN_VALUE (integer_type_node));
-    widest_irange one (INT (1), INT (1));
-    widest_irange op1;
+    int_range_max one (INT (1), INT (1));
+    int_range_max op1;
      op_rshift.op1_range (op1, integer_type_node, lhs, one);
      ASSERT_TRUE (op1.undefined_p ());
    }
@@ -3529,11 +3529,11 @@ operator_tests ()
    // signed: ~[-1] = OP1 >> 31
    if (TYPE_PRECISION (integer_type_node) > 31)
      {
-      widest_irange lhs (INT (-1), INT (-1), VR_ANTI_RANGE);
-      widest_irange shift (INT (31), INT (31));
-      widest_irange op1;
+      int_range_max lhs (INT (-1), INT (-1), VR_ANTI_RANGE);
+      int_range_max shift (INT (31), INT (31));
+      int_range_max op1;
        op_rshift.op1_range (op1, integer_type_node, lhs, shift);
-      widest_irange negatives = range_negatives (integer_type_node);
+      int_range_max negatives = range_negatives (integer_type_node);
        negatives.intersect (op1);
        ASSERT_TRUE (negatives.undefined_p ());
      }
@@ -3823,7 +3823,7 @@ range_tests ()
    ASSERT_TRUE (r0.nonzero_p ());

    multi_precision_range_tests ();
-  widest_irange_tests ();
+  int_range_max_tests ();
    operator_tests ();
  }

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index a165164bb40..f7b0692a407 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -3827,7 +3827,7 @@ find_case_label_range (gswitch *switch_stmt, const 
irange *range_of_op)
        tree label = gimple_switch_label (switch_stmt, i);
        tree case_high
  	= CASE_HIGH (label) ? CASE_HIGH (label) : CASE_LOW (label);
-      widest_irange label_range (CASE_LOW (label), case_high);
+      int_range_max label_range (CASE_LOW (label), case_high);
        label_range.intersect (range_of_op);
        if (label_range == *range_of_op)
  	return label;
@@ -3849,7 +3849,7 @@ find_case_label_range (gswitch *switch_stmt, const 
irange *range_of_op)
        tree case_high = CASE_HIGH (max_label);
        if (!case_high)
  	case_high = CASE_LOW (max_label);
-      widest_irange label_range (CASE_LOW (min_label), case_high);
+      int_range_max label_range (CASE_LOW (min_label), case_high);
        if (!types_compatible_p (label_range.type (), range_of_op->type ()))
  	range_cast (label_range, range_of_op->type ());
        label_range.intersect (range_of_op);
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 93164b7e2e2..20aa4f114c9 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -1690,7 +1690,7 @@ irange::irange_intersect (const irange &r)
    signop sign = TYPE_SIGN (TREE_TYPE(m_base[0]));
    unsigned bld_pair = 0;
    unsigned bld_lim = m_max_ranges;
-  widest_irange r2 (*this);
+  int_range_max r2 (*this);
    unsigned r2_lim = r2.num_pairs ();
    unsigned i2 = 0;
    for (unsigned i = 0; i < r.num_pairs (); )
@@ -1827,7 +1827,7 @@ irange::invert ()
    unsigned i = 0;
    wi::overflow_type ovf;
    // Construct leftmost range.
-  widest_irange orig_range (*this);
+  int_range_max orig_range (*this);
    unsigned nitems = 0;
    wide_int tmp;
    // If this is going to underflow on the MINUS 1, don't even bother
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 1ab39939703..8497791c7b3 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -174,7 +174,7 @@ typedef int_range<1> value_range;

  // This is an "infinite" precision irange for use in temporary
  // calculations.
-typedef int_range<255> widest_irange;
+typedef int_range<255> int_range_max;

  // Returns true for an old-school value_range as described above.
  inline bool