On 9/3/20 3:43 AM, Richard Biener wrote:
On Thu, Sep 3, 2020 at 9:21 AM Aldy Hernandez <al...@redhat.com> wrote:
On 8/31/20 2:55 PM, Richard Biener wrote:
On Mon, Aug 31, 2020 at 11:10 AM Aldy Hernandez <al...@redhat.com> wrote:
On 8/31/20 10:33 AM, Richard Biener wrote:
On Mon, Aug 31, 2020 at 10:20 AM Aldy Hernandez <al...@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.
