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