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, > > > >> + 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)
