Re: PING: [PATCH v2] tree-profile: Don't instrument an IFUNC resolver nor its callees

["H.J. Lu" <hjl.tools@gmail.com>]

On Tue, Apr 2, 2024 at 7:50 AM Jan Hubicka <hubicka@ucw.cz> wrote:
>
> > On Tue, Mar 5, 2024 at 1:45 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > We can't instrument an IFUNC resolver nor its callees as it may require
> > > TLS which hasn't been set up yet when the dynamic linker is resolving
> > > IFUNC symbols.
> > >
> > > Add an IFUNC resolver caller marker to cgraph_node and set it if the
> > > function is called by an IFUNC resolver.  Update tree_profiling to skip
> > > functions called by IFUNC resolver.
> > >
> > > Tested with profiledbootstrap on Fedora 39/x86-64.
> > >
> > > gcc/ChangeLog:
> > >
> > >         PR tree-optimization/114115
> > >         * cgraph.h (symtab_node): Add check_ifunc_callee_symtab_nodes.
> > >         (cgraph_node): Add called_by_ifunc_resolver.
> > >         * cgraphunit.cc (symbol_table::compile): Call
> > >         symtab_node::check_ifunc_callee_symtab_nodes.
> > >         * symtab.cc (check_ifunc_resolver): New.
> > >         (ifunc_ref_map): Likewise.
> > >         (is_caller_ifunc_resolver): Likewise.
> > >         (symtab_node::check_ifunc_callee_symtab_nodes): Likewise.
> > >         * tree-profile.cc (tree_profiling): Do not instrument an IFUNC
> > >         resolver nor its callees.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         PR tree-optimization/114115
> > >         * gcc.dg/pr114115.c: New test.
> >
> > PING.
>
> I am bit worried about commonly used functions getting "infected" by
> being called once from ifunc resolver.  I think we only use thread local
> storage for indirect call profiling, so we may just disable indirect
> call profiling for these functions.

Will change it.

> Also the patch will be noop with -flto -flto-partition=max, so probably
> we need to compute this flag at WPA time and stream to partitions.
>

Why is it a nop with -flto -flto-partition=max? I got

(gdb) bt
#0  symtab_node::check_ifunc_callee_symtab_nodes ()
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/symtab.cc:1440
#1  0x0000000000e487d3 in symbol_table::compile (this=0x7fffea006000)
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/cgraphunit.cc:2320
#2  0x0000000000d23ecf in lto_main ()
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/lto/lto.cc:687
#3  0x00000000015254d2 in compile_file ()
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/toplev.cc:449
#4  0x00000000015284a4 in do_compile ()
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/toplev.cc:2154
#5  0x0000000001528864 in toplev::main (this=0x7fffffffd84a, argc=16,
    argv=0x42261f0) at /export/gnu/import/git/gitlab/x86-gcc/gcc/toplev.cc:2310
#6  0x00000000030a3fe2 in main (argc=16, argv=0x7fffffffd958)
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/main.cc:39

Do you have a testcase to show that it is a nop?

-- 
H.J.