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??).

Suggestions welcome.
Aldy

Reply via email to