public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jan Hubicka <hubicka@ucw.cz>
To: Martin Jambor <mjambor@suse.cz>
Cc: "Arsen Arsenović" <arsen@aarsen.me>,
	"GCC Mailing List" <gcc@gcc.gnu.org>,
	"Martin Liska" <mliska@suse.cz>
Subject: Re: cgraph: does node->inlined_to imply node->clones is non-empty?
Date: Fri, 24 Mar 2023 14:29:22 +0100	[thread overview]
Message-ID: <ZB2lsmp1DG5Ve4ge@kam.mff.cuni.cz> (raw)
In-Reply-To: <ri6v8iq5nmf.fsf@suse.cz>

Hi,
> 
> It seems to me that the most correct fix is to add to
> walk_polymorphic_call_targets a check that the obtained possible target
> is still referenced_from_vtable_p() - because the alias that was
> originally a virtual function is referenced from a vtable that at this
> point is also known to be gone.  But the check looks like it is possibly
> expensive, so I wanted to discuss this with Honza first (hopefully next
> week).

External vtables are bit special since they can refer to things that are
not accessible in current unit (static functions from other translation
units etc).
We already have and test can_refer_decl_in_current_unit_p but we test it
before alias resolution (which is there just to avoid artifically
bumping up the number of possible targets)

So perhaps following untested patch would work?

diff --git a/gcc/ipa-devirt.cc b/gcc/ipa-devirt.cc
index 14cf132c767..b33ec708d47 100644
--- a/gcc/ipa-devirt.cc
+++ b/gcc/ipa-devirt.cc
@@ -2420,7 +2420,8 @@ maybe_record_node (vec <cgraph_node *> &nodes,
       alias_target = target_node->ultimate_alias_target (&avail);
       if (target_node != alias_target
          && avail >= AVAIL_AVAILABLE
-         && target_node->get_availability ())
+         && target_node->get_availability ()
+         && can_refer_decl_in_current_unit_p (target_node->decl, NULL))
        target_node = alias_target;
     }
 
I am at conference and will be able to test it only during weekend.
Honza
> 
> >
> > I had already figured that an error could've likely been in
> > reach-ability analysis, but my time ran low, and I had not confirmed
> > anything, or as little as formalized a theory, so I just wrote the
> > original email instead of following this trail of thought fully.
> >
> > Thank you for your guidance!  Have a lovely night :)
> 
> It is good thing that you asked, I also learned something new (that
> virtual and non-virtual functions can be ICFed together).
> 
> Martin

  reply	other threads:[~2023-03-24 13:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-13 10:24 Arsen Arsenović
2023-03-14 12:42 ` Martin Liška
2023-03-15 17:12 ` Martin Jambor
2023-03-18 15:26   ` Arsen Arsenović
2023-03-24 12:40     ` Martin Jambor
2023-03-24 13:29       ` Jan Hubicka [this message]
2023-03-28  1:10       ` Arsen Arsenović

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=ZB2lsmp1DG5Ve4ge@kam.mff.cuni.cz \
    --to=hubicka@ucw.cz \
    --cc=arsen@aarsen.me \
    --cc=gcc@gcc.gnu.org \
    --cc=mjambor@suse.cz \
    --cc=mliska@suse.cz \
    /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).