On 29 September 2017 at 12:28, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > I wonder what happens here when, say, ipa-icf redirect the call to eqivaelnt
>> > function and removes the callee?  Perhaps we realy want to have set of call
>> > sites rahter than nodes stored from analysis to execution. Call sites have
>> > unique stmts and uids, so it will be possible to map them back and forth.
>> IIUC, call site is represented using cgraph_edge ?
>
> Yes, there is call_stmt pointer when gimple is read and uid in WPA mode which
> are unique.  There is call_summary class which lets you to associate info with
> a call site. While it is not most memory effecient to store one bit of information
> this way, i guess it may be easiest to use it in anticipation of it becoming
> more useful in foreseeable future.  We have some bits in call edge itself that
> may be shuffled to the summary as well.
>
>> So change return_callees_map to be the mapping from node to subset of
>> it's call-sites where
>> node returns the value of one of it's callees:
>> static hash_map< cgraph_node *, vec<cgraph_edge *> *> *return_callees_map; ?
>
> This should work too, though storing direct pointers to edges is something we
> probably want to avoid to keep memory representation of edges in bounds.
>
> I would go with call_summary - everything else seems like bit of premature
> optimization.  If we will decide to optimize it later, we may invent a variant
> of summary datatype for that.
Hi Honza,
Thanks for the detailed suggestions, I have updated the patch accordingly.
I have following questions on call_summary:
1] I added field bool is_return_callee in ipa_call_summary to track
whether the caller possibly returns value returned by callee, which
gets rid of return_callees_map. I assume ipa_call_summary_t::remove()
and ipa_call_summary_t::duplicate() will already take care of handling
late insertion/removal of cgraph nodes ? I just initialized
is_return_callee to false in ipa_call_summary::reset and that seems to
work. I am not sure though if I have handled it correctly. Could you
please check that ?

2] ipa_inline() called ipa_free_fn_summary, which made
ipa_call_summaries unavailable during ipa-pure-const pass. I removed
call to ipa_free_fn_summary from ipa_inline, and moved it to
ipa_pure_const::execute(). Is that OK ?

Patch passes bootstrap+test and lto bootstrap+test on x86_64-unknown-linux-gnu.
Verfiied SPEC2k6 compiles and runs without miscompares with LTO
enabled on aarch64-linux-gnu.
Cross-tested on arm*-*-* and aarch64*-*-*. I will additionally test
the patch by building chromium or firefox.
Would it be OK to commit if it passes above validations ?

Thanks,
Prathamesh
>
> Thanks,
> Honza