public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com>
To: gcc-patches@gcc.gnu.org, Jan Hubicka <hubicka@ucw.cz>
Subject: Re: Check that passes do not forget to define profile
Date: Tue, 3 Oct 2023 17:56:16 +0100	[thread overview]
Message-ID: <101e48f0-6af5-96d4-8389-be04cdf4cf3c@arm.com> (raw)
In-Reply-To: <ZOdXuCEkw7On4xBo@kam.mff.cuni.cz>

[-- Attachment #1: Type: text/plain, Size: 7960 bytes --]

Hi Honza,

My current patch set for AArch64 VLA omp codegen started failing on 
gcc.dg/gomp/pr87898.c after this. I traced it back to 
'move_sese_region_to_fn' in tree/cfg.cc not setting count for the bb 
created.

I was able to 'fix' it locally by setting the count of the new bb to the 
accumulation of e->count () of all the entry_endges (if initialized). 
I'm however not even close to certain that's the right approach, 
attached patch for illustration.

Kind regards,
Andre

On 24/08/2023 14:14, Jan Hubicka via Gcc-patches 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.
> 
> 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;
>   }
>   \f
>   /* 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.  */

[-- Attachment #2: sese_count.patch --]
[-- Type: text/plain, Size: 716 bytes --]

diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index ffab7518b1568b58e610e26feb9e3cab18ddb3c2..32fc47ae683164bf8fac477fbe6e2c998382e754 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -8160,11 +8160,15 @@ move_sese_region_to_fn (struct function *dest_cfun, basic_block entry_bb,
   bb = create_empty_bb (entry_pred[0]);
   if (current_loops)
     add_bb_to_loop (bb, loop);
+  profile_count count = profile_count::zero ();
   for (i = 0; i < num_entry_edges; i++)
     {
       e = make_edge (entry_pred[i], bb, entry_flag[i]);
       e->probability = entry_prob[i];
+      if (e->count ().initialized_p ())
+	count += e->count ();
     }
+  bb->count = count;
 
   for (i = 0; i < num_exit_edges; i++)
     {

  parent reply	other threads:[~2023-10-03 16:56 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
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 ` Andre Vieira (lists) [this message]
2023-10-04 11:02   ` Check that passes do not forget to define profile 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=101e48f0-6af5-96d4-8389-be04cdf4cf3c@arm.com \
    --to=andre.simoesdiasvieira@arm.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).