From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id AA4C33858434 for ; Tue, 3 Oct 2023 16:56:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AA4C33858434 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E39B6C15; Tue, 3 Oct 2023 09:56:56 -0700 (PDT) Received: from [10.1.33.142] (E121495.arm.com [10.1.33.142]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CE78D3F762; Tue, 3 Oct 2023 09:56:17 -0700 (PDT) Content-Type: multipart/mixed; boundary="------------fS3CA5U2JXhu6TX6QJ5Gst3z" Message-ID: <101e48f0-6af5-96d4-8389-be04cdf4cf3c@arm.com> Date: Tue, 3 Oct 2023 17:56:16 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: Check that passes do not forget to define profile Content-Language: en-US To: gcc-patches@gcc.gnu.org, Jan Hubicka References: From: "Andre Vieira (lists)" In-Reply-To: X-Spam-Status: No, score=-14.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,NICE_REPLY_A,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: This is a multi-part message in MIME format. --------------fS3CA5U2JXhu6TX6QJ5Gst3z Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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; > } > > /* 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. */ --------------fS3CA5U2JXhu6TX6QJ5Gst3z Content-Type: text/plain; charset=UTF-8; name="sese_count.patch" Content-Disposition: attachment; filename="sese_count.patch" Content-Transfer-Encoding: base64 ZGlmZiAtLWdpdCBhL2djYy90cmVlLWNmZy5jYyBiL2djYy90cmVlLWNmZy5jYwppbmRleCBm ZmFiNzUxOGIxNTY4YjU4ZTYxMGUyNmZlYjllM2NhYjE4ZGRiM2MyLi4zMmZjNDdhZTY4MzE2 NGJmOGZhYzQ3N2ZiZTZlMmM5OTgzODJlNzU0IDEwMDY0NAotLS0gYS9nY2MvdHJlZS1jZmcu Y2MKKysrIGIvZ2NjL3RyZWUtY2ZnLmNjCkBAIC04MTYwLDExICs4MTYwLDE1IEBAIG1vdmVf c2VzZV9yZWdpb25fdG9fZm4gKHN0cnVjdCBmdW5jdGlvbiAqZGVzdF9jZnVuLCBiYXNpY19i bG9jayBlbnRyeV9iYiwKICAgYmIgPSBjcmVhdGVfZW1wdHlfYmIgKGVudHJ5X3ByZWRbMF0p OwogICBpZiAoY3VycmVudF9sb29wcykKICAgICBhZGRfYmJfdG9fbG9vcCAoYmIsIGxvb3Ap OworICBwcm9maWxlX2NvdW50IGNvdW50ID0gcHJvZmlsZV9jb3VudDo6emVybyAoKTsKICAg Zm9yIChpID0gMDsgaSA8IG51bV9lbnRyeV9lZGdlczsgaSsrKQogICAgIHsKICAgICAgIGUg PSBtYWtlX2VkZ2UgKGVudHJ5X3ByZWRbaV0sIGJiLCBlbnRyeV9mbGFnW2ldKTsKICAgICAg IGUtPnByb2JhYmlsaXR5ID0gZW50cnlfcHJvYltpXTsKKyAgICAgIGlmIChlLT5jb3VudCAo KS5pbml0aWFsaXplZF9wICgpKQorCWNvdW50ICs9IGUtPmNvdW50ICgpOwogICAgIH0KKyAg YmItPmNvdW50ID0gY291bnQ7CiAKICAgZm9yIChpID0gMDsgaSA8IG51bV9leGl0X2VkZ2Vz OyBpKyspCiAgICAgewo= --------------fS3CA5U2JXhu6TX6QJ5Gst3z--