Hi Jan,
These new checks are too strong for AutoFDO. For example, the edge
probabilities are not guaranteed to be initialized (see
afdo_calculate_branch_prob).
This currently breaks autoprofiledbootstrap build.
I suggest removing
cfun->cfg->full_profile = true;
from auto-profile.cc.
Eugene
-----Original Message-----
From: Gcc-patches <gcc-patches-bounces+erozen=microsoft....@gcc.gnu.org> On
Behalf Of Jan Hubicka via Gcc-patches
Sent: Thursday, August 24, 2023 6:15 AM
To: gcc-patches@gcc.gnu.org
Subject: [EXTERNAL] Check that passes do not forget to define profile
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;
}
/* 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. */
