From: Jakub Jelinek <jakub@redhat.com>
To: Richard Biener <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] lto: Fix up lto_fixup_prevailing_type [PR108910]
Date: Tue, 28 Feb 2023 12:37:05 +0100 [thread overview]
Message-ID: <Y/3nYSPgS/qk/Ghq@tucnak> (raw)
In-Reply-To: <nycvar.YFH.7.77.849.2302280942110.27913@jbgna.fhfr.qr>
[-- Attachment #1: Type: text/plain, Size: 4381 bytes --]
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 <jakub@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;
+ 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
[-- Attachment #2: R819 --]
[-- Type: text/plain, Size: 1881 bytes --]
2023-02-28 Jakub Jelinek <jakub@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.
--- gcc/lto/lto-common.cc.jj 2023-01-16 11:52:16.165732856 +0100
+++ gcc/lto/lto-common.cc 2023-02-28 12:34:33.698038478 +0100
@@ -984,21 +984,25 @@ 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. */
+ if (TREE_CODE (t) == POINTER_TYPE)
+ {
+ 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_REF_IS_RVALUE (t))
+ {
+ TYPE_NEXT_REF_TO (t) = TYPE_REFERENCE_TO (TREE_TYPE (t));
+ TYPE_REFERENCE_TO (TREE_TYPE (t)) = t;
+ }
}
}
next prev parent reply other threads:[~2023-02-28 11:37 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-28 8:02 Jakub Jelinek
2023-02-28 8:58 ` Richard Biener
2023-02-28 9:19 ` Jakub Jelinek
2023-02-28 9:43 ` Richard Biener
2023-02-28 11:37 ` Jakub Jelinek [this message]
2023-02-28 12:05 ` Richard Biener
2023-02-28 12:16 ` Jakub Jelinek
2023-02-28 13:36 ` Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y/3nYSPgS/qk/Ghq@tucnak \
--to=jakub@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).