public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
From: Martin Jambor <mjambor@suse.cz>
To: Erick Ochoa <eochoa@gcc.gnu.org>
Cc: gcc@gcc.gnu.org
Subject: Re: Question about speculative make_edge_direct_to_target during LTO/IPA_PASS
Date: Fri, 08 Jul 2022 17:31:50 +0200	[thread overview]
Message-ID: <ri68rp3hb89.fsf@suse.cz> (raw)
In-Reply-To: <CAJ_nqzhdMJpGj_u4Cak6ZYcG=W5JR1Ersvm8Z6QuBz4NFGN0cQ@mail.gmail.com>

Hi Erick,,

On Thu, Jul 07 2022, Erick Ochoa wrote:
> On Fri, 1 Jul 2022 at 14:48, Martin Jambor <mjambor@suse.cz> wrote:
>
>> Why so late, why not as part of number 4?
>>
> Thanks for the feedback. The original reason why the call to
> make_edge_direct_to_target was done so late is because of the lack of
> function bodies during WPA, summaries with insufficient information (I keep
> track of only some information which when access to function bodies is
> given during LTRANS I can finally have a cgraph_edge to use as a
> parameter), and I am still figuring out things with edge summaries.
>
> I have a couple of questions here to improve my summaries:
>
> If I were to call make_edge_direct_to_target during WPA (or number 4) then
> I need to have access to the indirect edge during WPA. This is because
> make_edge_direct_to_target receives a cgraph_edge as a parameter. In order
> to have access to the indirect edge during WPA, I need to generate
> summaries which keep track of indirect edges.

That is being done and should not be a problem.  You just iterate over
the list beginning with node->indirect_calls and analyze it like normal
edges.

>
> I think this would be done with edge summaries. Again, my understanding of
> edge summaries is incomplete.

They are meant to be a generic mechanism to describe calls that is
pass-specific, they contain what the author of the pass puts in there.

> My understanding is that they contain
> information to be transmitted along the edges of the call graph.

That is how they are often used but generally speaking they are simply
some sort of a description of a call.

> However,
> the implementation details are what is making it difficult for me to
> understand. Unlike cgraph_nodes which can be encoded and decoded, it seems
> to me that cgraph_edges have no encoding nor decoding. This makes it
> somewhat hard to say that an information belongs to a given edge.

I am not sure I understand, you mean the encoding and decoding during
streaming?  I simply suggest that you do what ipa-fnsummary and IPA-SRA
do.  The mechanism is quite ugly, improvement in the streaming API would
be IMHO desirable, but I guess that is quite outside the scope of your
project.

>
> Looking into ipa-cp / ipa-prop it looks like edge summaries are stored by
> first encoding the caller cgraph_node and then iterating over the edges in
> the node->callees and node->indirect_calls. However, I know that edges can
> be removed.

Fortunately, not in between streaming them to disk and then back, so this
should not be a concern.

> That's why there are edge removal hooks.

No, removal hooks are there mainly to deallocate memory that is no
longer necessary (plus they are used by reference counting and stuff,
but not for streaming consistency).

> Shouldn't this affect
> the information that is written if an edge was removed somewhere else?
> Something that is not explicit, is also that the order in the node->callees
> and node->indirect_calls is preserved from the moment the summary is
> written to the moment the summary is read. Is this the case?

Yes, otherwise the code in place would not work, I guess.

>
> My understanding is also limited from the fact that ipa-cp does not have a
> proper write_summary nor read_summary stages in its own class and instead
> ipa-cp's summaries are written and read during ipa-prop.

ipa-prop is a file, not a pass.  The summaries are generated and written
during pass_ipa_fn_summary.  The only reason for that is that both
IPA-CP and inlining (and now also IPA-modref, I think) use what
previously were summaries of the other pass and eventually it made sense
to gather and stream them together.

> Is this an
> implementation detail so that the edge information created for ipa-cp is
> propagated correctly during inlining? (Or is there some other explanation
> for this?)

See above, there are no correctness reasons for this.

> Would it also makes sense to write and read my edge summaries at
> the same time as ipa-cp's edge summaries?

That is difficult to say without knowing what is in your summary and
what else you need.  If you make heavy use of information gathered by
IPA-CP/inlining, then perhaps yes.

But some isolation of the individual passes is also good.  So for
example IPA-SRA has its own node and edge summaries.

> I know that the base class
> (call_summary) also has virtual methods for removal and duplication of
> edges (which should be triggered during inlining / cleaning up of the
> callgraph). But somehow the virtual methods ::remove and ::duplicate seem
> different to me from the callbacks which need to be registered in the
> IPA_PASSes.

I'm not sure I understand.  Do you mean hooks like add_edge_removal_hook
of the symbol_table class?  Summaries use these internally to implement
the virtual methods you describe but there is nothing compulsory about
the hooks.  I also do not see what difference in between the two you
find troubling.

> Again, why would one need to write and read ipa-cp's summaries
> during ipa-prop if these callbacks exist which are triggered when summaries
> need to be updated?

The summary updating callbacks exist because passes run one after each
other and when one modifies the call-graph, the others often need to
update their own internal data structures, which summaries are.  The
division of code between ipa-prop.cc and ipa-cp.cc does not really
matter here.

Martin

  reply	other threads:[~2022-07-08 15:31 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-01  9:46 Erick Ochoa
2022-07-01 12:48 ` Martin Jambor
2022-07-07 13:20   ` Erick Ochoa
2022-07-08 15:31     ` Martin Jambor [this message]
2022-07-14 12:40       ` Erick Ochoa

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=ri68rp3hb89.fsf@suse.cz \
    --to=mjambor@suse.cz \
    --cc=eochoa@gcc.gnu.org \
    --cc=gcc@gcc.gnu.org \
    /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).