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:
> >>>>>>> GCC supports targets without 'casesi'/'tablejump' instructions.  (For
> >>>>>>> example, '--target=amdgcn-amdhsa', '--target=nvptx-none' -- could
> >>>>>>> probably be implemented, but currently isn't.)  For x86_64, this can
> >>>>>>> be
> >>>>>>> "faked" by modifying 'gcc/config/i386/i386.md':
> >>>>>>>
> >>>>>>>       (define_expand "tablejump"
> >>>>>>>         [(parallel [(set (pc) (match_operand 0
> >>>>>>>         "indirect_branch_operand"))
> >>>>>>>                    (use (label_ref (match_operand 1)))])]
> >>>>>>>      -  ""
> >>>>>>>      +  "false"
> >>>>>>>
> >>>>>>>       (define_insn "*tablejump_1"
> >>>>>>>         [(set (pc) (match_operand:W 0 "indirect_branch_operand"
> >>>>>>>         "rBw"))
> >>>>>>>          (use (label_ref (match_operand 1)))]
> >>>>>>>      -  ""
> >>>>>>>      +  "false"
> >>>>>>>
> >>>>>>> For such GCC configurations, we've recently acquired a number of
> >>>>>>> GCC/Fortran regressions à la:
> >>>>>>>
> >>>>>>>      during RTL pass: expand
> >>>>>>>      
> >>>>>>> [...]/source-gcc/gcc/testsuite/gfortran.dg/assumed_rank_bounds_3.f90:178:38:
> >>>>>>>      internal compiler error: in emit_case_dispatch_table, at
> >>>>>>>      stmt.cc:1199
> >>>>>>>      0x1f2d35d internal_error(char const*, ...)
> >>>>>>>              [...]/source-gcc/gcc/diagnostic-global-context.cc:787
> >>>>>>>      0x8d368b fancy_abort(char const*, int, char const*)
> >>>>>>>              [...]/source-gcc/gcc/diagnostics/context.cc:1813
> >>>>>>>      0x773983 emit_case_dispatch_table
> >>>>>>>              [...]/source-gcc/gcc/stmt.cc:1199
> >>>>>>>      0x1078752 expand_case(gswitch*)
> >>>>>>>              [...]/source-gcc/gcc/stmt.cc:1359
> >>>>>>>
> >>>>>>> That's 'gcc/cfgexpand.cc:expand_gimple_stmt_1', 'case GIMPLE_SWITCH:'
> >>>>>>> calling 'gcc/stmt.cc:expand_case', which calls
> >>>>>>> 'gcc/stmt.cc:emit_case_dispatch_table', which does:
> >>>>>>>
> >>>>>>>      [...] try "casesi".  If that
> >>>>>>>      fails, try "tablejump".   A target *must* have one of them (or
> >>>>>>>      both).
> >>>>>>>
> >>>>>>> That means, for targets providing neither 'casesi' nor 'tablejump',
> >>>>>>> there
> >>>>>>> must not be any 'GIMPLE_SWITCH'es anymore, when getting to
> >>>>>>> 'gcc/cfgexpand.cc:expand_gimple_stmt_1' -- and usually there aren't,
> >>>>>>> due
> >>>>>>> to 'gcc/tree-switch-conversion.cc' doing what is appropriate; in
> >>>>>>> particular, 'gcc/tree-switch-conversion.h':
> >>>>>>>
> >>>>>>>      /* Return whether jump table expansion is allowed.  */
> >>>>>>>      bool jump_table_cluster::is_enabled (void)
> >>>>>>>      {
> >>>>>>>        /* If neither casesi or tablejump is available, or
> >>>>>>>        flag_jump_tables
> >>>>>>>           over-ruled us, we really have no choice.  */
> >>>>>>>        if (!targetm.have_casesi () && !targetm.have_tablejump ())
> >>>>>>>          return false;
> >>>>>>>      [...]
> >>>>>>>
> >>>>>>> ... deciding whether to lower 'GIMPLE_SWITCH'es into other control
> >>>>>>> flow
> >>>>>>> constructs supported by the target.
> >>>>>>>
> >>>>>>> Enter recent commit r17-2216-g3a8d9347f30b9d66ed6a3c7e0959c08e93ecb205
> >>>>>>> "fortran: Create a dedicated type for ranks and array dimensions",
> >>>>>>> which:
> >>>>>>>
> >>>>>>> | [...] adds a type to represent ranks and array dimension, using the
> >>>>>>> | same base type as originally used for the rank in array descriptors
> >>>>>>> | (signed char), but with the stricter bounds (0 to
> >>>>>>> | GFC_MAX_DIMENSIONS)
> >>>>>>> | brought to the knowledge of the middle-end.  [...]
> >>>>>>>
> >>>>>>>      --- 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?

> > 
> > Richard.
> > 
> >>>>>>>      +                                    build_zero_cst
> >>>>>>>      (signed_char_type_node),
> >>>>>>>      +                                    build_int_cst
> >>>>>>>      (signed_char_type_node,
> >>>>>>>      +
> >>>>>>>      GFC_MAX_DIMENSIONS));
> >>>>>>>      [...]
> >>>>>>>      --- a/gcc/fortran/trans-types.h
> >>>>>>>      +++ b/gcc/fortran/trans-types.h
> >>>>>>>      [...]
> >>>>>>>      +/* An integral type with bounds [0, GFC_MAX_DIMENSIONS] suitable
> >>>>>>>      to hold an
> >>>>>>>      +   array rank, or an array dimension index.  */
> >>>>>>>      +extern GTY(()) tree gfc_array_dim_rank_type;
> >>>>>>>      [...]
> >>>>>>>
> >>>>>>> ..., that is, a "subrange wrapper"; 'gcc/tree.cc':
> >>>>>>>
> >>>>>>>      /* Wrapper around build_range_type_1 with SHARED set to true.  */
> >>>>>>>
> >>>>>>>      tree
> >>>>>>>      build_range_type (tree type, tree lowval, tree highval)
> >>>>>>>      {
> >>>>>>>        return build_range_type_1 (type, lowval, highval, true);
> >>>>>>>      }
> >>>>>>>
> >>>>>>>      /* Create a range of some discrete type TYPE (an INTEGER_TYPE,
> >>>>>>>      ENUMERAL_TYPE
> >>>>>>>         or BOOLEAN_TYPE) with low bound LOWVAL and high bound HIGHVAL.
> >>>>>>>         If SHARED
> >>>>>>>         is true, reuse such a type that has already been constructed.
> >>>>>>>         */
> >>>>>>>
> >>>>>>>      static tree
> >>>>>>>      build_range_type_1 (tree type, tree lowval, tree highval, bool
> >>>>>>>      shared)
> >>>>>>>      {
> >>>>>>>      [...]
> >>>>>>>
> >>>>>>> That one has its own 'TREE_TYPE' pointing to 'signed_char_type_node',
> >>>>>>> and
> >>>>>>> itself has 'precision:8', but 'min <[...] 0> max <[...] 15>', that is,
> >>>>>>> a
> >>>>>>> restricted range compared to 'min <[...] -128> max <[...] 127>'.
> >>>>>>> Nothing
> >>>>>>> bad with that, as far as I can tell, and supposedly that enables
> >>>>>>> certain
> >>>>>>> code optimizations, due to the more restricted range.
> >>>>>>>
> >>>>>>> Now, 'gcc/tree-switch-conversion.cc':
> >>>>>>>
> >>>>>>>      /* Attempt to expand CLUSTERS as a decision tree.  Return true
> >>>>>>>      when
> >>>>>>>         expanded.  */
> >>>>>>>
> >>>>>>>      bool
> >>>>>>>      switch_decision_tree::try_switch_expansion (vec<cluster *>
> >>>>>>>      &clusters)
> >>>>>>>      {
> >>>>>>>        tree index_expr = gimple_switch_index (m_switch);
> >>>>>>>        tree index_type = TREE_TYPE (index_expr);
> >>>>>>>        basic_block bb = gimple_bb (m_switch);
> >>>>>>>
> >>>>>>>        if (gimple_switch_num_labels (m_switch) == 1
> >>>>>>>            || range_check_type (index_type) == NULL_TREE)
> >>>>>>>          return false;
> >>>>>>>      [...]
> >>>>>>>
> >>>>>>> ... calls 'gcc/fold-const.cc:range_check_type':
> >>>>>>>
> >>>>>>>      /* Helper routine for build_range_check and match.pd.  Return the
> >>>>>>>      type to
> >>>>>>>         perform the check or NULL if it shouldn't be optimized.  */
> >>>>>>>
> >>>>>>>      tree
> >>>>>>>      range_check_type (tree etype)
> >>>>>>>      {
> >>>>>>>        /* First make sure that arithmetics in this type is valid, then
> >>>>>>>        make sure
> >>>>>>>           that it wraps around.  */
> >>>>>>>        [...]
> >>>>>>>        if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_UNSIGNED
> >>>>>>>        (etype))
> >>>>>>>          {
> >>>>>>>            tree utype, minv, maxv;
> >>>>>>>
> >>>>>>>            /* Check if (unsigned) INT_MAX + 1 == (unsigned) INT_MIN
> >>>>>>>               for the type in question, as we rely on this here.  */
> >>>>>>>            utype = unsigned_type_for (etype);
> >>>>>>>            maxv = fold_convert (utype, TYPE_MAX_VALUE (etype));
> >>>>>>>            maxv = range_binop (PLUS_EXPR, NULL_TREE, maxv, 1,
> >>>>>>>                                build_int_cst (TREE_TYPE (maxv), 1),
> >>>>>>>            1);
> >>>>>>>            minv = fold_convert (utype, TYPE_MIN_VALUE (etype));
> >>>>>>>
> >>>>>>>            if (integer_zerop (range_binop (NE_EXPR, integer_type_node,
> >>>>>>>                                            minv, 1, maxv, 1)))
> >>>>>>>              etype = utype;
> >>>>>>>            else
> >>>>>>>              return NULL_TREE;
> >>>>>>>          }
> >>>>>>>      [...]
> >>>>>>>
> >>>>>>> Here we realize that 'TYPE_MAX_VALUE (gfc_array_dim_rank_type)' ('15')
> >>>>>>> does *not* wrap around to 'TYPE_MIN_VALUE (gfc_array_dim_rank_type)'
> >>>>>>> ('0') when adding '1' (contrary to '127 + 1 -> -128', for example),
> >>>>>>> and
> >>>>>>> therefore we 'return NULL_TREE;', and therefore block the
> >>>>>>> 'GIMPLE_SWITCH'
> >>>>>>> lowering.
> >>>>>>>
> >>>>>>> Per my understanding, that problem has been latent, just now exposed
> >>>>>>> via
> >>>>>>> this GCC/Fortran front end change.
> >>>>>>>
> >>>>>>> Resolve this by peeling off 'INTEGER_TYPE' subrange wrappers, similar
> >>>>>>> to
> >>>>>>> how that's already being done for 'ENUMERAL_TYPE's, 'BOOLEAN_TYPE's.
> >>>>>>> With that, the regressions for GCN and nvptx (as well as "faked"
> >>>>>>> x86_64)
> >>>>>>> disappear, and there's no other change in test results, including
> >>>>>>> x86_64-pc-linux-gnu as well as powerpc64le-unknown-linux-gnu
> >>>>>>> bootstrap.
> >>>>>>>
> >>>>>>>  PR tree-optimization/126392
> >>>>>>>  gcc/
> >>>>>>>  * fold-const.cc (range_check_type): Peel off subrange wrappers.
> >>>>>>> ---
> >>>>>>>   gcc/fold-const.cc | 4 ++++
> >>>>>>>   1 file changed, 4 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
> >>>>>>> index 1764942f34a..5eb25b547fe 100644
> >>>>>>> --- a/gcc/fold-const.cc
> >>>>>>> +++ b/gcc/fold-const.cc
> >>>>>>> @@ -5544,6 +5544,10 @@ range_check_type (tree etype)
> >>>>>>>       etype = TREE_TYPE (etype);
> >>>>>>>     else if (TREE_CODE (etype) == ENUMERAL_TYPE || TREE_CODE (etype)
> >>>>>>>     == BOOLEAN_TYPE)
> >>>>>>>       etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype),
> >>>>>>> 1);
> >>>>>>> +  else if (TREE_CODE (etype) == INTEGER_TYPE
> >>>>>>> +        && TREE_TYPE (etype))
> >>>>>>> +    /* Peel off subrange wrappers ('gcc/tree.cc:build_range_type_1').
> >>>>>>> */
> >>>>>>> +    etype = TREE_TYPE (etype);
> >>>>>>
> >>>>>> In principle reasonable, but I wonder whether we can rely on this
> >>>>>> being useful for the purpose.  TREE_TYPE on INTEGER_TYPE isn't
> >>>>>> documented
> >>>>>
> >>>>> Quite a number of front end as well as generic GCC code calls
> >>>>> 'build_range_type', 'build_nonshared_range_type', so this should become
> >>>>> documented, I infer?
> >>>>>
> >>>>>>   but IIRC it is indeed set when the INTEGER_TYPE is a
> >>>>>> "subtype" (but I don't think it has to).
> >>>>>
> >>>>> When you say "don't think it has to", is your worry that
> >>>>> 'gcc/tree.cc:build_range_type_1' might not return a type with
> >>>>> 'TREE_TYPE'
> >>>>> in the 'type_hash_canon' code path (I shall run a test with a
> >>>>> corresponding 'gcc_assert' added), or is it something else?
> >>>>>
> >>>>> Or, is your worry that other types may have 'TREE_TYPE' set, without
> >>>>> being such "subrange wrappers"?  Then we should disambiguate that?
> >>>>>
> >>>>>> I'll note that using this type does not get you a wrapping type
> >>>>>> in the original range either, so I'm not sure what the condition
> >>>>>> above is testing.
> >>>>>
> >>>>> I don't understand "what the condition above is testing", please
> >>>>> clarify?
> >>>>> With: 'TREE_CODE (etype) == INTEGER_TYPE && TREE_TYPE (etype)' I intend
> >>>>> to match (only) such "subrange wrappers", mirroring what
> >>>>> 'gcc/tree.cc:build_range_type_1' is doing.
> >>>>>
> >>>>> Indeed the base type ('TREE_TYPE (etype)') need not be 'unsigned'.  For
> >>>>> example, like in my example in the Git commit log cited above, where it
> >>>>> was 'signed char'.  But note that we don't just return that base type,
> >>>>> but instead assign it to 'etype' and then continue 'range_check_type'
> >>>>> analysis with that one.  That is, in particular the "make sure that it
> >>>>> wraps around" code gets executed:
> >>>>>
> >>>>>      if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_UNSIGNED (etype))
> >>>>>        {
> >>>>>          [...]
> >>>>>          utype = unsigned_type_for (etype);
> >>>>>          [...]
> >>>>>
> >>>>> ..., which then either returns this 'utype', or 'NULL'.
> >>>>>
> >>>>>> So you are simply by-passing the check, fixing the ICE but either
> >>>>>> showing the check is pointless or simply ignoring the issue when
> >>>>>> it doesn't pass.
> >>>>>
> >>>>> Sorry, again I don't understand what exactly this commen applies to.
> >>>>>
> >>>>> Is this about the 'range_check_type' usage in
> >>>>> 'gcc/tree-switch-conversion.cc:switch_decision_tree::try_switch_expansion'?
> >>>>> Indeed that one uses the 'range_check_type' return value only as a
> >>>>> boolean flag (appropriate types exists: continue vs. doesn't exist:
> >>>>> 'return false;'), but per my understanding the actual 'GIMPLE_SWITCH'
> >>>>> transformation code then individually agian calls 'range_check_type'
> >>>>> where necessary.
> >>>>>
> >>>>>
> >>>>> But please let me know if the underlying problem should be solved in
> >>>>> another way.  I'm certainly not an expert in 'GIMPLE_SWITCH'es, and
> >>>>> neither do I know all the details about GCC's type system.
> >>>>>
> >>>>> I came up with this patch by tracing down what was happening, and
> >>>>> noticing that 'range_check_type' already does similar fix-up for
> >>>>> 'ENUMERAL_TYPE's, 'BOOLEAN_TYPE's, so why not similarly also for other
> >>>>> "subrange" types.
> >>>>
> >>>> I wonder why range_check_type jumps through hoops here:
> >>>>
> >>>>    if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_UNSIGNED (etype))
> >>>>      {
> >>>>        tree utype, minv, maxv;
> >>>>
> >>>>        /* Check if (unsigned) INT_MAX + 1 == (unsigned) INT_MIN
> >>>>           for the type in question, as we rely on this here.  */
> >>>>        utype = unsigned_type_for (etype);
> >>>>        maxv = fold_convert (utype, TYPE_MAX_VALUE (etype));
> >>>>        maxv = range_binop (PLUS_EXPR, NULL_TREE, maxv, 1,
> >>>>                            build_int_cst (TREE_TYPE (maxv), 1), 1);
> >>>>        minv = fold_convert (utype, TYPE_MIN_VALUE (etype));
> >>>>
> >>>>        if (integer_zerop (range_binop (NE_EXPR, integer_type_node,
> >>>>                                        minv, 1, maxv, 1)))
> >>>>          etype = utype;
> >>>>        else
> >>>>          return NULL_TREE;
> >>>>      }
> >>>> ...
> >>>>    return etype;
> >>>>
> >>>> when, for an unsigned subrange type it does not bother to verify
> >>>> anything and when it practically ignores subrange types from
> >>>> ENUMERAL or BOOLEAN types by only looking at their "base" types?
> >>>>
> >>>> Why does it not just always return unsigned_type_for (etype)?!
> >>>>
> >>>> That is, what is the wrong thing it thinks might do that this
> >>>> guards?  And why isn't that an issue for enumeral or booleans
> >>>> or enumeral bitint types or for the case you are adding?
> >>>
> >>> That said, if the range type were [-2, 7] based on 'signed char'
> >>> then we might eventually want to convert a [-2, 2] range check
> >>> to an unsigned compare of (unsigned)val + 2 < 4.  I'm not sure
> >>> how this breaks when 7 + 1 isn't -2 and why it's OK to check
> >>> on the TREE_TYPE of the type when the switch () and the cases
> >>> are based on the range type (IIRC the case values never use
> >>> the range type but the underlying type).
> >>>
> >>> But I for sure must be missing something.
> >>
> >> This was added by r0-59059-ge1af8299421eb7 by jakub btw.
> >>
> >>> Richard.
> >>>
> >>>> Richard.
> >>>>
> >>>>>
> >>>>>
> >>>>> Grüße
> >>>>>   Thomas
> >>>>>
> >>>>
> >>>>
> >>>
> >>>
> >>
> >>
> > 
> 
> 

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