https://gcc.gnu.org/g:b4de9d73b47972bc89df288997c8f3a3862bc1b1
commit r17-4887-gb4de9d73b47972bc89df288997c8f3a3862bc1b1 Author: Richard Biener <[email protected]> Date: Mon Oct 5 10:07:06 2026 +0200 tree-optimization/127693 - missed FRE in loop #3 When walk_non_aliased_vuses translated the reference it made get_continuation_for_phi fail when reaching a region already recorded in the visited bitmap. This was to avoid trusting regions that were skipped when walking with the original reference (PR54498). But this also causes failure when two PHI argument paths merge before reaching the dominating definition since the second walk then runs into the region recorded by the first walk with the same reference. Instead of aborting, clear the visited bitmap when the reference is translated. This ensures all regions recorded in it were verified with the current reference and makes the abort_on_visited flag unnecessary. This is simpler and cheaper than attempting to set up secondary visited sets esp. since translations are uncommon. For those we now eventually visit ops multiple times though (counted against the walking budget). It's worth keeping patch #2 to avoid redundant walks. PR tree-optimization/127693 * tree-ssa-alias.h (get_continuation_for_phi): Remove abort_on_visited argument. * tree-ssa-alias.cc (maybe_skip_until): Likewise. Always succeed when reaching an already visited region. (get_continuation_for_phi): Remove abort_on_visited argument. (walk_non_aliased_vuses): Clear the visited bitmap when translating the reference instead of tracking whether we translated. * ipa-prop.cc (determine_known_aggregate_parts): Adjust. * tree-ssa-pre.cc (translate_vuse_through_block): Likewise. * gcc.dg/tree-ssa/ssa-fre-118.c: New testcase. Diff: --- gcc/ipa-prop.cc | 2 +- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-118.c | 22 ++++++++++++++++++++++ gcc/tree-ssa-alias.cc | 27 ++++++++++++--------------- gcc/tree-ssa-alias.h | 2 +- gcc/tree-ssa-pre.cc | 2 +- 5 files changed, 37 insertions(+), 18 deletions(-) diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc index 3f9f45b02c89..fb374fb53c91 100644 --- a/gcc/ipa-prop.cc +++ b/gcc/ipa-prop.cc @@ -2278,7 +2278,7 @@ determine_known_aggregate_parts (struct ipa_func_body_info *fbi, { dom_vuse = get_continuation_for_phi (phi, &r, true, fbi->aa_walk_budget, - &visited, false, NULL, NULL); + &visited, NULL, NULL); continue; } diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-118.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-118.c new file mode 100644 index 000000000000..42a58a02ee3b --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-118.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-fre1" } */ + +struct S { int a, b; } s, t; + +int f (int c, int d, float *fp) +{ + s.a = 1; + if (c) + { + *fp = 1.0f; + if (d) + fp[1] = 2.0f; + } + /* The lookup of t.a is translated to s.a here. The walk over the + PHI merging the three paths then visits the path through the + outer if twice which must not cause the walk to fail. */ + t = s; + return t.a; +} + +/* { dg-final { scan-tree-dump "return 1;" "fre1" } } */ diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc index de13ba3bcb6f..a8196d116355 100644 --- a/gcc/tree-ssa-alias.cc +++ b/gcc/tree-ssa-alias.cc @@ -3759,7 +3759,7 @@ ref_can_have_store_data_races (tree ref) static bool maybe_skip_until (gimple *phi, tree &target, basic_block target_bb, ao_ref *ref, tree vuse, bool tbaa_p, unsigned int &limit, - bitmap *visited, bool abort_on_visited, + bitmap *visited, void *(*translate)(ao_ref *, tree, void *, translate_flags *), bool (*is_backedge)(edge, void *), translate_flags disambiguate_only, @@ -3801,10 +3801,9 @@ maybe_skip_until (gimple *phi, tree &target, basic_block target_bb, { /* An already visited PHI node ends the walk successfully. */ if (bitmap_bit_p (*visited, SSA_NAME_VERSION (PHI_RESULT (phi)))) - return !abort_on_visited; + return true; vuse = get_continuation_for_phi (phi, ref, tbaa_p, limit, - visited, abort_on_visited, - translate, data, is_backedge, + visited, translate, data, is_backedge, disambiguate_only); if (!vuse) return false; @@ -3833,7 +3832,7 @@ maybe_skip_until (gimple *phi, tree &target, basic_block target_bb, if (gimple_bb (def_stmt) != bb) { if (!bitmap_set_bit (*visited, SSA_NAME_VERSION (vuse))) - return !abort_on_visited; + return true; bb = gimple_bb (def_stmt); } vuse = gimple_vuse (def_stmt); @@ -3864,7 +3863,6 @@ cmp_intpair (const void *a_, const void *b_) tree get_continuation_for_phi (gphi *phi, ao_ref *ref, bool tbaa_p, unsigned int &limit, bitmap *visited, - bool abort_on_visited, void *(*translate)(ao_ref *, tree, void *, translate_flags *), void *data, @@ -3923,13 +3921,10 @@ get_continuation_for_phi (gphi *phi, ao_ref *ref, bool tbaa_p, if (arg1 == arg0) ; else if (i > 0 && args[i].first == args[i-1].first) - /* Already visited paths can be skipped - we otherwise falsely - abort_on_visited. */ + /* Already walked paths can be skipped. */ ; else if (! maybe_skip_until (phi, arg0, dom, ref, arg1, tbaa_p, - limit, visited, - abort_on_visited, - translate, is_backedge, + limit, visited, translate, is_backedge, /* Do not valueize when walking over backedges. */ !args[i].second @@ -3980,7 +3975,6 @@ walk_non_aliased_vuses (ao_ref *ref, tree vuse, bool tbaa_p, { bitmap visited = NULL; void *res; - bool translated = false; timevar_push (TV_ALIAS_STMT_WALK); @@ -4014,7 +4008,7 @@ walk_non_aliased_vuses (ao_ref *ref, tree vuse, bool tbaa_p, break; else if (gphi *phi = dyn_cast <gphi *> (def_stmt)) vuse = get_continuation_for_phi (phi, ref, tbaa_p, limit, - &visited, translated, translate, data, + &visited, translate, data, is_backedge); else { @@ -4039,8 +4033,11 @@ walk_non_aliased_vuses (ao_ref *ref, tree vuse, bool tbaa_p, /* Lookup succeeded. */ else if (res != NULL) break; - /* Translation succeeded, continue walking. */ - translated = translated || disambiguate_only == TR_TRANSLATE; + /* Translation succeeded, continue walking. Regions skipped + so far were only verified to not clobber the original ref, + so forget about them. */ + if (disambiguate_only == TR_TRANSLATE && visited) + bitmap_clear (visited); } vuse = gimple_vuse (def_stmt); } diff --git a/gcc/tree-ssa-alias.h b/gcc/tree-ssa-alias.h index ccac9c539bb5..ac88b86df348 100644 --- a/gcc/tree-ssa-alias.h +++ b/gcc/tree-ssa-alias.h @@ -152,7 +152,7 @@ extern bool ref_can_have_store_data_races (tree); enum translate_flags { TR_TRANSLATE, TR_VALUEIZE_AND_DISAMBIGUATE, TR_DISAMBIGUATE }; extern tree get_continuation_for_phi (gphi *, ao_ref *, bool, - unsigned int &, bitmap *, bool, + unsigned int &, bitmap *, void *(*)(ao_ref *, tree, void *, translate_flags *), void *, diff --git a/gcc/tree-ssa-pre.cc b/gcc/tree-ssa-pre.cc index 478422518810..f1bd89509833 100644 --- a/gcc/tree-ssa-pre.cc +++ b/gcc/tree-ssa-pre.cc @@ -1444,7 +1444,7 @@ translate_vuse_through_block (vec<vn_reference_op_s> operands, non-clobbering statements. */ unsigned int cnt = param_sccvn_max_alias_queries_per_access; vuse = get_continuation_for_phi (phi, &ref, true, - cnt, &visited, false, NULL, NULL); + cnt, &visited, NULL, NULL); if (visited) BITMAP_FREE (visited); }
