On Wed, 29 Jul 2026, Mikael Morin wrote:

> Le 29/07/2026 à 08:47, Richard Biener a écrit :
> > On Tue, 28 Jul 2026, Mikael Morin wrote:
> > 
> >> Le 28/07/2026 à 17:22, Richard Biener a écrit :
> >>> On Tue, 28 Jul 2026, Richard Biener wrote:
> >>>
> >>>> On Tue, 28 Jul 2026, Richard Biener wrote:
> >>>>
> >>>>> On Tue, 28 Jul 2026, Richard Biener wrote:
> >>>>>
> >>>>>> On Tue, 28 Jul 2026, Thomas Schwinge wrote:
> >>>>>>
> >>>>>>> Hi Richard!
> >>>>>>>
> >>>>>>> Thanks for looking into this!  Regarding your review:
> >>>>>>>
> >>>>>>> On 2026-07-28T08:11:46+0200, Richard Biener <[email protected]> wrote:
> >>>>>>>> On Fri, 24 Jul 2026, Thomas Schwinge wrote:
> >>>>>>>>>
> >>>>>>>>>       --- a/gcc/fortran/trans-types.cc
> >>>>>>>>>       +++ b/gcc/fortran/trans-types.cc
> >>>>>>>>>       [...]
> >>>>>>>>>       +tree gfc_array_dim_rank_type;
> >>>>>>>>>       [...]
> >>>>>>>>>       @@ -1226,6 +1227,12 @@ gfc_init_types (void)
> >>>>>>>>>          gfc_charlen_int_kind = get_int_kind_from_node
> >>>>>>>>>          (size_type_node);
> >>>>>>>>>          gfc_charlen_type_node = gfc_get_int_type
> >>>>>>>>>          (gfc_charlen_int_kind);
> >>>>>>>>>
> >>>>>>>>>       +  gfc_array_dim_rank_type
> >>>>>>>>>       +                = build_range_type (signed_char_type_node,
> >>>
> >>> And basing this on unsigned_char_type_node might be an easier fix?
> >>
> >> But unsigned types have the same problem, don't they?
> >> Say if the type is [20, 30] and we are trying to check that a value v is
> >> within say [21, 24], a transformation to check that v - 21 is within [0, 3]
> >> can't use the original type because the values are no longer in the range
> >> of
> >> the original type.
> > 
> > Sure, but we're using an unsigned type of the original types precision
> > anyway.  And in the gfortran case the low bound is zero, so we
> > have [0, MAX_DIMENSIONS], meaning an unsigned char base type sounds
> > appropriate?
> 
> Well, I very much prefer the undefined overflow semantics of signed types over
> the modulus semantics of unsigned.  But if you can confirm that signedness
> doesn't matter ABI-wise, unsigned char can surely be used here.
> 
> But I don't get how it is a fix.  It probably avoids the regression, but a
> latent problem remains. For the "fix" to be complete, the [0,MAX_DIMENSION]
> (unsigned) type has to be extended to a full unsigned char for the range check
> transformation, just like Thomas' original patch did.

Yes, IMO range_check_type should _always_ return unsigned_type_for (...).
It does so for unconstrained signed integer types and select others
only.  That it does not for unsigned range types looks like a bug.

Given that switch conversion doesn't use range_binop and friends it
should probably stop using range_check_type and instead use
unsigned_type_for (...), switch indices are sufficiently constrained
to scalar integral types already to make that work I think.
Note that generate_range_test would need to be changed as well
(it does not even handle range_check_type to return NULL...),

Richard.

-- 
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)

Reply via email to