On Tue, 28 Feb 2023, Jakub Jelinek wrote: > On Tue, Feb 28, 2023 at 09:43:37AM +0000, Richard Biener wrote: > > We try to make sure to put all built types into the type merging > > machinery, so I think it shouldn't happen - but then I cannot rule > > it out. I'm also not sure whether duplicates would really pose > > a problem here (other than waste). If we want to make sure we should > > probably enhance verify_types to verify the TYPE_POINTER_TO chain ... > > > > > I'll try to debug why those user aligned types become TYPE_MAIN_VARIANT > > > though... > > Ok, so this happens already in the FEs when trying to add the attribute: > #0 build_distinct_type_copy (type=<pointer_type 0x7fffea0219d8>) at > ../../gcc/tree.cc:5735 > #1 0x0000000000c2327b in build_type_attribute_qual_variant > (otype=<pointer_type 0x7fffea0219d8>, attribute=<tree_list 0x7fffea01e938>, > quals=0) at ../../gcc/attribs.cc:1298 > #2 0x0000000000c245b0 in build_type_attribute_variant (ttype=<pointer_type > 0x7fffea0219d8>, attribute=<tree_list 0x7fffea01e938>) at > ../../gcc/attribs.cc:1591 > #3 0x0000000000c22325 in decl_attributes (node=0x7fffffffd008, > attributes=<tree_list 0x7fffea01e910>, flags=1, last_decl=<tree 0x0>) at > ../../gcc/attribs.cc:964 > > So, perhaps the !TYPE_QUALS (t) could be just an assert, but maybe next to > the !TYPE_USER_ALIGN (t) (or just instead of?) we need !TYPE_ATTRIBUTES (t). > Because addition of attributes (but anything else that causes > build_distinct_type_copy rather than build_variant_type_copy) will create > new TYPE_MAIN_VARIANTS. > Looking around, TYPE_REF_IS_RVALUE references also create distinct types, > and while the C++ FE sticks them into the TYPE_REFERENCE_TO chain, it > ensures they go after the corresponding !TYPE_REF_IS_RVALUE entry, so > perhaps LTO should !TYPE_REF_IS_RVALUE for REFERENCE_TYPEs. > Other uses of build_distinct_type_copy in the FEs are mostly related to > ARRAY_TYPEs (in C FE as well as c-common). Asan uses it solely on integral > types, etc. For attributes, big question is if it when we set > *no_addr_attrs = true we still tweak some things on the type (not in place) > or not. > > So, here are two possible variant patches which fix the ICE on the > testcase too. > > 2023-02-28 Jakub Jelinek <ja...@redhat.com> > > PR target/108910 > * lto-common.cc (lto_fixup_prevailing_type): Don't add t to > TYPE_POINTER_TO or TYPE_REFERENCE_TO chain if it has > TYPE_ATTRIBUTES, or is TYPE_REF_IS_RVALUE, or some other type > with the same TYPE_MODE and TYPE_REF_CAN_ALIAS_ALL flag is already > present. > > --- gcc/lto/lto-common.cc.jj 2023-01-16 11:52:16.165732856 +0100 > +++ gcc/lto/lto-common.cc 2023-02-28 12:30:37.014471255 +0100 > @@ -984,21 +984,36 @@ lto_fixup_prevailing_type (tree t) > TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (mv); > TYPE_NEXT_VARIANT (mv) = t; > } > - > - /* The following reconstructs the pointer chains > - of the new pointed-to type if we are a main variant. We do > - not stream those so they are broken before fixup. */ > - if (TREE_CODE (t) == POINTER_TYPE > - && TYPE_MAIN_VARIANT (t) == t) > - { > - TYPE_NEXT_PTR_TO (t) = TYPE_POINTER_TO (TREE_TYPE (t)); > - TYPE_POINTER_TO (TREE_TYPE (t)) = t; > - } > - else if (TREE_CODE (t) == REFERENCE_TYPE > - && TYPE_MAIN_VARIANT (t) == t) > + else if (!TYPE_ATTRIBUTES (t)) > { > - TYPE_NEXT_REF_TO (t) = TYPE_REFERENCE_TO (TREE_TYPE (t)); > - TYPE_REFERENCE_TO (TREE_TYPE (t)) = t; > + /* The following reconstructs the pointer chains > + of the new pointed-to type if we are a main variant. We do > + not stream those so they are broken before fixup. > + Don't add it if despite being main variant it has > + attributes (then it was created with build_distinct_type_copy). > + Similarly don't add TYPE_REF_IS_RVALUE REFERENCE_TYPEs. > + Don't add it if there is something in the chain already. */ > + tree *p = NULL; > + if (TREE_CODE (t) == POINTER_TYPE) > + p = &TYPE_POINTER_TO (TREE_TYPE (t)); > + else if (TREE_CODE (t) == REFERENCE_TYPE && !TYPE_REF_IS_RVALUE (t)) > + p = &TYPE_REFERENCE_TO (TREE_TYPE (t)); > + if (p) > + { > + tree t2; > + for (t2 = *p; t2; t2 = TYPE_NEXT_PTR_TO (t2)) > + if (TYPE_MODE (t2) == TYPE_MODE (t) > + && TYPE_REF_CAN_ALIAS_ALL (t2) == TYPE_REF_CAN_ALIAS_ALL (t)) > + break;
Can we elide this searching please? Having duplicated should be harmless unless proved otherwise. OK with that change. Richard. > + if (t2 == NULL_TREE) > + { > + if (TREE_CODE (t) == POINTER_TYPE) > + TYPE_NEXT_PTR_TO (t) = *p; > + else > + TYPE_NEXT_REF_TO (t) = *p; > + *p = t; > + } > + } > } > } > > > > Jakub > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)