On Mon, Apr 28, 2025 at 5:31 AM liuhongt <hongtao....@intel.com> wrote:
>
> From: "hongtao.liu" <hongtao....@intel.com>
>
> For BB with all debug_stmt, it will be ignored by afdo_set_bb_count,
> but it can be set with count of single successors PHIs which edge from
> the BB.(only nonzero count is annotatted).
>
> Tested with -march=x86-64-v3 -O2 autofdo enabled, the issue in the PR
> is fixed.
> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> Ok for trunk?
>
> gcc/ChangeLog:
>
> PR gcov-profile/118581
> * auto-profile.cc (autofdo_source_profile::get_count_info):
> Overload the function with parameter gimple location instead
> of stmt.
> (afdo_set_bb_count): For !has_annotated BB, Check single
> successors PHIs corresponding to the block and use those
> count.
> ---
> gcc/auto-profile.cc | 53 ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/auto-profile.cc b/gcc/auto-profile.cc
> index 2d2d4a428f2..1fa2946c065 100644
> --- a/gcc/auto-profile.cc
> +++ b/gcc/auto-profile.cc
> @@ -303,6 +303,10 @@ public:
> in INFO and return true; otherwise return false. */
> bool get_count_info (gimple *stmt, count_info *info) const;
>
> + /* Find count_info for a given gimple location GIMPLE_LOC. If found,
> + store the count_info in INFO and return true; otherwise return false.
> */
> + bool get_count_info (location_t gimple_loc, count_info *info) const;
> +
> /* Find total count of the callee of EDGE. */
> gcov_type get_callsite_total_count (struct cgraph_edge *edge) const;
>
> @@ -724,11 +728,18 @@ autofdo_source_profile::get_function_instance_by_decl
> (tree decl) const
> bool
> autofdo_source_profile::get_count_info (gimple *stmt, count_info *info) const
> {
> - if (LOCATION_LOCUS (gimple_location (stmt)) == cfun->function_end_locus)
> + return get_count_info (gimple_location (stmt), info);
> +}
> +
> +bool
> +autofdo_source_profile::get_count_info (location_t gimple_loc,
> + count_info *info) const
> +{
> + if (LOCATION_LOCUS (gimple_loc) == cfun->function_end_locus)
> return false;
>
> inline_stack stack;
> - get_inline_stack (gimple_location (stmt), &stack);
> + get_inline_stack (gimple_loc, &stack);
> if (stack.length () == 0)
> return false;
> function_instance *s = get_function_instance_by_inline_stack (stack);
> @@ -1132,7 +1143,43 @@ afdo_set_bb_count (basic_block bb, const stmt_set
> &promoted)
> }
>
> if (!has_annotated)
> - return false;
> + {
> + /* For BB with all debug stmt which assigne a value with constant,
> + check successors PHIs corresponding to the block and
> + use those counts. */
I think the comment is a bit off, it should be "For an empty BB ..." since
we should not change behavior on whether there are debug stmts or not.
> + edge tmp_e;
> + edge_iterator tmp_ei;
> + FOR_EACH_EDGE (tmp_e, tmp_ei, bb->succs)
> + {
> + basic_block bb_succ = tmp_e->dest;
> + for (gphi_iterator gpi = gsi_start_phis (bb_succ);
> + !gsi_end_p (gpi);
> + gsi_next (&gpi))
> + {
> + gphi *phi = gpi.phi ();
> + size_t i;
> + for (i = 0; i < gimple_phi_num_args (phi); i++)
> + {
> + edge e = gimple_phi_arg_edge (phi, i);
> + if (e->src != bb)
> + continue;
> + location_t phi_loc = gimple_phi_arg_location (phi, i);
> + inline_stack stack;
> + count_info info;
> + if (afdo_source_profile->get_count_info (phi_loc, &info)
> + && info.count != 0)
> + {
> + if (info.count > max_count)
> + max_count = info.count;
> + has_annotated = true;
> + }
> + }
> + }
> + }
> +
> + if (!has_annotated)
> + return false;
> + }
>
> for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> afdo_source_profile->mark_annotated (gimple_location (gsi_stmt (gsi)));
> --
> 2.34.1
>
