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