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. Grüße Thomas
