From: Xionghu Luo <xionghuluo@tencent.com>
To: gcc-patches@gcc.gnu.org
Cc: luoxhu@gcc.gnu.org, hubicka@ucw.cz, rguenther@suse.de,
sandra@codesourcery.com, Xionghu Luo <xionghuluo@tencent.com>
Subject: [RFC PATCH] ipa-visibility: Fix ICE in lto-partition caused by incorrect comdat group solving in ipa-visibility
Date: Mon, 27 Mar 2023 16:52:52 +0800 [thread overview]
Message-ID: <20230327085252.3390790-1-xionghuluo@tencent.com> (raw)
I have a case ICE in lto-partion.c:158 not easy to reduce, this ICE
appears some time ago from link:
https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586290.html.
I tried the proposed patch but it doesn't work for me.
Then I did some hack and finally got a successful lto link to compare
with a ICE lto link. It seems to me that the ICE in
add_symbol_to_partition_1, at lto/lto-partition.c:158
is caused by not dissovle comdat_group_list correctly in
update_visibility_by_resolution_info.
The ICE node is a preempted_reg '__dt_del' function with same_comdat_group
linked to '__dt_base' function, succeeded by '__dt_comp' function.
Success resolution is:
2420 f75f1945 PREVAILING_DEF _ZN6google8protobuf8internal16FunctionClosure1IPKNS0_15FieldDescriptorEED2Ev/81027
2422 f75f1945 PREVAILING_DEF _ZN6google8protobuf8internal16FunctionClosure1IPKNS0_15FieldDescriptorEED1Ev/81028
2424 f75f1945 PREEMPTED_REG _ZN6google8protobuf8internal16FunctionClosure1IPKNS0_15FieldDescriptorEED0Ev/81029
with FOR_EACH_FUNCTION access order:
81029(__dt_del) -> 81028(__dt_comp) -> 81027(__dt_base)
81029 is accessed first, and it is markded externally_visable false,
then accessing 81028 removed all the same_comdat_groups as expected.
ICE resolution is:
2362 f75f1945 PREEMPTED_REG _ZN6google8protobuf8internal16FunctionClosure1IPKNS0_15FieldDescriptorEED0Ev/81029
2365 f75f1945 PREVAILING_DEF _ZN6google8protobuf8internal16FunctionClosure1IPKNS0_15FieldDescriptorEED2Ev/81027
2367 f75f1945 PREVAILING_DEF _ZN6google8protobuf8internal16FunctionClosure1IPKNS0_15FieldDescriptorEED1Ev/81028
with FOR_EACH_FUNCTION access order:
81028(__dt_comp) -> 81027(__dt_base) -> 81029(__dt_del)
81028 is accessed firstly, and node 81029's externally_visible is still
true, when calling 81028's update_visibility_by_resolution_info,
it early returns as 'same_def' is false then fail to dissolve
same_comdat_group list.
So the point here is if PREEMPTED_REG node is not accessed first, the
externallay_visable variable won't be updated on time when accessing
PREVAILING_DEF nodes first.
This patch using *function check* instead of *variable check* to eliminate
the resolution sequence influence.
Not sure whether this patch could also fix Sandra's ICE either?
gcc/ChangeLog:
* ipa-visibility.cc (update_visibility_by_resolution_info):
Check node's externally_visiable with function instead of
variable.
(function_and_variable_visibility): New parameter whole_program.
Signed-off-by: Xionghu Luo <xionghuluo@tencent.com>
---
gcc/ipa-visibility.cc | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc
index 8ec82bb333e..1ebc584ffd9 100644
--- a/gcc/ipa-visibility.cc
+++ b/gcc/ipa-visibility.cc
@@ -393,7 +393,7 @@ update_vtable_references (tree *tp, int *walk_subtrees,
resolution info. */
static void
-update_visibility_by_resolution_info (symtab_node * node)
+update_visibility_by_resolution_info (symtab_node * node, bool whole_program)
{
bool define;
@@ -412,7 +412,12 @@ update_visibility_by_resolution_info (symtab_node * node)
for (symtab_node *next = node->same_comdat_group;
next != node; next = next->same_comdat_group)
{
- if (!next->externally_visible || next->transparent_alias)
+ if ((is_a<varpool_node *> (next)
+ && !dyn_cast<varpool_node *> (next)->externally_visible_p ())
+ || (is_a<cgraph_node *> (next)
+ && !cgraph_externally_visible_p (dyn_cast<cgraph_node *> (next),
+ whole_program))
+ || next->transparent_alias)
continue;
bool same_def
@@ -750,7 +755,7 @@ function_and_variable_visibility (bool whole_program)
DECL_EXTERNAL (node->decl) = 1;
}
- update_visibility_by_resolution_info (node);
+ update_visibility_by_resolution_info (node, whole_program);
if (node->weakref)
optimize_weakref (node);
}
@@ -842,7 +847,7 @@ function_and_variable_visibility (bool whole_program)
&& !DECL_EXTERNAL (vnode->decl))
localize_node (whole_program, vnode);
- update_visibility_by_resolution_info (vnode);
+ update_visibility_by_resolution_info (vnode, whole_program);
/* Update virtual tables to point to local aliases where possible. */
if (DECL_VIRTUAL_P (vnode->decl)
--
2.27.0
reply other threads:[~2023-03-27 8:53 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=20230327085252.3390790-1-xionghuluo@tencent.com \
--to=xionghuluo@tencent.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hubicka@ucw.cz \
--cc=luoxhu@gcc.gnu.org \
--cc=rguenther@suse.de \
--cc=sandra@codesourcery.com \
/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).