Hi, On Wed, Oct 30 2019, Jan Hubicka wrote: >> >> Looking at PR 92278, I think I found the real problem. In >> ipa_read_edge_info, you added code to throw away jump functions of edges >> that do not pass possibly_call_in_translation_unit_p() test. But that >> predicate incorrectly - or at least I think so, see below - returns >> false for edges leading to interposable symbols. The reason why I think >> it is incorrect is because a node which is considered interposable >> before merging can apparently later on be upgraded to a local one by >> ipa-visibility. >> >> I am testing the following fix, is it OK if it passes? >> >> The testcase passes a version script to the linker, I guess our LTO >> testsuite does not support that, does it? >> >> Thanks, >> >> Martin >> >> >> 2019-10-30 Martin Jambor <mjam...@suse.cz> >> >> ipa/92278 >> * cgraph.c (cgraph_edge::possibly_call_in_translation_unit_p): Fix >> availability comparison. > > yes, this is OK - thanks for looking into this! > While jump functions are interesting only for available symbols > interposable symbols may be promoted to them based on resolution info. > > Lets see how much extra memory this will cost. If it is too bad I can > add resolution info logic but duplicating it from ipa-visibility is not > cool.
OK, I have committed the patch. I wonder whether we want to revert the early exit in update_jump_functions_after_inlining - or replace it with a gcc_checking_assert - now. Anyway, thanks, Martin > > Honza >> --- >> gcc/cgraph.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/gcc/cgraph.c b/gcc/cgraph.c >> index d47d4128b1c..8057ccdb7c0 100644 >> --- a/gcc/cgraph.c >> +++ b/gcc/cgraph.c >> @@ -3813,7 +3813,7 @@ cgraph_edge::possibly_call_in_translation_unit_p (void) >> if (node->previous_sharing_asm_name) >> node = symtab_node::get_for_asmname (DECL_ASSEMBLER_NAME >> (callee->decl)); >> gcc_assert (TREE_PUBLIC (node->decl)); >> - return node->get_availability () >= AVAIL_AVAILABLE; >> + return node->get_availability () >= AVAIL_INTERPOSABLE; >> } >> >> /* A stashed copy of "symtab" for use by selftest::symbol_table_test. >> -- >> 2.23.0 >>
