public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Jan Hubicka <hubicka@ucw.cz>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: Check that passes do not forget to define profile
Date: Thu, 24 Aug 2023 15:20:29 +0200	[thread overview]
Message-ID: <CAFiYyc0LXMS1U6JVX8ogJVmrGybPRR75M6L3QefOLtjR9ziB=g@mail.gmail.com> (raw)
In-Reply-To: <ZOdXuCEkw7On4xBo@kam.mff.cuni.cz>

On Thu, Aug 24, 2023 at 3:15 PM Jan Hubicka via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
> this patch extends verifier to check that all probabilities and counts are
> initialized if profile is supposed to be present.  This is a bit complicated
> by the posibility that we inline !flag_guess_branch_probability function
> into function with profile defined and in this case we need to stop
> verification.  For this reason I added flag to cfg structure tracking this.
>
> Bootstrapped/regtested x86_64-linux, comitted.

Couldn't we have massaged profile_status to avoid extra full_profile?
Aka add PROFILE_{READ,GUESSED}_PARTIAL?

> gcc/ChangeLog:
>
>         * cfg.h (struct control_flow_graph): New field full_profile.
>         * auto-profile.cc (afdo_annotate_cfg): Set full_profile to true.
>         * cfg.cc (init_flow): Set full_profile to false.
>         * graphite.cc (graphite_transform_loops): Set full_profile to false.
>         * lto-streamer-in.cc (input_cfg): Initialize full_profile flag.
>         * predict.cc (pass_profile::execute): Set full_profile to true.
>         * symtab-thunks.cc (expand_thunk): Set full_profile to true.
>         * tree-cfg.cc (gimple_verify_flow_info): Verify that profile is full
>         if full_profile is set.
>         * tree-inline.cc (initialize_cfun): Initialize full_profile.
>         (expand_call_inline): Combine full_profile.
>
>
> diff --git a/gcc/auto-profile.cc b/gcc/auto-profile.cc
> index e3af3555e75..ff3b763945c 100644
> --- a/gcc/auto-profile.cc
> +++ b/gcc/auto-profile.cc
> @@ -1578,6 +1578,7 @@ afdo_annotate_cfg (const stmt_set &promoted_stmts)
>      }
>    update_max_bb_count ();
>    profile_status_for_fn (cfun) = PROFILE_READ;
> +  cfun->cfg->full_profile = true;
>    if (flag_value_profile_transformations)
>      {
>        gimple_value_profile_transformations ();
> diff --git a/gcc/cfg.cc b/gcc/cfg.cc
> index 9eb9916f61a..b7865f14e7f 100644
> --- a/gcc/cfg.cc
> +++ b/gcc/cfg.cc
> @@ -81,6 +81,7 @@ init_flow (struct function *the_fun)
>      = ENTRY_BLOCK_PTR_FOR_FN (the_fun);
>    the_fun->cfg->edge_flags_allocated = EDGE_ALL_FLAGS;
>    the_fun->cfg->bb_flags_allocated = BB_ALL_FLAGS;
> +  the_fun->cfg->full_profile = false;
>  }
>
>  /* Helper function for remove_edge and free_cffg.  Frees edge structure
> diff --git a/gcc/cfg.h b/gcc/cfg.h
> index a0e944979c8..53e2553012c 100644
> --- a/gcc/cfg.h
> +++ b/gcc/cfg.h
> @@ -78,6 +78,9 @@ struct GTY(()) control_flow_graph {
>    /* Dynamically allocated edge/bb flags.  */
>    int edge_flags_allocated;
>    int bb_flags_allocated;
> +
> +  /* Set if the profile is computed on every edge and basic block.  */
> +  bool full_profile;
>  };
>
>
> diff --git a/gcc/graphite.cc b/gcc/graphite.cc
> index 19f8975ffa2..2b387d5b016 100644
> --- a/gcc/graphite.cc
> +++ b/gcc/graphite.cc
> @@ -512,6 +512,8 @@ graphite_transform_loops (void)
>
>    if (changed)
>      {
> +      /* FIXME: Graphite does not update profile meaningfully currently.  */
> +      cfun->cfg->full_profile = false;
>        cleanup_tree_cfg ();
>        profile_status_for_fn (cfun) = PROFILE_ABSENT;
>        release_recorded_exits (cfun);
> diff --git a/gcc/lto-streamer-in.cc b/gcc/lto-streamer-in.cc
> index 0cce14414ca..d3128fcebe4 100644
> --- a/gcc/lto-streamer-in.cc
> +++ b/gcc/lto-streamer-in.cc
> @@ -1030,6 +1030,7 @@ input_cfg (class lto_input_block *ib, class data_in *data_in,
>    basic_block p_bb;
>    unsigned int i;
>    int index;
> +  bool full_profile = false;
>
>    init_empty_tree_cfg_for_function (fn);
>
> @@ -1071,6 +1072,8 @@ input_cfg (class lto_input_block *ib, class data_in *data_in,
>           data_in->location_cache.input_location_and_block (&e->goto_locus,
>                                                             &bp, ib, data_in);
>           e->probability = profile_probability::stream_in (ib);
> +         if (!e->probability.initialized_p ())
> +           full_profile = false;
>
>         }
>
> @@ -1145,6 +1148,7 @@ input_cfg (class lto_input_block *ib, class data_in *data_in,
>
>    /* Rebuild the loop tree.  */
>    flow_loops_find (loops);
> +  cfun->cfg->full_profile = full_profile;
>  }
>
>
> diff --git a/gcc/predict.cc b/gcc/predict.cc
> index 5a1a561cc24..396746cbfd1 100644
> --- a/gcc/predict.cc
> +++ b/gcc/predict.cc
> @@ -4131,6 +4131,7 @@ pass_profile::execute (function *fun)
>      scev_initialize ();
>
>    tree_estimate_probability (false);
> +  cfun->cfg->full_profile = true;
>
>    if (nb_loops > 1)
>      scev_finalize ();
> diff --git a/gcc/symtab-thunks.cc b/gcc/symtab-thunks.cc
> index 4c04235c41b..23ead0d2138 100644
> --- a/gcc/symtab-thunks.cc
> +++ b/gcc/symtab-thunks.cc
> @@ -648,6 +648,7 @@ expand_thunk (cgraph_node *node, bool output_asm_thunks,
>           ? PROFILE_READ : PROFILE_GUESSED;
>        /* FIXME: C++ FE should stop setting TREE_ASM_WRITTEN on thunks.  */
>        TREE_ASM_WRITTEN (thunk_fndecl) = false;
> +      cfun->cfg->full_profile = true;
>        delete_unreachable_blocks ();
>        update_ssa (TODO_update_ssa);
>        checking_verify_flow_info ();
> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> index 272d5ce321e..ffab7518b15 100644
> --- a/gcc/tree-cfg.cc
> +++ b/gcc/tree-cfg.cc
> @@ -5684,6 +5684,26 @@ gimple_verify_flow_info (void)
>         error ("fallthru to exit from bb %d", e->src->index);
>         err = true;
>        }
> +  if (cfun->cfg->full_profile
> +      && !ENTRY_BLOCK_PTR_FOR_FN (cfun)->count.initialized_p ())
> +    {
> +      error ("entry block count not initialized");
> +      err = true;
> +    }
> +  if (cfun->cfg->full_profile
> +      && !EXIT_BLOCK_PTR_FOR_FN (cfun)->count.initialized_p ())
> +    {
> +      error ("exit block count not initialized");
> +      err = true;
> +    }
> +  if (cfun->cfg->full_profile
> +      && !single_succ_edge
> +             (ENTRY_BLOCK_PTR_FOR_FN (cfun))->probability.initialized_p ())
> +    {
> +      error ("probability of edge from entry block not initialized");
> +      err = true;
> +    }
> +
>
>    FOR_EACH_BB_FN (bb, cfun)
>      {
> @@ -5691,6 +5711,22 @@ gimple_verify_flow_info (void)
>
>        stmt = NULL;
>
> +      if (cfun->cfg->full_profile)
> +        {
> +         if (!bb->count.initialized_p ())
> +           {
> +             error ("count of bb %d not initialized", bb->index);
> +             err = true;
> +           }
> +         FOR_EACH_EDGE (e, ei, bb->succs)
> +           if (!e->probability.initialized_p ())
> +             {
> +               error ("probability of edge %d->%d not initialized",
> +                      bb->index, e->dest->index);
> +               err = true;
> +             }
> +        }
> +
>        /* Skip labels on the start of basic block.  */
>        for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>         {
> diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
> index 954b39ae1c6..1d98d96df71 100644
> --- a/gcc/tree-inline.cc
> +++ b/gcc/tree-inline.cc
> @@ -2815,6 +2815,7 @@ initialize_cfun (tree new_fndecl, tree callee_fndecl, profile_count count)
>    init_empty_tree_cfg ();
>
>    profile_status_for_fn (cfun) = profile_status_for_fn (src_cfun);
> +  cfun->cfg->full_profile = src_cfun->cfg->full_profile;
>
>    profile_count num = count;
>    profile_count den = ENTRY_BLOCK_PTR_FOR_FN (src_cfun)->count;
> @@ -4953,6 +4954,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
>    id->src_cfun = DECL_STRUCT_FUNCTION (fn);
>    id->reset_location = DECL_IGNORED_P (fn);
>    id->call_stmt = call_stmt;
> +  cfun->cfg->full_profile &= id->src_cfun->cfg->full_profile;
>
>    /* When inlining into an OpenMP SIMD-on-SIMT loop, arrange for new automatic
>       variables to be added to IFN_GOMP_SIMT_ENTER argument list.  */

  reply	other threads:[~2023-08-24 13:22 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-24 13:14 Jan Hubicka
2023-08-24 13:20 ` Richard Biener [this message]
2023-08-24 15:36   ` Jan Hubicka
2023-08-31  0:15 ` [EXTERNAL] " Eugene Rozenfeld
2023-09-05  7:16 ` [PATCH 16/12] _BitInt profile fixes [PR102989] Jakub Jelinek
2023-10-03 16:56 ` Check that passes do not forget to define profile Andre Vieira (lists)
2023-10-04 11:02   ` Jan Hubicka
2023-10-17 10:29     ` Andre Vieira (lists)
2023-10-17 13:52       ` Jan Hubicka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFiYyc0LXMS1U6JVX8ogJVmrGybPRR75M6L3QefOLtjR9ziB=g@mail.gmail.com' \
    --to=richard.guenther@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).