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

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

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.
> ---
>  gcc/cgraph.h                    |  6 +++
>  gcc/cgraphunit.cc               |  2 +
>  gcc/symtab.cc                   | 89 +++++++++++++++++++++++++++++++++
>  gcc/testsuite/gcc.dg/pr114115.c | 24 +++++++++
>  gcc/tree-profile.cc             |  4 ++
>  5 files changed, 125 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/pr114115.c
>
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index 47f35e8078d..a8c3224802c 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)
>    {}
>
> @@ -1495,6 +1499,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 d200166f7e9..2bd0289ffba 100644
> --- a/gcc/cgraphunit.cc
> +++ b/gcc/cgraphunit.cc
> @@ -2317,6 +2317,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 4c7e3c135ca..3256133891d 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;
> +
> +/* 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 aed13e2b1bc..2e55a5d8402 100644
> --- a/gcc/tree-profile.cc
> +++ b/gcc/tree-profile.cc
> @@ -848,6 +848,10 @@ tree_profiling (void)
>             }
>         }
>
> +      /* Do not instrument an IFUNC resolver nor its callees.  */
> +      if (node->called_by_ifunc_resolver)
> +       continue;
> +
>        push_cfun (DECL_STRUCT_FUNCTION (node->decl));
>
>        if (dump_file)
> --
> 2.44.0
>

PING.

-- 
H.J.