On Mon, 15 Apr 2024, Richard Biener wrote:
> On Sun, 14 Apr 2024, H.J. Lu wrote:
>
> > We can't profile indirect calls to IFUNC resolvers nor their callees as
> > it requires 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. Disable indirect call profiling
> > for IFUNC resolvers and their callees.
> >
> > 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 (gimple_gen_ic_func_profiler): Disable indirect
> > call profiling for IFUNC resolvers and their callees.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR tree-optimization/114115
> > * gcc.dg/pr114115.c: New test.
> >
> > (cherry picked from commit cab32bacaea268ec062b1fb4fc662d90c9d1cfce)
> > ---
> > gcc/cgraph.h | 6 +++
> > gcc/cgraphunit.cc | 2 +
> > gcc/symtab.cc | 89 +++++++++++++++++++++++++++++++++
> > gcc/testsuite/gcc.dg/pr114115.c | 24 +++++++++
> > gcc/tree-profile.cc | 8 ++-
> > 5 files changed, 128 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/gcc.dg/pr114115.c
> >
> > diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> > index c1a3691b6f5..430c87d8bb7 100644
> > --- a/gcc/cgraph.h
> > +++ b/gcc/cgraph.h
> > @@ -479,6 +479,9 @@ public:
> > Return NULL if there's no such node. */
> > static symtab_node *get_for_asmname (const_tree asmname);
> >
> > + /* Check symbol table for callees of IFUNC resolvers. */
> > + static void check_ifunc_callee_symtab_nodes (void);
> > +
> > /* Verify symbol table for internal consistency. */
> > static DEBUG_FUNCTION void verify_symtab_nodes (void);
> >
> > @@ -896,6 +899,7 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node :
> > public symtab_node
> > redefined_extern_inline (false), tm_may_enter_irr (false),
> > ipcp_clone (false), declare_variant_alt (false),
> > calls_declare_variant_alt (false), gc_candidate (false),
> > + called_by_ifunc_resolver (false),
> > m_uid (uid), m_summary_id (-1)
> > {}
> >
> > @@ -1491,6 +1495,8 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node :
> > public symtab_node
> > is set for local SIMD clones when they are created and cleared if the
> > vectorizer uses them. */
> > unsigned gc_candidate : 1;
> > + /* Set if the function is called by an IFUNC resolver. */
> > + unsigned called_by_ifunc_resolver : 1;
> >
> > private:
> > /* Unique id of the node. */
> > diff --git a/gcc/cgraphunit.cc b/gcc/cgraphunit.cc
> > index bccd2f2abb5..40dcceccca5 100644
> > --- a/gcc/cgraphunit.cc
> > +++ b/gcc/cgraphunit.cc
> > @@ -2313,6 +2313,8 @@ symbol_table::compile (void)
> >
> > symtab_node::checking_verify_symtab_nodes ();
> >
> > + symtab_node::check_ifunc_callee_symtab_nodes ();
> > +
> > timevar_push (TV_CGRAPHOPT);
> > if (pre_ipa_mem_report)
> > dump_memory_report ("Memory consumption before IPA");
> > diff --git a/gcc/symtab.cc b/gcc/symtab.cc
> > index 0470509a98d..df09def81e9 100644
> > --- a/gcc/symtab.cc
> > +++ b/gcc/symtab.cc
> > @@ -1369,6 +1369,95 @@ symtab_node::verify (void)
> > timevar_pop (TV_CGRAPH_VERIFY);
> > }
> >
> > +/* Return true and set *DATA to true if NODE is an ifunc resolver. */
> > +
> > +static bool
> > +check_ifunc_resolver (cgraph_node *node, void *data)
> > +{
> > + if (node->ifunc_resolver)
> > + {
> > + bool *is_ifunc_resolver = (bool *) data;
> > + *is_ifunc_resolver = true;
> > + return true;
> > + }
> > + return false;
> > +}
> > +
> > +static auto_bitmap ifunc_ref_map;
>
> Please don't use static auto_bitmap, that isn't constructed
> properly.
>
> Instead allocate it in the proper place and make sure to
> initialize the global bitmap obstack.
Sorry - ignore that. I've not seen 2/2.
Richard.
> > +
> > +/* Return true if any caller of NODE is an ifunc resolver. */
> > +
> > +static bool
> > +is_caller_ifunc_resolver (cgraph_node *node)
> > +{
> > + bool is_ifunc_resolver = false;
> > +
> > + for (cgraph_edge *e = node->callers; e; e = e->next_caller)
> > + {
> > + /* Return true if caller is known to be an IFUNC resolver. */
> > + if (e->caller->called_by_ifunc_resolver)
> > + return true;
> > +
> > + /* Check for recursive call. */
> > + if (e->caller == node)
> > + continue;
> > +
> > + /* Skip if it has been visited. */
> > + unsigned int uid = e->caller->get_uid ();
> > + if (bitmap_bit_p (ifunc_ref_map, uid))
> > + continue;
> > + bitmap_set_bit (ifunc_ref_map, uid);
> > +
> > + if (is_caller_ifunc_resolver (e->caller))
> > + {
> > + /* Return true if caller is an IFUNC resolver. */
> > + e->caller->called_by_ifunc_resolver = true;
> > + return true;
> > + }
> > +
> > + /* Check if caller's alias is an IFUNC resolver. */
> > + e->caller->call_for_symbol_and_aliases (check_ifunc_resolver,
> > + &is_ifunc_resolver,
> > + true);
> > + if (is_ifunc_resolver)
> > + {
> > + /* Return true if caller's alias is an IFUNC resolver. */
> > + e->caller->called_by_ifunc_resolver = true;
> > + return true;
> > + }
> > + }
> > +
> > + return false;
> > +}
> > +
> > +/* Check symbol table for callees of IFUNC resolvers. */
> > +
> > +void
> > +symtab_node::check_ifunc_callee_symtab_nodes (void)
> > +{
> > + symtab_node *node;
> > +
> > + FOR_EACH_SYMBOL (node)
> > + {
> > + cgraph_node *cnode = dyn_cast <cgraph_node *> (node);
> > + if (!cnode)
> > + continue;
> > +
> > + unsigned int uid = cnode->get_uid ();
> > + if (bitmap_bit_p (ifunc_ref_map, uid))
> > + continue;
> > + bitmap_set_bit (ifunc_ref_map, uid);
> > +
> > + bool is_ifunc_resolver = false;
> > + cnode->call_for_symbol_and_aliases (check_ifunc_resolver,
> > + &is_ifunc_resolver, true);
> > + if (is_ifunc_resolver || is_caller_ifunc_resolver (cnode))
> > + cnode->called_by_ifunc_resolver = true;
> > + }
> > +
> > + bitmap_clear (ifunc_ref_map);
> > +}
> > +
> > /* Verify symbol table for internal consistency. */
> >
> > DEBUG_FUNCTION void
> > diff --git a/gcc/testsuite/gcc.dg/pr114115.c
> > b/gcc/testsuite/gcc.dg/pr114115.c
> > new file mode 100644
> > index 00000000000..2629f591877
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/pr114115.c
> > @@ -0,0 +1,24 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O0 -fprofile-generate -fdump-tree-optimized" } */
> > +/* { dg-require-profiling "-fprofile-generate" } */
> > +/* { dg-require-ifunc "" } */
> > +
> > +void *foo_ifunc2() __attribute__((ifunc("foo_resolver")));
> > +
> > +void bar(void)
> > +{
> > +}
> > +
> > +static int f3()
> > +{
> > + bar ();
> > + return 5;
> > +}
> > +
> > +void (*foo_resolver(void))(void)
> > +{
> > + f3();
> > + return bar;
> > +}
> > +
> > +/* { dg-final { scan-tree-dump-not "__gcov_indirect_call_profiler_v"
> > "optimized" } } */
> > diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
> > index da300d5f9e8..b5de0fb914f 100644
> > --- a/gcc/tree-profile.cc
> > +++ b/gcc/tree-profile.cc
> > @@ -418,7 +418,13 @@ gimple_gen_ic_func_profiler (void)
> > gcall *stmt1;
> > tree tree_uid, cur_func, void0;
> >
> > - if (c_node->only_called_directly_p ())
> > + /* Disable indirect call profiling for an IFUNC resolver and its
> > + callees since it requires TLS which hasn't been set up yet when
> > + the dynamic linker is resolving IFUNC symbols. See
> > + https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114115
> > + */
> > + if (c_node->only_called_directly_p ()
> > + || c_node->called_by_ifunc_resolver)
> > return;
> >
> > gimple_init_gcov_profiler ();
> >
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
