On Mon, 24 Nov 2025, Jakub Jelinek wrote: > On Mon, Nov 24, 2025 at 09:28:53AM +0100, Richard Biener wrote: > > I think INTEGER_TYPEs should be handled the same. > > Ok, here is an updated patch (so far untested) to handle > INTEGER_TYPEs the same. Even for the standard INTEGER_TYPEs > it will have an advantage that we just remember the TYPE_ALIAS_SET > on the unsigned ones, so while the first call of get_alias_set > on a particular INTEGER_TYPE will be slightly slower due to the > c_common_*signed_type call, we won't call on the other side > get_alias_set again and again. > > BTW, I don't really understand the > /* t1 == t can happen for boolean nodes which are always unsigned. */ > comment, boolean types are not INTEGER_TYPEs nor BITINT_TYPES, and > furthermore, I think for BOOLEAN_TYPE c_common_signed_type will just > return non-standard TYPE_PRECISION (boolean_type_node) signed INTEGER_TYPE. > Only for !INTEGRAL_TYPE_P types it will just return TYPE_MAIN_VARIANT > of the passed in type without changing it. > Thus, I've changed it into gcc_checking_assert. > > > I wonder if the > > pointer handling get_alias_set does (re-constructing "simplified" > > chains) suffers from a similar issue in case such chain is GCed > > before we query for the exact same alias set again ... > > I don't see how that can happen. The problem with the signed vs. > unsigned is that it creates (or can create) new types which aren't > in the GTY reachable locations yet. The pointer handling doesn't > do this, it walks the referenced types from the type and picks something > based on that. The only exception is prevailing_odr_type, but I think > that isn't done again and again. That build a new type, use alias set > from it, GC collect it, build a new type again and use alias set from > it is the problem. > > > So I wonder whether we should keep a back-mapping of alias-set to > > type as GC root? Meaning, add a pointer to the tree to > > alias_set_entry? > > I'd like to avoid that if at all possible.
Yeah, it looks like a last resort if the issue is more widespread (I did not attempt to look at other FEs get_alias_set hook). The patch LGTM. Thanks, Richard. > 2025-11-24 Jakub Jelinek <[email protected]> > > PR middle-end/122624 > * tree.cc (build_bitint_type): Use type_hash_canon_hash. > > * c-common.cc (c_common_get_alias_set): Fix up handling of BITINT_TYPE > and non-standard INTEGER_TYPEs. For unsigned _BitInt(1) always return > -1. For other unsigned types set TYPE_ALIAS_SET to get_alias_set of > corresponding signed type and return that. For signed types check if > corresponding unsigned type has TYPE_ALIAS_SET_KNOWN_P and if so copy > its TYPE_ALIAS_SET and return that. > > --- gcc/tree.cc.jj 2025-11-24 09:02:41.856988851 +0100 > +++ gcc/tree.cc 2025-11-24 09:42:18.382123139 +0100 > @@ -7414,9 +7414,8 @@ build_bitint_type (unsigned HOST_WIDE_IN > else > fixup_signed_type (itype); > > - inchash::hash hstate; > - inchash::add_expr (TYPE_MAX_VALUE (itype), hstate); > - ret = type_hash_canon (hstate.end (), itype); > + hashval_t hash = type_hash_canon_hash (itype); > + ret = type_hash_canon (hash, itype); > if (precision <= MAX_INT_CACHED_PREC) > (*bitint_type_cache)[precision + unsignedp] = ret; > > --- gcc/c-family/c-common.cc.jj 2025-11-24 09:02:41.761990505 +0100 > +++ gcc/c-family/c-common.cc 2025-11-24 09:50:05.732088006 +0100 > @@ -3943,14 +3943,40 @@ c_common_get_alias_set (tree t) > /* The C standard specifically allows aliasing between signed and > unsigned variants of the same type. We treat the signed > variant as canonical. */ > - if ((TREE_CODE (t) == INTEGER_TYPE || TREE_CODE (t) == BITINT_TYPE) > - && TYPE_UNSIGNED (t)) > + if (TREE_CODE (t) == INTEGER_TYPE || TREE_CODE (t) == BITINT_TYPE) > { > - tree t1 = c_common_signed_type (t); > - > - /* t1 == t can happen for boolean nodes which are always unsigned. */ > - if (t1 != t) > - return get_alias_set (t1); > + /* For normal INTEGER_TYPEs (except ones built by > + build_nonstandard_integer_type), both signed and unsigned variants > + of the type are always reachable from GTY roots, so just calling > + get_alias_set on the signed type is ok. For BITINT_TYPE and > + non-standard INTEGER_TYPEs, only unsigned could be used and the > + corresponding signed type could be created on demand and garbage > + collected as unused, so the alias set of unsigned type could keep > + changing. > + Avoid that by remembering the signed type alias set in > + TYPE_ALIAS_SET and also when being asked about !TYPE_UNSIGNED > + check if there isn't a corresponding unsigned type with > + TYPE_ALIAS_SET_KNOWN_P. */ > + if (TYPE_UNSIGNED (t)) > + { > + /* There is no signed _BitInt(1). */ > + if (TREE_CODE (t) == BITINT_TYPE && TYPE_PRECISION (t) == 1) > + return -1; > + tree t1 = c_common_signed_type (t); > + gcc_checking_assert (t != t1); > + TYPE_ALIAS_SET (t) = get_alias_set (t1); > + return TYPE_ALIAS_SET (t); > + } > + else > + { > + tree t1 = c_common_unsigned_type (t); > + gcc_checking_assert (t != t1); > + if (TYPE_ALIAS_SET_KNOWN_P (t1)) > + { > + TYPE_ALIAS_SET (t) = TYPE_ALIAS_SET (t1); > + return TYPE_ALIAS_SET (t); > + } > + } > } > > return -1; > > > Jakub > > -- Richard Biener <[email protected]> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Jochen Jaser, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
