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.
Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
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.
---
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(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-118.c
diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
index 3f9f45b02c8..fb374fb53c9 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 00000000000..42a58a02ee3
--- /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 de13ba3bcb6..a8196d11635 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 ccac9c539bb..ac88b86df34 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 47842251881..f1bd8950983 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);
}
--
2.51.0
