Re: [PATCH, 2/2] Handle recursive restrict in function parameter
On Tue, 3 Nov 2015, Tom de Vries wrote:
> On 03/11/15 16:08, Richard Biener wrote:
> > On Tue, 3 Nov 2015, Tom de Vries wrote:
> >
> > > On 01/11/15 19:20, Tom de Vries wrote:
> > > > On 01/11/15 19:03, Tom de Vries wrote:
> > > > > So, the new patch series is:
> > > > >
> > > > >1Rename make_restrict_var_constraints to
> > > > > make_param_constraints
> > > > >2Handle recursive restrict in function parameter
> > > > >
> > > > > I'll repost in reply to this message.
> > > > >
> > > >
> > > > This patch adds handling of all the restrict qualifiers in the type of a
> > > > function parameter.
> > > >
> > >
> > > And reposting an updated version, now that the toplevel parameter in
> > > make_param_constraints has been eliminated.
> >
> > @@ -5195,6 +5197,8 @@ struct fieldoff
> > unsigned may_have_pointers : 1;
> >
> > unsigned only_restrict_pointers : 1;
> > +
> > + varinfo_t restrict_var;
> > };
> >
> > store the varinfo ID here, 'unsigned int restrict_var' which ends
> > up not changing fieldoff size. get_varinfo (restrict_var) will get
> > you the varinfo_t.
>
> Done, attached.
>
> >
> > @@ -5374,6 +5380,19 @@ push_fields_onto_fieldstack (tree type,
> > vec *fieldstack,
> >= (!has_unknown_size
> > && POINTER_TYPE_P (field_type)
> > && TYPE_RESTRICT (field_type));
> > + if (handle_param
> > + && e.only_restrict_pointers
> > + && !type_contains_placeholder_p (TREE_TYPE
> > (field_type)))
> > + {
> > + varinfo_t rvi;
> > + tree heapvar = build_fake_var_decl (TREE_TYPE
> > (field_type));
> > + DECL_EXTERNAL (heapvar) = 1;
> > + rvi = create_variable_info_for_1 (heapvar,
> > "PARM_NOALIAS",
> > + true, true);
> > + rvi->is_restrict_var = 1;
> > + insert_vi_for_tree (heapvar, rvi);
> > + e.restrict_var = rvi;
> > + }
> >
> > hmm, can you delay this to the point we actually will use field-sensitive
> > stuff? That is, until create_variable_info_for_1 decided to use a
> > multi-field variable?
>
> AFAIU your concern is that in the current patch we're creating heapvars that
> may end up being ignored, f.i. if we hit the MAX_FIELDS_FOR_FIELD_SENSITIVE
> threshold?
Yes (or for other reasons).
> > Say, here:
> >
> > + if (handle_param
> > + && newvi->only_restrict_pointers
> > + && fo->restrict_var != NULL)
> > + {
> > + make_constraint_from (newvi, fo->restrict_var->id);
> > + make_param_constraints (fo->restrict_var);
> > + }
> >
> > ? Looks like then you don't need the new field at all.
> >
>
> The build_fake_var_decl call needs TREE_TYPE (field_type), the type the
> restrict pointer field points to.
>
> The field type is no longer available once we've abstracted the struct type
> into a field stack in create_variable_info_for_1.
>
> I think I can postpone the creation of the heapvar till where you suggest in
> create_variable_info_for_1, but I'd still need a means
> to communicate the TREE_TYPE (field_type) from push_fields_onto_fieldstack to
> create_variable_info_for_1.
>
> A simple implementation would be a new field:
> ...
> @@ -5195,6 +5197,8 @@ struct fieldoff
> unsigned may_have_pointers : 1;
>
> unsigned only_restrict_pointers : 1;
> +
> + tree restrict_pointed_type;
> };
> ...
> Which AFAIU will change fieldoff size.
It's ok to change fieldoff size if there is a reason to ;)
Patch is ok along this line.
Thanks,
Richard.
Re: [PATCH, 2/2] Handle recursive restrict in function parameter
On 03/11/15 16:08, Richard Biener wrote:
On Tue, 3 Nov 2015, Tom de Vries wrote:
On 01/11/15 19:20, Tom de Vries wrote:
On 01/11/15 19:03, Tom de Vries wrote:
So, the new patch series is:
1Rename make_restrict_var_constraints to make_param_constraints
2Handle recursive restrict in function parameter
I'll repost in reply to this message.
This patch adds handling of all the restrict qualifiers in the type of a
function parameter.
And reposting an updated version, now that the toplevel parameter in
make_param_constraints has been eliminated.
@@ -5195,6 +5197,8 @@ struct fieldoff
unsigned may_have_pointers : 1;
unsigned only_restrict_pointers : 1;
+
+ varinfo_t restrict_var;
};
store the varinfo ID here, 'unsigned int restrict_var' which ends
up not changing fieldoff size. get_varinfo (restrict_var) will get
you the varinfo_t.
Done, attached.
@@ -5374,6 +5380,19 @@ push_fields_onto_fieldstack (tree type,
vec *fieldstack,
= (!has_unknown_size
&& POINTER_TYPE_P (field_type)
&& TYPE_RESTRICT (field_type));
+ if (handle_param
+ && e.only_restrict_pointers
+ && !type_contains_placeholder_p (TREE_TYPE
(field_type)))
+ {
+ varinfo_t rvi;
+ tree heapvar = build_fake_var_decl (TREE_TYPE
(field_type));
+ DECL_EXTERNAL (heapvar) = 1;
+ rvi = create_variable_info_for_1 (heapvar,
"PARM_NOALIAS",
+ true, true);
+ rvi->is_restrict_var = 1;
+ insert_vi_for_tree (heapvar, rvi);
+ e.restrict_var = rvi;
+ }
hmm, can you delay this to the point we actually will use field-sensitive
stuff? That is, until create_variable_info_for_1 decided to use a
multi-field variable?
AFAIU your concern is that in the current patch we're creating heapvars
that may end up being ignored, f.i. if we hit the
MAX_FIELDS_FOR_FIELD_SENSITIVE threshold?
Say, here:
+ if (handle_param
+ && newvi->only_restrict_pointers
+ && fo->restrict_var != NULL)
+ {
+ make_constraint_from (newvi, fo->restrict_var->id);
+ make_param_constraints (fo->restrict_var);
+ }
? Looks like then you don't need the new field at all.
The build_fake_var_decl call needs TREE_TYPE (field_type), the type the
restrict pointer field points to.
The field type is no longer available once we've abstracted the struct
type into a field stack in create_variable_info_for_1.
I think I can postpone the creation of the heapvar till where you
suggest in create_variable_info_for_1, but I'd still need a means
to communicate the TREE_TYPE (field_type) from
push_fields_onto_fieldstack to create_variable_info_for_1.
A simple implementation would be a new field:
...
@@ -5195,6 +5197,8 @@ struct fieldoff
unsigned may_have_pointers : 1;
unsigned only_restrict_pointers : 1;
+
+ tree restrict_pointed_type;
};
...
Which AFAIU will change fieldoff size.
Thanks,
- Tom
Handle recursive restrict in function parameter
* tree-ssa-structalias.c (struct fieldoff): Add restrict_var field.
(push_fields_onto_fieldstack): Add and handle handle_param parameter.
(create_variable_info_for_1): Add and handle
handle_param parameter. Add extra arg to call to
push_fields_onto_fieldstack. Handle restrict pointer fields.
(create_variable_info_for): Call create_variable_info_for_1 with extra
arg.
(make_param_constraints): Drop restrict_name parameter. Ignore
vi->only_restrict_pointers.
(intra_create_variable_infos): Call create_variable_info_for_1 with
extra arg. Remove restrict handling. Call make_param_constraints with
one less arg.
* gcc.dg/tree-ssa/restrict-7.c: New test.
* gcc.dg/tree-ssa/restrict-8.c: New test.
---
gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c | 12
gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c | 17 ++
gcc/tree-ssa-structalias.c | 91 ++
3 files changed, 84 insertions(+), 36 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c b/gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c
new file mode 100644
index 000..f7a68c7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
+
+int
+f (int *__restrict__ *__restrict__ *__restrict__ a, int *b)
+{
+ *b = 1;
+ ***a = 2;
+ return *b;
+}
+
+/* { dg-final { scan-tree-dump-times "return 1" 1 "fre1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c b/gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c
new file mode 100644
index 000..b0ab164
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c
@@ -0,0 +1
Re: [PATCH, 2/2] Handle recursive restrict in function parameter
On Tue, 3 Nov 2015, Tom de Vries wrote:
> On 01/11/15 19:20, Tom de Vries wrote:
> > On 01/11/15 19:03, Tom de Vries wrote:
> > > So, the new patch series is:
> > >
> > > 1Rename make_restrict_var_constraints to make_param_constraints
> > > 2Handle recursive restrict in function parameter
> > >
> > > I'll repost in reply to this message.
> > >
> >
> > This patch adds handling of all the restrict qualifiers in the type of a
> > function parameter.
> >
>
> And reposting an updated version, now that the toplevel parameter in
> make_param_constraints has been eliminated.
@@ -5195,6 +5197,8 @@ struct fieldoff
unsigned may_have_pointers : 1;
unsigned only_restrict_pointers : 1;
+
+ varinfo_t restrict_var;
};
store the varinfo ID here, 'unsigned int restrict_var' which ends
up not changing fieldoff size. get_varinfo (restrict_var) will get
you the varinfo_t.
@@ -5374,6 +5380,19 @@ push_fields_onto_fieldstack (tree type,
vec *fieldstack,
= (!has_unknown_size
&& POINTER_TYPE_P (field_type)
&& TYPE_RESTRICT (field_type));
+ if (handle_param
+ && e.only_restrict_pointers
+ && !type_contains_placeholder_p (TREE_TYPE
(field_type)))
+ {
+ varinfo_t rvi;
+ tree heapvar = build_fake_var_decl (TREE_TYPE
(field_type));
+ DECL_EXTERNAL (heapvar) = 1;
+ rvi = create_variable_info_for_1 (heapvar,
"PARM_NOALIAS",
+ true, true);
+ rvi->is_restrict_var = 1;
+ insert_vi_for_tree (heapvar, rvi);
+ e.restrict_var = rvi;
+ }
hmm, can you delay this to the point we actually will use field-sensitive
stuff? That is, until create_variable_info_for_1 decided to use a
multi-field variable? Say, here:
+ if (handle_param
+ && newvi->only_restrict_pointers
+ && fo->restrict_var != NULL)
+ {
+ make_constraint_from (newvi, fo->restrict_var->id);
+ make_param_constraints (fo->restrict_var);
+ }
? Looks like then you don't need the new field at all.
Thanks,
Richard.
Re: [PATCH, 2/2] Handle recursive restrict in function parameter
On 01/11/15 19:20, Tom de Vries wrote:
On 01/11/15 19:03, Tom de Vries wrote:
So, the new patch series is:
1Rename make_restrict_var_constraints to make_param_constraints
2Handle recursive restrict in function parameter
I'll repost in reply to this message.
This patch adds handling of all the restrict qualifiers in the type of a
function parameter.
And reposting an updated version, now that the toplevel parameter in
make_param_constraints has been eliminated.
Thanks,
- Tom
Handle recursive restrict in function parameter
* tree-ssa-structalias.c (struct fieldoff): Add restrict_var field.
(push_fields_onto_fieldstack): Add and handle handle_param parameter.
(create_variable_info_for_1): Add and handle
handle_param parameter. Add extra arg to call to
push_fields_onto_fieldstack. Handle restrict pointer fields.
(create_variable_info_for): Call create_variable_info_for_1 with extra
arg.
(make_param_constraints): Drop restrict_name parameter. Ignore
vi->only_restrict_pointers.
(intra_create_variable_infos): Call create_variable_info_for_1 with
extra arg. Remove restrict handling. Call make_param_constraints with
one less arg.
* gcc.dg/tree-ssa/restrict-7.c: New test.
* gcc.dg/tree-ssa/restrict-8.c: New test.
---
gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c | 12
gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c | 17 ++
gcc/tree-ssa-structalias.c | 90 ++
3 files changed, 83 insertions(+), 36 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c b/gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c
new file mode 100644
index 000..f7a68c7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
+
+int
+f (int *__restrict__ *__restrict__ *__restrict__ a, int *b)
+{
+ *b = 1;
+ ***a = 2;
+ return *b;
+}
+
+/* { dg-final { scan-tree-dump-times "return 1" 1 "fre1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c b/gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c
new file mode 100644
index 000..b0ab164
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
+
+struct s
+{
+ int *__restrict__ *__restrict__ pp;
+};
+
+int
+f (struct s s, int *b)
+{
+ *b = 1;
+ **s.pp = 2;
+ return *b;
+}
+
+/* { dg-final { scan-tree-dump-times "return 1" 1 "fre1" } } */
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index ded5a1e..3c65db8 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -307,6 +307,7 @@ static varinfo_t first_or_preceding_vi_for_offset (varinfo_t,
unsigned HOST_WIDE_INT);
static varinfo_t lookup_vi_for_tree (tree);
static inline bool type_can_have_subvars (const_tree);
+static void make_param_constraints (varinfo_t);
/* Pool of variable info structures. */
static object_allocator variable_info_pool
@@ -393,6 +394,7 @@ new_var_info (tree t, const char *name, bool add_id)
return ret;
}
+static varinfo_t create_variable_info_for_1 (tree, const char *, bool, bool);
/* A map mapping call statements to per-stmt variables for uses
and clobbers specific to the call. */
@@ -5195,6 +5197,8 @@ struct fieldoff
unsigned may_have_pointers : 1;
unsigned only_restrict_pointers : 1;
+
+ varinfo_t restrict_var;
};
typedef struct fieldoff fieldoff_s;
@@ -5289,11 +5293,12 @@ field_must_have_pointers (tree t)
OFFSET is used to keep track of the offset in this entire
structure, rather than just the immediately containing structure.
Returns false if the caller is supposed to handle the field we
- recursed for. */
+ recursed for. If HANDLE_PARAM is set, we're handling part of a function
+ parameter. */
static bool
push_fields_onto_fieldstack (tree type, vec *fieldstack,
- HOST_WIDE_INT offset)
+ HOST_WIDE_INT offset, bool handle_param)
{
tree field;
bool empty_p = true;
@@ -5319,7 +5324,7 @@ push_fields_onto_fieldstack (tree type, vec *fieldstack,
|| TREE_CODE (field_type) == UNION_TYPE)
push = true;
else if (!push_fields_onto_fieldstack
- (field_type, fieldstack, offset + foff)
+ (field_type, fieldstack, offset + foff, handle_param)
&& (DECL_SIZE (field)
&& !integer_zerop (DECL_SIZE (field
/* Empty structures may have actual size, like in C++. So
@@ -5340,7 +5345,8 @@ push_fields_onto_fieldstack (tree type, vec *fieldstack,
if (!pair
&& offset + foff != 0)
{
- fieldoff_s e = {0, offset + foff, false, false, false, false};
+ fieldoff_s e = {0, offset + foff, false, false, false, false,
+NULL};
pair = fieldstack->safe_push (e);
}
@@ -5374,6 +5380,19 @@ push_fields_onto_fieldstack (tree type, ve
[PATCH, 2/2] Handle recursive restrict in function parameter
On 01/11/15 19:03, Tom de Vries wrote:
So, the new patch series is:
1Rename make_restrict_var_constraints to make_param_constraints
2Handle recursive restrict in function parameter
I'll repost in reply to this message.
This patch adds handling of all the restrict qualifiers in the type of a
function parameter.
Thanks,
- Tom
Handle recursive restrict in function parameter
* tree-ssa-structalias.c (struct fieldoff): Add restrict_var field.
(push_fields_onto_fieldstack): Add and handle handle_param parameter.
(create_variable_info_for_1): Add and handle
handle_param parameter. Add extra arg to call to
push_fields_onto_fieldstack. Handle restrict pointer fields.
(create_variable_info_for): Call create_variable_info_for_1 with extra
arg.
(make_param_constraints): Drop restrict_name parameter. Ignore
vi->only_restrict_pointers.
(intra_create_variable_infos): Call create_variable_info_for_1 with
extra arg. Remove restrict handling. Call make_param_constraints with
one less arg.
* gcc.dg/tree-ssa/restrict-7.c: New test.
* gcc.dg/tree-ssa/restrict-8.c: New test.
---
gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c | 12 +
gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c | 17 ++
gcc/tree-ssa-structalias.c | 87 ++
3 files changed, 82 insertions(+), 34 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c b/gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c
new file mode 100644
index 000..f7a68c7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
+
+int
+f (int *__restrict__ *__restrict__ *__restrict__ a, int *b)
+{
+ *b = 1;
+ ***a = 2;
+ return *b;
+}
+
+/* { dg-final { scan-tree-dump-times "return 1" 1 "fre1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c b/gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c
new file mode 100644
index 000..b0ab164
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
+
+struct s
+{
+ int *__restrict__ *__restrict__ pp;
+};
+
+int
+f (struct s s, int *b)
+{
+ *b = 1;
+ **s.pp = 2;
+ return *b;
+}
+
+/* { dg-final { scan-tree-dump-times "return 1" 1 "fre1" } } */
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index ea34764..f4e9b0a 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -320,6 +320,7 @@ static varinfo_t first_or_preceding_vi_for_offset (varinfo_t,
unsigned HOST_WIDE_INT);
static varinfo_t lookup_vi_for_tree (tree);
static inline bool type_can_have_subvars (const_tree);
+static void make_param_constraints (varinfo_t, bool);
/* Pool of variable info structures. */
static object_allocator variable_info_pool
@@ -406,6 +407,7 @@ new_var_info (tree t, const char *name, bool add_id)
return ret;
}
+static varinfo_t create_variable_info_for_1 (tree, const char *, bool, bool);
/* A map mapping call statements to per-stmt variables for uses
and clobbers specific to the call. */
@@ -5208,6 +5210,8 @@ struct fieldoff
unsigned may_have_pointers : 1;
unsigned only_restrict_pointers : 1;
+
+ varinfo_t restrict_var;
};
typedef struct fieldoff fieldoff_s;
@@ -5302,11 +5306,12 @@ field_must_have_pointers (tree t)
OFFSET is used to keep track of the offset in this entire
structure, rather than just the immediately containing structure.
Returns false if the caller is supposed to handle the field we
- recursed for. */
+ recursed for. If HANDLE_PARAM is set, we're handling part of a function
+ parameter. */
static bool
push_fields_onto_fieldstack (tree type, vec *fieldstack,
- HOST_WIDE_INT offset)
+ HOST_WIDE_INT offset, bool handle_param)
{
tree field;
bool empty_p = true;
@@ -5332,7 +5337,7 @@ push_fields_onto_fieldstack (tree type, vec *fieldstack,
|| TREE_CODE (field_type) == UNION_TYPE)
push = true;
else if (!push_fields_onto_fieldstack
- (field_type, fieldstack, offset + foff)
+ (field_type, fieldstack, offset + foff, handle_param)
&& (DECL_SIZE (field)
&& !integer_zerop (DECL_SIZE (field
/* Empty structures may have actual size, like in C++. So
@@ -5353,7 +5358,8 @@ push_fields_onto_fieldstack (tree type, vec *fieldstack,
if (!pair
&& offset + foff != 0)
{
- fieldoff_s e = {0, offset + foff, false, false, false, false};
+ fieldoff_s e = {0, offset + foff, false, false, false, false,
+NULL};
pair = fieldstack->safe_push (e);
}
@@ -5387,6 +5393,19 @@ push_fields_onto_fieldstack (tree type, vec *fieldstack,
= (!has_unknown_size
&& POINTER_TYPE_P (field_type)
&& TYPE_RESTRICT (field_type));
+ if (handle_param
+ &&
