Hi, > On 8 Jan 2026, at 5:37 am, Jan Hubička <[email protected]> wrote: > > External email: Use caution opening links or attachments > Hello, > sorry for being late here. > The logic being doing fixup_cfg is the following. While passes like > pure-const does change the function flags and technically breaks the CFG in > all functions calling the function which has been discovered pure or const, > they can not call fixup_cfg on all those callers since it would be quadratic > and also the callers does not need to be load in memory. > > For this reason, we call fixup_cfg at the beginning of each block of local > optimisations, where is a chance that pure/const or other IPA pass changed > something relevant. > This is the local optimisation passes (done in early opts) since first run of > pure-const is there; then it is in the late optimisations for the same > reason. Fixup after IPA passes happens in the inliner. > > ipa-pure-const/modref executed fixup cfg on function being compiled (in local > mode) so later optimisations can continue. > > Now, what goes wrong here is that we added fnsplit as a subpass of autofdo. > This is the same as tree-profile. Within tree-profile we fixup to fix > precisely the same issue > /* Local pure-const may imply need to fixup the cfg. */ > if (gimple_has_body_p (node->decl) > && (execute_fixup_cfg () & TODO_cleanup_cfg)) > cleanup_tree_cfg (); > So it would be more consistent if auto-profile did its own fixups too. (We > will get more fixups with -fauto-profile, but less without, which is probably > an overall win).
Attached patch does it in similar way. Is this OK? Thanks, Kugan > > Honza > > On Wed, Jan 7, 2026 at 1:18 AM Andrew Pinski <[email protected]> > wrote: > On Sun, Dec 21, 2025 at 9:56 PM Kugan Vivekanandarajah > <[email protected]> wrote: > > > > Hi, > > Thanks for the review. > > > > > On 21 Dec 2025, at 8:37 am, Andrew Pinski > > > <[email protected]> wrote: > > > > > > External email: Use caution opening links or attachments > > > > > > > > > On Sun, Dec 14, 2025 at 1:18 PM Kugan Vivekanandarajah > > > <[email protected]> wrote: > > >> > > >> Hi, > > >> > > >> This patch shows up with AutoFDO but IMO this is not limited to AutoFDO. > > >> > > >> The bug is a stale Virtual SSA on calls to functions that have > > >> been marked const or pure. > > >> > > >> pure_const pass analyzes function rocksdb::y::y() and determines it has > > >> no side > > >> effects and marks it as const. > > >> > > >> At this point, existing call sites to y::y() in other functions still > > >> have: > > >> # .MEM_12 = VDEF <.MEM_11> rocksdb::y::y (&l, _9); > > >> > > >> Exact ICE is: > > >> cache/lru_cache.cc:921:1: error: virtual definition of statement not up > > >> to date > > >> # .MEM_12 = VDEF <.MEM_11> > > >> rocksdb::y::y (&l, _9); > > >> during GIMPLE pass: feedback_fnsplit > > >> dump file: lru_cache_reduced.ii.076t.feedback_fnsplit1 > > >> cache/lru_cache.cc:921:1: internal compiler error: verify_ssa failed > > >> 0x26f4073 internal_error(char const*, ...) > > >> ../../gcc-ro/gcc/diagnostic-global-context.cc:787 > > >> 0x16f91df verify_ssa(bool, bool) > > >> ../../gcc-ro/gcc/tree-ssa.cc:1203 > > >> 0x12f0643 execute_function_todo > > >> ../../gcc-ro/gcc/passes.cc:2104 > > >> 0x12f0beb execute_todo > > >> ../../gcc-ro/gcc/passes.cc:2149 > > >> > > >> > > >> In ipa-pure-const, we do execute_fixup_cfg only for the changed. IMO, we > > >> should call for all the functions that has this function called. Calling > > >> fixup for all also fixes this ICE. > > >> > > >> Later in feedback_fnsplit SSA verification fails. Added fixup > > >> after execute_feedback_split_functions also fixes this ICE. > > > This looks ok to me except I would add a comment on why you need the > > > fixup and no reason for the todo variable either: > > > > > > /* Update virtual SSA as some functions could have changed to be > > > pure/const. */ > > > retval |= execute_fixup_cfg (); > > > return retval; > > > > Added the comments. Is this OK now? > > Ok. (was waiting for the official announcement for being appointed as > global reviewer before sending approval for this). > > Thanks, > Andrew > > > > > Thanks, > > Kugan > > > > > > > > > > > > Thanks, > > > Andrew > > > > > >> > > >> gcc/ChangeLog: > > >> > > >> 2025-12-08 Kugan Vivekanandarajah <[email protected]> > > >> > > >> * ipa-split.cc (execute_feedback_split_functions): Call > > >> * execute_fixup_cfg. > > > >
0001-Subject-PATCH-Bug-gcov-profile-123019-V2-Fix-Virtual.patch
Description: 0001-Subject-PATCH-Bug-gcov-profile-123019-V2-Fix-Virtual.patch
