On Wed, 13 Jan 2016, Tom de Vries wrote:
> Hi,
>
> Consider testcase test.c:
> ...
> struct pgm_slist_t
> {
> struct pgm_slist_t *__restrict next;
> };
>
> void
> fn1 (struct pgm_slist_t p1)
> {
>
> }
> ...
>
> The compilation of the testcase enters into infinite recursion, due to the
> more aggressive restrict support added recently.
>
> The patch fixes this by:-
> - tracking which structs are being traversed when following restrict
> pointers in create_variable_info_for_1, and
> - not following restrict pointers that point to already tracked structs.
>
> Bootstrapped and reg-tested on x86_64.
>
> OK for trunk?
Comments inline (unqouted)
Fix infinite recursion in create_variable_info_for_1
PR tree-optimization/69169
* tree-ssa-structalias.c (handled_struct_type): New hash set.
(create_variable_info_for_1): Use handled_struct_type to keep track
of structs recursed by following restrict pointers.
(intra_create_variable_infos): Allocate and cleanup
handled_struct_type.
* gcc.dg/pr69169.c: New test.
---
gcc/testsuite/gcc.dg/pr69169.c | 13 +++++++++++++
gcc/tree-ssa-structalias.c | 19 ++++++++++++++++---
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/pr69169.c b/gcc/testsuite/gcc.dg/pr69169.c
new file mode 100644
index 0000000..ecf847c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr69169.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+struct pgm_slist_t
+{
+ struct pgm_slist_t *__restrict next;
+};
+
+void
+fn1 (struct pgm_slist_t p1)
+{
+
+}
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index fd98352..72bef07 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5767,6 +5767,12 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)
return false;
}
+/* Hash set used to register structs traversed in create_variable_info_for_1
+ by following restrict pointers. This is needed to prevent infinite
+ recursion. */
+
+hash_set<tree> *handled_struct_type = NULL;
+
A bitmap indexed by TYPE_UID would be cheaper I guess? Maybe not.
At least a hash_set<unsigned> would be ;)
Plase make the above static and GTY((skip)), no need to put it in
GC memory.
/* Create a varinfo structure for NAME and DECL, and add it to VARMAP.
This will also create any varinfo structures necessary for fields
of DECL. DECL is a function parameter if HANDLE_PARAM is set. */
@@ -5851,7 +5857,8 @@ create_variable_info_for_1 (tree decl, const char *name,
bool add_id,
vi->only_restrict_pointers = 1;
if (vi->only_restrict_pointers
&& !type_contains_placeholder_p (TREE_TYPE (decl_type))
- && handle_param)
+ && handle_param
+ && !handled_struct_type->contains (TREE_TYPE (decl_type)))
{
varinfo_t rvi;
tree heapvar = build_fake_var_decl (TREE_TYPE (decl_type));
This path misses to add/remove decl_type to the set. Thus
struct X { struct X * restrict p; } *; would still recurse infinitely?
@@ -5871,6 +5878,8 @@ create_variable_info_for_1 (tree decl, const char *name,
bool add_id,
vi->fullsize = tree_to_uhwi (declsize);
if (fieldstack.length () == 1)
vi->is_full_var = true;
+ if (handle_param)
+ handled_struct_type->add (decl_type);
while here we add it to the set even if in the end we won't create
a fake var decl (and recurse). I think we should add to the set only
if we actually recurse.
for (i = 0, newvi = vi;
fieldstack.iterate (i, &fo);
++i, newvi = vi_next (newvi))
@@ -5902,7 +5911,8 @@ create_variable_info_for_1 (tree decl, const char *name,
bool add_id,
newvi->only_restrict_pointers = fo->only_restrict_pointers;
if (handle_param
&& newvi->only_restrict_pointers
- && !type_contains_placeholder_p (fo->restrict_pointed_type))
+ && !type_contains_placeholder_p (fo->restrict_pointed_type)
+ && !handled_struct_type->contains (fo->restrict_pointed_type))
{
varinfo_t rvi;
tree heapvar = build_fake_var_decl (fo->restrict_pointed_type);
@@ -5921,6 +5931,8 @@ create_variable_info_for_1 (tree decl, const char *name,
bool add_id,
tem->head = vi->id;
}
}
+ if (handle_param)
+ handled_struct_type->remove (decl_type);
return vi;
}
@@ -6065,10 +6077,11 @@ intra_create_variable_infos (struct function *fn)
passed-by-reference argument. */
for (t = DECL_ARGUMENTS (fn->decl); t; t = DECL_CHAIN (t))
{
+ handled_struct_type = hash_set<tree>::create_ggc (4);
As said, please put it in heap memory and eventually just pass it down
as argument to create_variable_info_for_1 avoiding the global variable
(it's just three callers as far as I can see).
varinfo_t p
= create_variable_info_for_1 (t, alias_get_name (t), false, true);
+ handled_struct_type = NULL;
insert_vi_for_tree (t, p);
-
make_param_constraints (p);
}
