public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, 1/2] Add handle_param parameter to create_variable_info_for_1
@ 2015-10-26 11:23 Tom de Vries
  2015-10-26 13:46 ` Richard Biener
  0 siblings, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-10-26 11:23 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 480 bytes --]

Hi,

this no-functional-changes patch copies the restrict var declaration 
code from intra_create_variable_infos to create_variable_info_for_1.

The code was copied rather than moved, since in fipa-pta mode the 
varinfo p for the parameter t may already exist due to 
create_function_info_for, in which case we're not calling 
create_variable_info_for_1 to set p, meaning the copied code won't get 
triggered.

Bootstrapped and reg-tested on x86_64.

OK for trunk?

Thanks,
- Tom

[-- Attachment #2: 0001-Add-handle_param-parameter-to-create_variable_info_f.patch --]
[-- Type: text/x-patch, Size: 3902 bytes --]

Add handle_param parameter to create_variable_info_for_1

2015-10-26  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (struct variable_info): Add
	restrict_pointed_var field.
	(create_variable_info_for_1): Add and handle handle_param parameter.
	(create_variable_info_for): Call create_variable_info_for_1 with extra
	arg.
	(intra_create_variable_infos): Same.  Handle case that
	p->restrict_pointed_var is unequal zero.
---
 gcc/tree-ssa-structalias.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 63a3d02..3fdad3a 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -272,6 +272,9 @@ struct variable_info
   /* True if this field has only restrict qualified pointers.  */
   unsigned int only_restrict_pointers : 1;
 
+  /* The id of the pointed-to restrict var in case only_restrict_pointers.  */
+  unsigned int restrict_pointed_var;
+
   /* True if this represents a heap var created for a restrict qualified
      pointer.  */
   unsigned int is_restrict_var : 1;
@@ -5608,10 +5611,10 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)
 
 /* 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.  */
+   of DECL.  DECL is a function parameter if HANDLE_PARAM is set.  */
 
 static varinfo_t
-create_variable_info_for_1 (tree decl, const char *name)
+create_variable_info_for_1 (tree decl, const char *name, bool handle_param)
 {
   varinfo_t vi, newvi;
   tree decl_type = TREE_TYPE (decl);
@@ -5687,6 +5690,17 @@ create_variable_info_for_1 (tree decl, const char *name)
       if (POINTER_TYPE_P (TREE_TYPE (decl))
 	  && TYPE_RESTRICT (TREE_TYPE (decl)))
 	vi->only_restrict_pointers = 1;
+      if (vi->only_restrict_pointers
+	  && handle_param)
+	{
+	  varinfo_t rvi;
+	  tree heapvar = build_fake_var_decl (TREE_TYPE (decl_type));
+	  DECL_EXTERNAL (heapvar) = 1;
+	  rvi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", false);
+	  rvi->is_restrict_var = 1;
+	  insert_vi_for_tree (heapvar, rvi);
+	  vi->restrict_pointed_var = rvi->id;
+	}
       fieldstack.release ();
       return vi;
     }
@@ -5738,7 +5752,7 @@ create_variable_info_for_1 (tree decl, const char *name)
 static unsigned int
 create_variable_info_for (tree decl, const char *name)
 {
-  varinfo_t vi = create_variable_info_for_1 (decl, name);
+  varinfo_t vi = create_variable_info_for_1 (decl, name, false);
   unsigned int id = vi->id;
 
   insert_vi_for_tree (decl, vi);
@@ -5880,7 +5894,8 @@ intra_create_variable_infos (struct function *fn)
       varinfo_t p = lookup_vi_for_tree (t);
       if (p == NULL)
 	{
-	  p = create_variable_info_for_1 (t, alias_get_name (t));
+	  p = create_variable_info_for_1 (t, alias_get_name (t),
+					  recursive_restrict_p);
 	  insert_vi_for_tree (t, p);
 	}
 
@@ -5890,12 +5905,17 @@ intra_create_variable_infos (struct function *fn)
 	 in the first/last subfield of the object.  */
       if (recursive_restrict_p)
 	{
-	  varinfo_t vi;
-	  tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
-	  DECL_EXTERNAL (heapvar) = 1;
-	  vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS");
-	  vi->is_restrict_var = 1;
-	  insert_vi_for_tree (heapvar, vi);
+	  varinfo_t vi = get_varinfo (p->restrict_pointed_var);
+	  if (vi == NULL)
+	    {
+	      tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
+	      DECL_EXTERNAL (heapvar) = 1;
+	      vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", false);
+	      vi->is_restrict_var = 1;
+	      insert_vi_for_tree (heapvar, vi);
+	      p->restrict_pointed_var = vi->id;
+	      p->only_restrict_pointers = 1;
+	    }
 	  make_constraint_from (p, vi->id);
 	  make_restrict_var_constraints (vi);
 	  continue;
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 1/2] Add handle_param parameter to create_variable_info_for_1
  2015-10-26 11:23 [PATCH, 1/2] Add handle_param parameter to create_variable_info_for_1 Tom de Vries
@ 2015-10-26 13:46 ` Richard Biener
  2015-10-26 16:26   ` Tom de Vries
  0 siblings, 1 reply; 35+ messages in thread
From: Richard Biener @ 2015-10-26 13:46 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Richard Biener, gcc-patches

On Mon, Oct 26, 2015 at 12:22 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> Hi,
>
> this no-functional-changes patch copies the restrict var declaration code
> from intra_create_variable_infos to create_variable_info_for_1.
>
> The code was copied rather than moved, since in fipa-pta mode the varinfo p
> for the parameter t may already exist due to create_function_info_for, in
> which case we're not calling create_variable_info_for_1 to set p, meaning
> the copied code won't get triggered.
>
> Bootstrapped and reg-tested on x86_64.
>
> OK for trunk?

@@ -272,6 +272,9 @@ struct variable_info
   /* True if this field has only restrict qualified pointers.  */
   unsigned int only_restrict_pointers : 1;

+  /* The id of the pointed-to restrict var in case only_restrict_pointers.  */
+  unsigned int restrict_pointed_var;
+
   /* True if this represents a heap var created for a restrict qualified
      pointer.  */
   unsigned int is_restrict_var : 1;
@@ -5608,10 +5611,10 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)


Please don't split the  bitfield like that.  Note that variable_info
is kept as small as
possible because there may be a _lot_ of them.  Is it really necessary to have
this info be persistent?  Why does create_variable_info_for_1 only handle
the single-field case?  That is, I expected this to be handled by c_v_r_f_1
fully.

Richard.





> Thanks,
> - Tom

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 1/2] Add handle_param parameter to create_variable_info_for_1
  2015-10-26 13:46 ` Richard Biener
@ 2015-10-26 16:26   ` Tom de Vries
  2015-10-27  7:59     ` [PATCH, PR67742] Handle restrict struct fields recursively Tom de Vries
  0 siblings, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-10-26 16:26 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2026 bytes --]

On 26/10/15 14:42, Richard Biener wrote:
> On Mon, Oct 26, 2015 at 12:22 PM, Tom de Vries<Tom_deVries@mentor.com>  wrote:
>> >Hi,
>> >
>> >this no-functional-changes patch copies the restrict var declaration code
>> >from intra_create_variable_infos to create_variable_info_for_1.
>> >
>> >The code was copied rather than moved, since in fipa-pta mode the varinfo p
>> >for the parameter t may already exist due to create_function_info_for, in
>> >which case we're not calling create_variable_info_for_1 to set p, meaning
>> >the copied code won't get triggered.
>> >
>> >Bootstrapped and reg-tested on x86_64.
>> >
>> >OK for trunk?
> @@ -272,6 +272,9 @@ struct variable_info
>     /* True if this field has only restrict qualified pointers.  */
>     unsigned int only_restrict_pointers : 1;
>
> +  /* The id of the pointed-to restrict var in case only_restrict_pointers.  */
> +  unsigned int restrict_pointed_var;
> +
>     /* True if this represents a heap var created for a restrict qualified
>        pointer.  */
>     unsigned int is_restrict_var : 1;
> @@ -5608,10 +5611,10 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)
>
>
> Please don't split the  bitfield like that.

Ah, indeed.

> Note that variable_info
> is kept as small as
> possible because there may be a_lot_  of them.  Is it really necessary to have
> this info be persistent?

The info is only needed for a short period, roughly the period of an 
invocation of intra_create_variable_infos.

I've removed the field, and now the info is stored in a hash_map.

>  Why does create_variable_info_for_1 only handle
> the single-field case?  That is, I expected this to be handled by c_v_r_f_1
> fully.
>

Yep, that's the goal of PR67742. I've written a patch in an earlier 
version of the patch series that implements that, I'm currently porting 
that patch to this patch series. I'll post asap.

I decided to post these two patches which do not present a full 
solution, since they are already an improvement over the current situation.

Thanks,
- Tom

[-- Attachment #2: 0001-Add-handle_param-parameter-to-create_variable_info_f.patch --]
[-- Type: text/x-patch, Size: 4818 bytes --]

Add handle_param parameter to create_variable_info_for_1

2015-10-26  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (restrict_pointed_var): New static variable.
	* (insert_restrict_pointed_var, lookup_restrict_pointed_var): New
	function.
	(create_variable_info_for_1): Add and handle handle_param parameter.
	(create_variable_info_for): Call create_variable_info_for_1 with extra
	arg.
	(intra_create_variable_infos): Same.  Handle case that
	lookup_restrict_pointed_var (p) is not NULL.
---
 gcc/tree-ssa-structalias.c | 73 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 63 insertions(+), 10 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 63a3d02..f1325e0 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5606,12 +5606,45 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)
   return false;
 }
 
+/* Map from restrict pointer variable info to restrict var variable info.  */
+
+static hash_map<varinfo_t, varinfo_t> *restrict_pointed_var = NULL;
+
+/* Insert VI2 as the restrict var for VI in the restrict_pointed_var map.  */
+
+static void
+insert_restrict_pointed_var (varinfo_t vi, varinfo_t vi2)
+{
+  if (restrict_pointed_var == NULL)
+  restrict_pointed_var = new hash_map<varinfo_t, varinfo_t>;
+
+  bool mapped = restrict_pointed_var->put (vi, vi2);
+  gcc_assert (!mapped);
+}
+
+/* Find the restrict var for restrict pointer VI in the restrict_pointed_var
+   map.  If VI does not exist in the map, return NULL, otherwise, return the
+   varinfo we found.  */
+
+static varinfo_t
+lookup_restrict_pointed_var (varinfo_t vi)
+{
+  if (restrict_pointed_var == NULL)
+    return NULL;
+  varinfo_t *slot = restrict_pointed_var->get (vi);
+  if (slot == NULL)
+    return NULL;
+
+  return *slot;
+}
+
+
 /* 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.  */
+   of DECL.  DECL is a function parameter if HANDLE_PARAM is set.  */
 
 static varinfo_t
-create_variable_info_for_1 (tree decl, const char *name)
+create_variable_info_for_1 (tree decl, const char *name, bool handle_param)
 {
   varinfo_t vi, newvi;
   tree decl_type = TREE_TYPE (decl);
@@ -5687,6 +5720,17 @@ create_variable_info_for_1 (tree decl, const char *name)
       if (POINTER_TYPE_P (TREE_TYPE (decl))
 	  && TYPE_RESTRICT (TREE_TYPE (decl)))
 	vi->only_restrict_pointers = 1;
+      if (vi->only_restrict_pointers
+	  && handle_param)
+	{
+	  varinfo_t rvi;
+	  tree heapvar = build_fake_var_decl (TREE_TYPE (decl_type));
+	  DECL_EXTERNAL (heapvar) = 1;
+	  rvi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", false);
+	  rvi->is_restrict_var = 1;
+	  insert_vi_for_tree (heapvar, rvi);
+	  insert_restrict_pointed_var (vi, rvi);
+	}
       fieldstack.release ();
       return vi;
     }
@@ -5738,7 +5782,7 @@ create_variable_info_for_1 (tree decl, const char *name)
 static unsigned int
 create_variable_info_for (tree decl, const char *name)
 {
-  varinfo_t vi = create_variable_info_for_1 (decl, name);
+  varinfo_t vi = create_variable_info_for_1 (decl, name, false);
   unsigned int id = vi->id;
 
   insert_vi_for_tree (decl, vi);
@@ -5880,7 +5924,8 @@ intra_create_variable_infos (struct function *fn)
       varinfo_t p = lookup_vi_for_tree (t);
       if (p == NULL)
 	{
-	  p = create_variable_info_for_1 (t, alias_get_name (t));
+	  p = create_variable_info_for_1 (t, alias_get_name (t),
+					  recursive_restrict_p);
 	  insert_vi_for_tree (t, p);
 	}
 
@@ -5890,12 +5935,17 @@ intra_create_variable_infos (struct function *fn)
 	 in the first/last subfield of the object.  */
       if (recursive_restrict_p)
 	{
-	  varinfo_t vi;
-	  tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
-	  DECL_EXTERNAL (heapvar) = 1;
-	  vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS");
-	  vi->is_restrict_var = 1;
-	  insert_vi_for_tree (heapvar, vi);
+	  varinfo_t vi = lookup_restrict_pointed_var (p);
+	  if (vi == NULL)
+	    {
+	      tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
+	      DECL_EXTERNAL (heapvar) = 1;
+	      vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", false);
+	      vi->is_restrict_var = 1;
+	      insert_vi_for_tree (heapvar, vi);
+	      insert_restrict_pointed_var (p, vi);
+	      p->only_restrict_pointers = 1;
+	    }
 	  make_constraint_from (p, vi->id);
 	  make_restrict_var_constraints (vi);
 	  continue;
@@ -5915,6 +5965,9 @@ intra_create_variable_infos (struct function *fn)
 	}
     }
 
+  delete restrict_pointed_var;
+  restrict_pointed_var = NULL;
+
   /* Add a constraint for a result decl that is passed by reference.  */
   if (DECL_RESULT (fn->decl)
       && DECL_BY_REFERENCE (DECL_RESULT (fn->decl)))
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH, PR67742] Handle restrict struct fields recursively
  2015-10-26 16:26   ` Tom de Vries
@ 2015-10-27  7:59     ` Tom de Vries
  2015-10-27 12:26       ` Tom de Vries
  0 siblings, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-10-27  7:59 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

[ was: Re: [PATCH, 1/2] Add handle_param parameter to 
create_variable_info_for_1 ]

On 26/10/15 17:23, Tom de Vries wrote:
>>  Why does create_variable_info_for_1 only handle
>> the single-field case?  That is, I expected this to be handled by
>> c_v_r_f_1
>> fully.
>>
>
> Yep, that's the goal of PR67742. I've written a patch in an earlier
> version of the patch series that implements that, I'm currently porting
> that patch to this patch series. I'll post asap.

This is the follow-up patch that handles restrict parameters 
recursively, also in struct fields.

Bootstrapped and reg-tested on x86_64.

OK for trunk?

Thanks,
- Tom

[-- Attachment #2: 0009-Handle-restrict-struct-fields-recursively.patch --]
[-- Type: text/x-patch, Size: 6117 bytes --]

Handle restrict struct fields recursively

2015-10-26  Tom de Vries  <tom@codesourcery.com>

	* 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 extra arg to call to
	push_fields_onto_fieldstack.  Add type_contains_placeholder_p test.
	Handle restrict pointer fields.
	(intra_create_variable_infos): Call create_variable_info_for_1 with
	handle_param == true.  Call make_restrict_var_constraints.

	* gcc.dg/tree-ssa/restrict-8.c: New test.
---
 gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c | 17 +++++++++++
 gcc/tree-ssa-structalias.c                 | 47 +++++++++++++++++++++++++-----
 2 files changed, 56 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c

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 0000000..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 344c3b2..2031175 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -399,6 +399,7 @@ new_var_info (tree t, const char *name)
   return ret;
 }
 
+static varinfo_t create_variable_info_for_1 (tree, const char *, bool);
 
 /* A map mapping call statements to per-stmt variables for uses
    and clobbers specific to the call.  */
@@ -5196,6 +5197,8 @@ struct fieldoff
   unsigned may_have_pointers : 1;
 
   unsigned only_restrict_pointers : 1;
+
+  varinfo_t restrict_var;
 };
 typedef struct fieldoff fieldoff_s;
 
@@ -5290,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<fieldoff_s> *fieldstack,
-			     HOST_WIDE_INT offset)
+			     HOST_WIDE_INT offset, bool handle_param)
 {
   tree field;
   bool empty_p = true;
@@ -5320,7 +5324,7 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *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
@@ -5341,7 +5345,8 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *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);
 	      }
 
@@ -5375,6 +5380,19 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *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);
+		    rvi->is_restrict_var = 1;
+		    insert_vi_for_tree (heapvar, rvi);
+		    e.restrict_var = rvi;
+		  }
 		fieldstack->safe_push (e);
 	      }
 	  }
@@ -5679,7 +5697,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool handle_param)
       bool notokay = false;
       unsigned int i;
 
-      push_fields_onto_fieldstack (decl_type, &fieldstack, 0);
+      push_fields_onto_fieldstack (decl_type, &fieldstack, 0, handle_param);
 
       for (i = 0; !notokay && fieldstack.iterate (i, &fo); i++)
 	if (fo->has_unknown_size
@@ -5722,6 +5740,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool handle_param)
 	  && TYPE_RESTRICT (TREE_TYPE (decl)))
 	vi->only_restrict_pointers = 1;
       if (vi->only_restrict_pointers
+	  && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (decl)))
 	  && handle_param)
 	{
 	  varinfo_t rvi;
@@ -5769,6 +5788,10 @@ create_variable_info_for_1 (tree decl, const char *name, bool handle_param)
       newvi->fullsize = vi->fullsize;
       newvi->may_have_pointers = fo->may_have_pointers;
       newvi->only_restrict_pointers = fo->only_restrict_pointers;
+      if (handle_param
+	  && newvi->only_restrict_pointers
+	  && fo->restrict_var != NULL)
+	insert_restrict_pointed_var (newvi, fo->restrict_var);
       if (i + 1 < fieldstack.length ())
 	{
 	  varinfo_t tem = new_var_info (decl, name);
@@ -5934,8 +5957,7 @@ intra_create_variable_infos (struct function *fn)
       varinfo_t p = lookup_vi_for_tree (t);
       if (p == NULL)
 	{
-	  p = create_variable_info_for_1 (t, alias_get_name (t),
-					  recursive_restrict_p);
+	  p = create_variable_info_for_1 (t, alias_get_name (t), true);
 	  insert_vi_for_tree (t, p);
 	}
 
@@ -5968,7 +5990,16 @@ intra_create_variable_infos (struct function *fn)
 	  for (; p; p = vi_next (p))
 	    {
 	      if (p->only_restrict_pointers)
-		make_constraint_from_global_restrict (p, "PARM_RESTRICT");
+		{
+		  varinfo_t vi = lookup_restrict_pointed_var (p);
+		  if (vi != NULL)
+		    {
+		      make_constraint_from (p, vi->id);
+		      make_restrict_var_constraints (vi);
+		    }
+		  else
+		    make_constraint_from_global_restrict (p, "PARM_RESTRICT");
+		}
 	      else if (p->may_have_pointers)
 		make_constraint_from (p, nonlocal_id);
 	    }
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, PR67742] Handle restrict struct fields recursively
  2015-10-27  7:59     ` [PATCH, PR67742] Handle restrict struct fields recursively Tom de Vries
@ 2015-10-27 12:26       ` Tom de Vries
  2015-10-27 12:49         ` [PATCH, 1/6] Simplify constraint handling Tom de Vries
                           ` (5 more replies)
  0 siblings, 6 replies; 35+ messages in thread
From: Tom de Vries @ 2015-10-27 12:26 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

On 27/10/15 08:25, Tom de Vries wrote:
> @@ -5968,7 +5990,16 @@ intra_create_variable_infos (struct function *fn)
>   	  for (; p; p = vi_next (p))
>   	    {
>   	      if (p->only_restrict_pointers)
> -		make_constraint_from_global_restrict (p, "PARM_RESTRICT");
> +		{
> +		  varinfo_t vi = lookup_restrict_pointed_var (p);
> +		  if (vi != NULL)
> +		    {
> +		      make_constraint_from (p, vi->id);
> +		      make_restrict_var_constraints (vi);
> +		    }
> +		  else
> +		    make_constraint_from_global_restrict (p, "PARM_RESTRICT");
> +		}
>   	      else if (p->may_have_pointers)
>   		make_constraint_from (p, nonlocal_id);
>   	    }
> -- 1.9.1

Thinking it over a bit more, I realized the constraint handling started 
to be messy. I've reworked the patch series to simplify that first.

      1	Simplify constraint handling
      2	Rename make_restrict_var_constraints to make_param_constraints
      3	Add recursion to make_param_constraints
      4	Add handle_param parameter to create_variable_info_for_1
      5	Handle recursive restrict pointer in create_variable_info_for_1
      6	Handle restrict struct fields recursively

Currently doing bootstrap and regtest on x86_64.

I'll repost the patch series in reply to this message.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH, 1/6] Simplify constraint handling
  2015-10-27 12:26       ` Tom de Vries
@ 2015-10-27 12:49         ` Tom de Vries
  2015-10-28 15:38           ` Richard Biener
  2015-10-27 12:50         ` [PATCH, 2/6] Rename make_restrict_var_constraints to make_param_constraints Tom de Vries
                           ` (4 subsequent siblings)
  5 siblings, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-10-27 12:49 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

On 27/10/15 13:24, Tom de Vries wrote:
> Thinking it over a bit more, I realized the constraint handling started
> to be messy. I've reworked the patch series to simplify that first.
>
>       1    Simplify constraint handling
>       2    Rename make_restrict_var_constraints to make_param_constraints
>       3    Add recursion to make_param_constraints
>       4    Add handle_param parameter to create_variable_info_for_1
>       5    Handle recursive restrict pointer in create_variable_info_for_1
>       6    Handle restrict struct fields recursively
>
> Currently doing bootstrap and regtest on x86_64.
>
> I'll repost the patch series in reply to this message.

This patch gets rid of this bit of code in intra_create_variable_infos:
...
       if (restrict_pointer_p)
	make_constraint_from_global_restrict (p, "PARM_RESTRICT");
       else
..

I already proposed to remove it here ( 
https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02426.html ) but there is 
a problem with that approach: It can happen that restrict_pointer_p is 
true, but p->only_restrict_pointers is false. This happens with 
fipa-pta, when create_function_info_for created a varinfo for the 
parameter before intra_create_variable_infos was called.

The patch handles that case now by setting p->only_restrict_pointers.

Thanks,
- Tom

[-- Attachment #2: 0001-Simplify-constraint-handling.patch --]
[-- Type: text/x-patch, Size: 1474 bytes --]

Simplify constraint handling

2015-10-27  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (intra_create_variable_infos): Simplify
	constraint handling.
---
 gcc/tree-ssa-structalias.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 5e070bc..4610914 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5884,6 +5884,8 @@ intra_create_variable_infos (struct function *fn)
 	  p = create_variable_info_for_1 (t, alias_get_name (t));
 	  insert_vi_for_tree (t, p);
 	}
+      else if (restrict_pointer_p)
+	p->only_restrict_pointers = 1;
 
       /* For restrict qualified pointers build a representative for
 	 the pointed-to object.  Note that this ends up handling
@@ -5902,17 +5904,12 @@ intra_create_variable_infos (struct function *fn)
 	  continue;
 	}
 
-      if (restrict_pointer_p)
-	make_constraint_from_global_restrict (p, "PARM_RESTRICT");
-      else
+      for (; p; p = vi_next (p))
 	{
-	  for (; p; p = vi_next (p))
-	    {
-	      if (p->only_restrict_pointers)
-		make_constraint_from_global_restrict (p, "PARM_RESTRICT");
-	      else if (p->may_have_pointers)
-		make_constraint_from (p, nonlocal_id);
-	    }
+	  if (p->only_restrict_pointers)
+	    make_constraint_from_global_restrict (p, "PARM_RESTRICT");
+	  else if (p->may_have_pointers)
+	    make_constraint_from (p, nonlocal_id);
 	}
     }
 
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH, 2/6] Rename make_restrict_var_constraints to make_param_constraints
  2015-10-27 12:26       ` Tom de Vries
  2015-10-27 12:49         ` [PATCH, 1/6] Simplify constraint handling Tom de Vries
@ 2015-10-27 12:50         ` Tom de Vries
  2015-10-28 15:45           ` Richard Biener
  2015-10-27 13:04         ` [PATCH, 3/6] Add recursion " Tom de Vries
                           ` (3 subsequent siblings)
  5 siblings, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-10-27 12:50 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

On 27/10/15 13:24, Tom de Vries wrote:
> Thinking it over a bit more, I realized the constraint handling started
> to be messy. I've reworked the patch series to simplify that first.
>
>       1    Simplify constraint handling
>       2    Rename make_restrict_var_constraints to make_param_constraints
>       3    Add recursion to make_param_constraints
>       4    Add handle_param parameter to create_variable_info_for_1
>       5    Handle recursive restrict pointer in create_variable_info_for_1
>       6    Handle restrict struct fields recursively
>
> Currently doing bootstrap and regtest on x86_64.
>
> I'll repost the patch series in reply to this message.
>

This no-functional-changes patch:
- moves the one constraint handling loop left in
   intra_create_variable_infos to make_restrict_var_constraints
- renames make_restrict_var_constraints to make_param_constraints
- adds a parameter toplevel to make_param_constraints to distinguish
   between the two calling contexts

Thanks,
- Tom

[-- Attachment #2: 0002-Rename-make_restrict_var_constraints-to-make_param_c.patch --]
[-- Type: text/x-patch, Size: 2208 bytes --]

Rename make_restrict_var_constraints to make_param_constraints

2015-10-27  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (make_restrict_var_constraints): Rename to ...
	(make_param_constraints): ... this.  Add toplevel parameter.
	(intra_create_variable_infos): Use make_param_constraints.
---
 gcc/tree-ssa-structalias.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 4610914..e88fbf0 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5845,18 +5845,29 @@ debug_solution_for_var (unsigned int var)
   dump_solution_for_var (stderr, var);
 }
 
-/* Register the constraints for restrict var VI.  */
+/* Register the constraints for VI.  If TOPLEVEL then VI is a function
+   parameter, otherwise VI is part of a function parameter.  */
 
 static void
-make_restrict_var_constraints (varinfo_t vi)
+make_param_constraints (varinfo_t vi, bool toplevel)
 {
   for (; vi; vi = vi_next (vi))
     if (vi->may_have_pointers)
       {
 	if (vi->only_restrict_pointers)
-	  make_constraint_from_global_restrict (vi, "GLOBAL_RESTRICT");
+	  {
+	    if (toplevel)
+	      make_constraint_from_global_restrict (vi, "PARM_RESTRICT");
+	    else
+	      make_constraint_from_global_restrict (vi, "GLOBAL_RESTRICT");
+	  }
 	else
-	  make_copy_constraint (vi, nonlocal_id);
+	  {
+	    if (toplevel)
+	      make_constraint_from (vi, nonlocal_id);
+	    else
+	      make_copy_constraint (vi, nonlocal_id);
+	  }
       }
 }
 
@@ -5900,17 +5911,11 @@ intra_create_variable_infos (struct function *fn)
 	  vi->is_restrict_var = 1;
 	  insert_vi_for_tree (heapvar, vi);
 	  make_constraint_from (p, vi->id);
-	  make_restrict_var_constraints (vi);
+	  make_param_constraints (vi, false);
 	  continue;
 	}
 
-      for (; p; p = vi_next (p))
-	{
-	  if (p->only_restrict_pointers)
-	    make_constraint_from_global_restrict (p, "PARM_RESTRICT");
-	  else if (p->may_have_pointers)
-	    make_constraint_from (p, nonlocal_id);
-	}
+      make_param_constraints (p, true);
     }
 
   /* Add a constraint for a result decl that is passed by reference.  */
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH, 3/6] Add recursion to make_param_constraints
  2015-10-27 12:26       ` Tom de Vries
  2015-10-27 12:49         ` [PATCH, 1/6] Simplify constraint handling Tom de Vries
  2015-10-27 12:50         ` [PATCH, 2/6] Rename make_restrict_var_constraints to make_param_constraints Tom de Vries
@ 2015-10-27 13:04         ` Tom de Vries
  2015-11-01 18:04           ` Tom de Vries
  2015-10-27 13:06         ` [PATCH, 4/6] Add handle_param parameter to create_variable_info_for_1 Tom de Vries
                           ` (2 subsequent siblings)
  5 siblings, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-10-27 13:04 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]

On 27/10/15 13:24, Tom de Vries wrote:
> Thinking it over a bit more, I realized the constraint handling started
> to be messy. I've reworked the patch series to simplify that first.
>
>       1    Simplify constraint handling
>       2    Rename make_restrict_var_constraints to make_param_constraints
>       3    Add recursion to make_param_constraints
>       4    Add handle_param parameter to create_variable_info_for_1
>       5    Handle recursive restrict pointer in create_variable_info_for_1
>       6    Handle restrict struct fields recursively
>
> Currently doing bootstrap and regtest on x86_64.
>
> I'll repost the patch series in reply to this message.
>

This patch:
- registers the connection between a restrict pointer var and a
   restrict var in a new hash_map restrict_pointed_var.
- move the restrict pointer constraint handling from
   intra_create_variable_infos to make_param_constraints

The result of this and the two preceding patches is that the constraint 
handling for params in intra_create_variable_infos is reduced to a 
single call to make_param_constraints.

Thanks,
- Tom

[-- Attachment #2: 0003-Add-recursion-to-make_param_constraints.patch --]
[-- Type: text/x-patch, Size: 3038 bytes --]

Add recursion to make_param_constraints

2015-10-27  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (restrict_pointed_var): New static var.
	(insert_restrict_pointed_var, lookup_restrict_pointed_var): New
	function.
	(make_param_constraints): Handle case that
	lookup_restrict_pointed_var (vi) != NULL.
	(intra_create_variable_infos): Call insert_restrict_pointed_var.
	Simplify constraint handling. Delete restrict_pointed_var.
---
 gcc/tree-ssa-structalias.c | 48 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index e88fbf0..93bc325 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5607,6 +5607,39 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)
   return false;
 }
 
+/* Map from restrict pointer variable info to restrict var variable info.  */
+
+static hash_map<varinfo_t, varinfo_t> *restrict_pointed_var = NULL;
+
+/* Insert VI2 as the restrict var for VI in the restrict_pointed_var map.  */
+
+static void
+insert_restrict_pointed_var (varinfo_t vi, varinfo_t vi2)
+{
+  if (restrict_pointed_var == NULL)
+  restrict_pointed_var = new hash_map<varinfo_t, varinfo_t>;
+
+  bool mapped = restrict_pointed_var->put (vi, vi2);
+  gcc_assert (!mapped);
+}
+
+/* Find the restrict var for restrict pointer VI in the restrict_pointed_var
+   map.  If VI does not exist in the map, return NULL, otherwise, return the
+   varinfo we found.  */
+
+static varinfo_t
+lookup_restrict_pointed_var (varinfo_t vi)
+{
+  if (restrict_pointed_var == NULL)
+    return NULL;
+  varinfo_t *slot = restrict_pointed_var->get (vi);
+  if (slot == NULL)
+    return NULL;
+
+  return *slot;
+}
+
+
 /* 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.  */
@@ -5856,7 +5889,13 @@ make_param_constraints (varinfo_t vi, bool toplevel)
       {
 	if (vi->only_restrict_pointers)
 	  {
-	    if (toplevel)
+	    varinfo_t rvi = lookup_restrict_pointed_var (vi);
+	    if (rvi != NULL)
+	      {
+		make_constraint_from (vi, rvi->id);
+		make_param_constraints (rvi, false);
+	      }
+	    else if (toplevel)
 	      make_constraint_from_global_restrict (vi, "PARM_RESTRICT");
 	    else
 	      make_constraint_from_global_restrict (vi, "GLOBAL_RESTRICT");
@@ -5910,14 +5949,15 @@ intra_create_variable_infos (struct function *fn)
 	  vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS");
 	  vi->is_restrict_var = 1;
 	  insert_vi_for_tree (heapvar, vi);
-	  make_constraint_from (p, vi->id);
-	  make_param_constraints (vi, false);
-	  continue;
+	  insert_restrict_pointed_var (p, vi);
 	}
 
       make_param_constraints (p, true);
     }
 
+  delete restrict_pointed_var;
+  restrict_pointed_var = NULL;
+
   /* Add a constraint for a result decl that is passed by reference.  */
   if (DECL_RESULT (fn->decl)
       && DECL_BY_REFERENCE (DECL_RESULT (fn->decl)))
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH, 4/6] Add handle_param parameter to create_variable_info_for_1
  2015-10-27 12:26       ` Tom de Vries
                           ` (2 preceding siblings ...)
  2015-10-27 13:04         ` [PATCH, 3/6] Add recursion " Tom de Vries
@ 2015-10-27 13:06         ` Tom de Vries
  2015-10-27 13:12         ` [PATCH, 5/6] Handle recursive restrict pointer in create_variable_info_for_1 Tom de Vries
  2015-10-27 13:17         ` [PATCH, 6/6] Handle restrict struct fields recursively Tom de Vries
  5 siblings, 0 replies; 35+ messages in thread
From: Tom de Vries @ 2015-10-27 13:06 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 913 bytes --]

On 27/10/15 13:24, Tom de Vries wrote:
it over a bit more, I realized the constraint handling started
> to be messy. I've reworked the patch series to simplify that first.
>
>       1    Simplify constraint handling
>       2    Rename make_restrict_var_constraints to make_param_constraints
>       3    Add recursion to make_param_constraints
>       4    Add handle_param parameter to create_variable_info_for_1
>       5    Handle recursive restrict pointer in create_variable_info_for_1
>       6    Handle restrict struct fields recursively
>
> Currently doing bootstrap and regtest on x86_64.
>
> I'll repost the patch series in reply to this message.

A repost of the patch at 
https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02776.html .

I've changed the structure a bit such that it's clear that the remaining 
'build_fake_var_decl' bit is in 'the lookup_vi_for_tree (t) != NULL' branch.

Thanks,
- Tom


[-- Attachment #2: 0004-Add-handle_param-parameter-to-create_variable_info_f.patch --]
[-- Type: text/x-patch, Size: 4162 bytes --]

Add handle_param parameter to create_variable_info_for_1

2015-10-26  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (create_variable_info_for_1): Add and handle
	handle_param parameter.
	(create_variable_info_for): Call create_variable_info_for_1 with extra
	arg.
	(intra_create_variable_infos): Same.  Handle case that
	lookup_restrict_pointed_var (p) is not NULL.
---
 gcc/tree-ssa-structalias.c | 53 ++++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 93bc325..a639944 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5642,10 +5642,10 @@ lookup_restrict_pointed_var (varinfo_t vi)
 
 /* 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.  */
+   of DECL.  DECL is a function parameter if HANDLE_PARAM is set.  */
 
 static varinfo_t
-create_variable_info_for_1 (tree decl, const char *name)
+create_variable_info_for_1 (tree decl, const char *name, bool handle_param)
 {
   varinfo_t vi, newvi;
   tree decl_type = TREE_TYPE (decl);
@@ -5721,6 +5721,18 @@ create_variable_info_for_1 (tree decl, const char *name)
       if (POINTER_TYPE_P (TREE_TYPE (decl))
 	  && TYPE_RESTRICT (TREE_TYPE (decl)))
 	vi->only_restrict_pointers = 1;
+      if (vi->only_restrict_pointers
+	  && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (decl)))
+	  && handle_param)
+	{
+	  varinfo_t rvi;
+	  tree heapvar = build_fake_var_decl (TREE_TYPE (decl_type));
+	  DECL_EXTERNAL (heapvar) = 1;
+	  rvi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", false);
+	  rvi->is_restrict_var = 1;
+	  insert_vi_for_tree (heapvar, rvi);
+	  insert_restrict_pointed_var (vi, rvi);
+	}
       fieldstack.release ();
       return vi;
     }
@@ -5772,7 +5784,7 @@ create_variable_info_for_1 (tree decl, const char *name)
 static unsigned int
 create_variable_info_for (tree decl, const char *name)
 {
-  varinfo_t vi = create_variable_info_for_1 (decl, name);
+  varinfo_t vi = create_variable_info_for_1 (decl, name, false);
   unsigned int id = vi->id;
 
   insert_vi_for_tree (decl, vi);
@@ -5925,31 +5937,30 @@ intra_create_variable_infos (struct function *fn)
     {
       bool restrict_pointer_p = (POINTER_TYPE_P (TREE_TYPE (t))
 				 && TYPE_RESTRICT (TREE_TYPE (t)));
-      bool recursive_restrict_p
-	= (restrict_pointer_p
-	   && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (t))));
       varinfo_t p = lookup_vi_for_tree (t);
       if (p == NULL)
 	{
-	  p = create_variable_info_for_1 (t, alias_get_name (t));
+	  p = create_variable_info_for_1 (t, alias_get_name (t), true);
 	  insert_vi_for_tree (t, p);
 	}
       else if (restrict_pointer_p)
-	p->only_restrict_pointers = 1;
-
-      /* For restrict qualified pointers build a representative for
-	 the pointed-to object.  Note that this ends up handling
-	 out-of-bound references conservatively by aggregating them
-	 in the first/last subfield of the object.  */
-      if (recursive_restrict_p)
 	{
-	  varinfo_t vi;
-	  tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
-	  DECL_EXTERNAL (heapvar) = 1;
-	  vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS");
-	  vi->is_restrict_var = 1;
-	  insert_vi_for_tree (heapvar, vi);
-	  insert_restrict_pointed_var (p, vi);
+	  p->only_restrict_pointers = 1;
+
+	  /* For restrict qualified pointers build a representative for
+	     the pointed-to object.  Note that this ends up handling
+	     out-of-bound references conservatively by aggregating them
+	     in the first/last subfield of the object.  */
+	  if (!type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (t))))
+	    {
+	      tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
+	      DECL_EXTERNAL (heapvar) = 1;
+	      varinfo_t vi
+		= create_variable_info_for_1 (heapvar, "PARM_NOALIAS", false);
+	      vi->is_restrict_var = 1;
+	      insert_vi_for_tree (heapvar, vi);
+	      insert_restrict_pointed_var (p, vi);
+	    }
 	}
 
       make_param_constraints (p, true);
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH, 5/6] Handle recursive restrict pointer in create_variable_info_for_1
  2015-10-27 12:26       ` Tom de Vries
                           ` (3 preceding siblings ...)
  2015-10-27 13:06         ` [PATCH, 4/6] Add handle_param parameter to create_variable_info_for_1 Tom de Vries
@ 2015-10-27 13:12         ` Tom de Vries
  2015-10-27 13:17         ` [PATCH, 6/6] Handle restrict struct fields recursively Tom de Vries
  5 siblings, 0 replies; 35+ messages in thread
From: Tom de Vries @ 2015-10-27 13:12 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

On 27/10/15 13:24, Tom de Vries wrote:
> Thinking it over a bit more, I realized the constraint handling started
> to be messy. I've reworked the patch series to simplify that first.
>
>       1    Simplify constraint handling
>       2    Rename make_restrict_var_constraints to make_param_constraints
>       3    Add recursion to make_param_constraints
>       4    Add handle_param parameter to create_variable_info_for_1
>       5    Handle recursive restrict pointer in create_variable_info_for_1
>       6    Handle restrict struct fields recursively
>
> Currently doing bootstrap and regtest on x86_64.
>
> I'll repost the patch series in reply to this message.

A repost of the patch at 
https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02779.html .

With the constraint handling dealt with earlier, it has become rather 
minimal.

Thanks,
- Tom


[-- Attachment #2: 0005-Handle-recursive-restrict-pointer-in-create_variable.patch --]
[-- Type: text/x-patch, Size: 1614 bytes --]

Handle recursive restrict pointer in create_variable_info_for_1

2015-10-26  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (create_variable_info_for_1): Enable recursive
	handling of restrict pointers.

	* gcc.dg/tree-ssa/restrict-7.c: New test.
---
 gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c | 12 ++++++++++++
 gcc/tree-ssa-structalias.c                 |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/restrict-7.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 0000000..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/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index a639944..a297a36 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5728,7 +5728,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool handle_param)
 	  varinfo_t rvi;
 	  tree heapvar = build_fake_var_decl (TREE_TYPE (decl_type));
 	  DECL_EXTERNAL (heapvar) = 1;
-	  rvi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", false);
+	  rvi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", true);
 	  rvi->is_restrict_var = 1;
 	  insert_vi_for_tree (heapvar, rvi);
 	  insert_restrict_pointed_var (vi, rvi);
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH, 6/6] Handle restrict struct fields recursively
  2015-10-27 12:26       ` Tom de Vries
                           ` (4 preceding siblings ...)
  2015-10-27 13:12         ` [PATCH, 5/6] Handle recursive restrict pointer in create_variable_info_for_1 Tom de Vries
@ 2015-10-27 13:17         ` Tom de Vries
  5 siblings, 0 replies; 35+ messages in thread
From: Tom de Vries @ 2015-10-27 13:17 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

On 27/10/15 13:24, Tom de Vries wrote:
> Thinking it over a bit more, I realized the constraint handling started
> to be messy. I've reworked the patch series to simplify that first.
>
>       1    Simplify constraint handling
>       2    Rename make_restrict_var_constraints to make_param_constraints
>       3    Add recursion to make_param_constraints
>       4    Add handle_param parameter to create_variable_info_for_1
>       5    Handle recursive restrict pointer in create_variable_info_for_1
>       6    Handle restrict struct fields recursively
>
> Currently doing bootstrap and regtest on x86_64.
>
> I'll repost the patch series in reply to this message.
>

A repost of the patch at 
https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02838.html .

Thanks,
- Tom


[-- Attachment #2: 0006-Handle-restrict-struct-fields-recursively.patch --]
[-- Type: text/x-patch, Size: 4728 bytes --]

Handle restrict struct fields recursively

2015-10-26  Tom de Vries  <tom@codesourcery.com>

	* 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 extra arg to call to
	push_fields_onto_fieldstack.  Handle restrict pointer fields.

	* gcc.dg/tree-ssa/restrict-8.c: New test.
---
 gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c | 17 ++++++++++++++++
 gcc/tree-ssa-structalias.c                 | 32 +++++++++++++++++++++++++-----
 2 files changed, 44 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c

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 0000000..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 a297a36..fac1739 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -399,6 +399,7 @@ new_var_info (tree t, const char *name)
   return ret;
 }
 
+static varinfo_t create_variable_info_for_1 (tree, const char *, bool);
 
 /* A map mapping call statements to per-stmt variables for uses
    and clobbers specific to the call.  */
@@ -5196,6 +5197,8 @@ struct fieldoff
   unsigned may_have_pointers : 1;
 
   unsigned only_restrict_pointers : 1;
+
+  varinfo_t restrict_var;
 };
 typedef struct fieldoff fieldoff_s;
 
@@ -5290,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<fieldoff_s> *fieldstack,
-			     HOST_WIDE_INT offset)
+			     HOST_WIDE_INT offset, bool handle_param)
 {
   tree field;
   bool empty_p = true;
@@ -5320,7 +5324,7 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *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
@@ -5341,7 +5345,8 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *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);
 	      }
 
@@ -5375,6 +5380,19 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *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);
+		    rvi->is_restrict_var = 1;
+		    insert_vi_for_tree (heapvar, rvi);
+		    e.restrict_var = rvi;
+		  }
 		fieldstack->safe_push (e);
 	      }
 	  }
@@ -5679,7 +5697,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool handle_param)
       bool notokay = false;
       unsigned int i;
 
-      push_fields_onto_fieldstack (decl_type, &fieldstack, 0);
+      push_fields_onto_fieldstack (decl_type, &fieldstack, 0, handle_param);
 
       for (i = 0; !notokay && fieldstack.iterate (i, &fo); i++)
 	if (fo->has_unknown_size
@@ -5770,6 +5788,10 @@ create_variable_info_for_1 (tree decl, const char *name, bool handle_param)
       newvi->fullsize = vi->fullsize;
       newvi->may_have_pointers = fo->may_have_pointers;
       newvi->only_restrict_pointers = fo->only_restrict_pointers;
+      if (handle_param
+	  && newvi->only_restrict_pointers
+	  && fo->restrict_var != NULL)
+	insert_restrict_pointed_var (newvi, fo->restrict_var);
       if (i + 1 < fieldstack.length ())
 	{
 	  varinfo_t tem = new_var_info (decl, name);
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 1/6] Simplify constraint handling
  2015-10-27 12:49         ` [PATCH, 1/6] Simplify constraint handling Tom de Vries
@ 2015-10-28 15:38           ` Richard Biener
  2015-10-28 16:06             ` Tom de Vries
  0 siblings, 1 reply; 35+ messages in thread
From: Richard Biener @ 2015-10-28 15:38 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Richard Biener, gcc-patches

On Tue, 27 Oct 2015, Tom de Vries wrote:

> On 27/10/15 13:24, Tom de Vries wrote:
> > Thinking it over a bit more, I realized the constraint handling started
> > to be messy. I've reworked the patch series to simplify that first.
> > 
> >       1    Simplify constraint handling
> >       2    Rename make_restrict_var_constraints to make_param_constraints
> >       3    Add recursion to make_param_constraints
> >       4    Add handle_param parameter to create_variable_info_for_1
> >       5    Handle recursive restrict pointer in create_variable_info_for_1
> >       6    Handle restrict struct fields recursively
> > 
> > Currently doing bootstrap and regtest on x86_64.
> > 
> > I'll repost the patch series in reply to this message.
> 
> This patch gets rid of this bit of code in intra_create_variable_infos:
> ...
>       if (restrict_pointer_p)
> 	make_constraint_from_global_restrict (p, "PARM_RESTRICT");
>       else
> ..
> 
> I already proposed to remove it here (
> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02426.html ) but there is a
> problem with that approach: It can happen that restrict_pointer_p is true, but
> p->only_restrict_pointers is false. This happens with fipa-pta, when
> create_function_info_for created a varinfo for the parameter before
> intra_create_variable_infos was called.
> 
> The patch handles that case now by setting p->only_restrict_pointers.

Hmm, but ... restrict only has an effect in non-IPA mode.  That we
use intra_create_variable_infos in IPA mode is only done for correctness
(to set up incoming NONLOCAL) for functions we do not see all callers of.

Maybe we should fix that (in intra_create_variable_infos properly
add constraints from NONLOCAL for all such functions).

Richard.

> Thanks,
> - Tom
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 2/6] Rename make_restrict_var_constraints to make_param_constraints
  2015-10-27 12:50         ` [PATCH, 2/6] Rename make_restrict_var_constraints to make_param_constraints Tom de Vries
@ 2015-10-28 15:45           ` Richard Biener
  2015-10-28 21:56             ` Tom de Vries
  0 siblings, 1 reply; 35+ messages in thread
From: Richard Biener @ 2015-10-28 15:45 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Richard Biener, gcc-patches

On Tue, 27 Oct 2015, Tom de Vries wrote:

> On 27/10/15 13:24, Tom de Vries wrote:
> > Thinking it over a bit more, I realized the constraint handling started
> > to be messy. I've reworked the patch series to simplify that first.
> > 
> >       1    Simplify constraint handling
> >       2    Rename make_restrict_var_constraints to make_param_constraints
> >       3    Add recursion to make_param_constraints
> >       4    Add handle_param parameter to create_variable_info_for_1
> >       5    Handle recursive restrict pointer in create_variable_info_for_1
> >       6    Handle restrict struct fields recursively
> > 
> > Currently doing bootstrap and regtest on x86_64.
> > 
> > I'll repost the patch series in reply to this message.
> > 
> 
> This no-functional-changes patch:
> - moves the one constraint handling loop left in
>   intra_create_variable_infos to make_restrict_var_constraints
> - renames make_restrict_var_constraints to make_param_constraints
> - adds a parameter toplevel to make_param_constraints to distinguish
>   between the two calling contexts

Please pass in the name "PARAM_RESTRICT" vs. "GLOBAL_RESTRICT" instead.

> Thanks,
> - Tom
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 1/6] Simplify constraint handling
  2015-10-28 15:38           ` Richard Biener
@ 2015-10-28 16:06             ` Tom de Vries
  2015-10-28 21:32               ` Tom de Vries
  2015-10-29 11:16               ` Richard Biener
  0 siblings, 2 replies; 35+ messages in thread
From: Tom de Vries @ 2015-10-28 16:06 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2117 bytes --]

On 28/10/15 16:35, Richard Biener wrote:
> On Tue, 27 Oct 2015, Tom de Vries wrote:
>
>> On 27/10/15 13:24, Tom de Vries wrote:
>>> Thinking it over a bit more, I realized the constraint handling started
>>> to be messy. I've reworked the patch series to simplify that first.
>>>
>>>        1    Simplify constraint handling
>>>        2    Rename make_restrict_var_constraints to make_param_constraints
>>>        3    Add recursion to make_param_constraints
>>>        4    Add handle_param parameter to create_variable_info_for_1
>>>        5    Handle recursive restrict pointer in create_variable_info_for_1
>>>        6    Handle restrict struct fields recursively
>>>
>>> Currently doing bootstrap and regtest on x86_64.
>>>
>>> I'll repost the patch series in reply to this message.
>>
>> This patch gets rid of this bit of code in intra_create_variable_infos:
>> ...
>>        if (restrict_pointer_p)
>> 	make_constraint_from_global_restrict (p, "PARM_RESTRICT");
>>        else
>> ..
>>
>> I already proposed to remove it here (
>> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02426.html ) but there is a
>> problem with that approach: It can happen that restrict_pointer_p is true, but
>> p->only_restrict_pointers is false. This happens with fipa-pta, when
>> create_function_info_for created a varinfo for the parameter before
>> intra_create_variable_infos was called.
>>
>> The patch handles that case now by setting p->only_restrict_pointers.
>
> Hmm, but ... restrict only has an effect in non-IPA mode.

Indeed, I also realized that by now.

I wrote attached patch to make that explicit and simplify fipa-pta.

OK for trunk if bootstrap and reg-test succeeds?

> That we
> use intra_create_variable_infos in IPA mode is only done for correctness
> (to set up incoming NONLOCAL) for functions we do not see all callers of.
>

Right.

> Maybe we should fix that (in intra_create_variable_infos properly
> add constraints from NONLOCAL for all such functions).
>

Aren't we are adding those constraints currently in 
intra_create_variable_infos? Maybe you meant 'in ipa_pta_execute'?

Thanks,
- Tom



[-- Attachment #2: 0009-Don-t-interpret-restrict-in-fipa-pta.patch --]
[-- Type: text/x-patch, Size: 2309 bytes --]

Don't interpret restrict in fipa-pta

2015-10-28  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (push_fields_onto_fieldstack)
	(create_variable_info_for_1, create_variable_info_for)
	(intra_create_variable_infos): Ignore restrict if !flag_ipa_pta.
---
 gcc/tree-ssa-structalias.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 06415a2..878e837 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5383,7 +5383,8 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *fieldstack,
 		e.must_have_pointers = must_have_pointers_p;
 		e.may_have_pointers = true;
 		e.only_restrict_pointers
-		  = (!has_unknown_size
+		  = (!flag_ipa_pta
+		     && !has_unknown_size
 		     && POINTER_TYPE_P (field_type)
 		     && TYPE_RESTRICT (field_type));
 		fieldstack->safe_push (e);
@@ -5696,7 +5697,8 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       vi->fullsize = tree_to_uhwi (declsize);
       vi->size = vi->fullsize;
       vi->is_full_var = true;
-      if (POINTER_TYPE_P (TREE_TYPE (decl))
+      if (!flag_ipa_pta
+	  && POINTER_TYPE_P (TREE_TYPE (decl))
 	  && TYPE_RESTRICT (TREE_TYPE (decl)))
 	vi->only_restrict_pointers = 1;
       fieldstack.release ();
@@ -5766,9 +5768,10 @@ create_variable_info_for (tree decl, const char *name, bool add_id)
 	continue;
 
       /* Mark global restrict qualified pointers.  */
-      if ((POINTER_TYPE_P (TREE_TYPE (decl))
-	   && TYPE_RESTRICT (TREE_TYPE (decl)))
-	  || vi->only_restrict_pointers)
+      if (!flag_ipa_pta
+	  && ((POINTER_TYPE_P (TREE_TYPE (decl))
+	       && TYPE_RESTRICT (TREE_TYPE (decl)))
+	      || vi->only_restrict_pointers))
 	{
 	  varinfo_t rvi
 	    = make_constraint_from_global_restrict (vi, "GLOBAL_RESTRICT",
@@ -5885,7 +5888,8 @@ intra_create_variable_infos (struct function *fn)
      passed-by-reference argument.  */
   for (t = DECL_ARGUMENTS (fn->decl); t; t = DECL_CHAIN (t))
     {
-      bool restrict_pointer_p = (POINTER_TYPE_P (TREE_TYPE (t))
+      bool restrict_pointer_p = (!flag_ipa_pta
+				 && POINTER_TYPE_P (TREE_TYPE (t))
 				 && TYPE_RESTRICT (TREE_TYPE (t)));
       bool recursive_restrict_p
 	= (restrict_pointer_p
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 1/6] Simplify constraint handling
  2015-10-28 16:06             ` Tom de Vries
@ 2015-10-28 21:32               ` Tom de Vries
  2015-10-29 11:16               ` Richard Biener
  1 sibling, 0 replies; 35+ messages in thread
From: Tom de Vries @ 2015-10-28 21:32 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

On 28/10/15 17:05, Tom de Vries wrote:
> On 28/10/15 16:35, Richard Biener wrote:
>> On Tue, 27 Oct 2015, Tom de Vries wrote:
>>
>>> On 27/10/15 13:24, Tom de Vries wrote:
>>>> Thinking it over a bit more, I realized the constraint handling started
>>>> to be messy. I've reworked the patch series to simplify that first.
>>>>
>>>>        1    Simplify constraint handling
>>>>        2    Rename make_restrict_var_constraints to
>>>> make_param_constraints
>>>>        3    Add recursion to make_param_constraints
>>>>        4    Add handle_param parameter to create_variable_info_for_1
>>>>        5    Handle recursive restrict pointer in
>>>> create_variable_info_for_1
>>>>        6    Handle restrict struct fields recursively
>>>>
>>>> Currently doing bootstrap and regtest on x86_64.
>>>>
>>>> I'll repost the patch series in reply to this message.
>>>
>>> This patch gets rid of this bit of code in intra_create_variable_infos:
>>> ...
>>>        if (restrict_pointer_p)
>>>     make_constraint_from_global_restrict (p, "PARM_RESTRICT");
>>>        else
>>> ..
>>>
>>> I already proposed to remove it here (
>>> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02426.html ) but there
>>> is a
>>> problem with that approach: It can happen that restrict_pointer_p is
>>> true, but
>>> p->only_restrict_pointers is false. This happens with fipa-pta, when
>>> create_function_info_for created a varinfo for the parameter before
>>> intra_create_variable_infos was called.
>>>
>>> The patch handles that case now by setting p->only_restrict_pointers.
>>
>> Hmm, but ... restrict only has an effect in non-IPA mode.
>
> Indeed, I also realized that by now.

So I've committed the original, approved patch from 
https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02426.html

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 2/6] Rename make_restrict_var_constraints to make_param_constraints
  2015-10-28 15:45           ` Richard Biener
@ 2015-10-28 21:56             ` Tom de Vries
  0 siblings, 0 replies; 35+ messages in thread
From: Tom de Vries @ 2015-10-28 21:56 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

On 28/10/15 16:38, Richard Biener wrote:
> On Tue, 27 Oct 2015, Tom de Vries wrote:
>
>> On 27/10/15 13:24, Tom de Vries wrote:
>>> Thinking it over a bit more, I realized the constraint handling started
>>> to be messy. I've reworked the patch series to simplify that first.
>>>
>>>        1    Simplify constraint handling
>>>        2    Rename make_restrict_var_constraints to make_param_constraints
>>>        3    Add recursion to make_param_constraints
>>>        4    Add handle_param parameter to create_variable_info_for_1
>>>        5    Handle recursive restrict pointer in create_variable_info_for_1
>>>        6    Handle restrict struct fields recursively
>>>
>>> Currently doing bootstrap and regtest on x86_64.
>>>
>>> I'll repost the patch series in reply to this message.
>>>
>>
>> This no-functional-changes patch:
>> - moves the one constraint handling loop left in
>>    intra_create_variable_infos to make_restrict_var_constraints
>> - renames make_restrict_var_constraints to make_param_constraints
>> - adds a parameter toplevel to make_param_constraints to distinguish
>>    between the two calling contexts
>
> Please pass in the name "PARAM_RESTRICT" vs. "GLOBAL_RESTRICT" instead.
>

I've added a restrict_name parameter. But I don't see how I can 
eliminate the toplevel parameter.

Thanks,
- Tom


[-- Attachment #2: 0011-Rename-make_restrict_var_constraints-to-make_param_c.patch --]
[-- Type: text/x-patch, Size: 2484 bytes --]

Rename make_restrict_var_constraints to make_param_constraints

2015-10-27  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (make_restrict_var_constraints): Rename to ...
	(make_param_constraints): ... this.  Add toplevel and restrict_name
	parameter.
	(intra_create_variable_infos): Use make_param_constraints.
---
 gcc/tree-ssa-structalias.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index e2fbbf1..6e16177 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5860,19 +5860,28 @@ debug_solution_for_var (unsigned int var)
   dump_solution_for_var (stderr, var);
 }
 
-/* Register the constraints for restrict var VI.  */
+/* Register the constraints for VI.  If TOPLEVEL then VI is a function
+   parameter, otherwise VI is part of a function parameter.  Use RESTRICT_NAME
+   as the base name of created restrict vars.  */
 
 static void
-make_restrict_var_constraints (varinfo_t vi)
+make_param_constraints (varinfo_t vi, bool toplevel, const char *restrict_name)
 {
   for (; vi; vi = vi_next (vi))
-    if (vi->may_have_pointers)
-      {
-	if (vi->only_restrict_pointers)
-	  make_constraint_from_global_restrict (vi, "GLOBAL_RESTRICT", true);
-	else
-	  make_copy_constraint (vi, nonlocal_id);
-      }
+    {
+      if (vi->only_restrict_pointers)
+	make_constraint_from_global_restrict (vi, restrict_name, true);
+      else if (vi->may_have_pointers)
+	{
+	  if (toplevel)
+	    make_constraint_from (vi, nonlocal_id);
+	  else
+	    make_copy_constraint (vi, nonlocal_id);
+	}
+
+    if (vi->is_full_var)
+      break;
+    }
 }
 
 /* Create varinfo structures for all of the variables in the
@@ -5914,19 +5923,11 @@ intra_create_variable_infos (struct function *fn)
 	  vi->is_restrict_var = 1;
 	  insert_vi_for_tree (heapvar, vi);
 	  make_constraint_from (p, vi->id);
-	  make_restrict_var_constraints (vi);
+	  make_param_constraints (vi, false, "GLOBAL_RESTRICT");
 	  continue;
 	}
 
-      for (; p; p = vi_next (p))
-	{
-	  if (p->only_restrict_pointers)
-	    make_constraint_from_global_restrict (p, "PARM_RESTRICT", true);
-	  else if (p->may_have_pointers)
-	    make_constraint_from (p, nonlocal_id);
-	  if (p->is_full_var)
-	    break;
-	}
+      make_param_constraints (p, true, "PARM_RESTRICT");
     }
 
   /* Add a constraint for a result decl that is passed by reference.  */
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 1/6] Simplify constraint handling
  2015-10-28 16:06             ` Tom de Vries
  2015-10-28 21:32               ` Tom de Vries
@ 2015-10-29 11:16               ` Richard Biener
  2015-10-29 12:31                 ` Tom de Vries
  1 sibling, 1 reply; 35+ messages in thread
From: Richard Biener @ 2015-10-29 11:16 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Richard Biener, gcc-patches

On Wed, 28 Oct 2015, Tom de Vries wrote:

> On 28/10/15 16:35, Richard Biener wrote:
> > On Tue, 27 Oct 2015, Tom de Vries wrote:
> > 
> > > On 27/10/15 13:24, Tom de Vries wrote:
> > > > Thinking it over a bit more, I realized the constraint handling started
> > > > to be messy. I've reworked the patch series to simplify that first.
> > > > 
> > > >        1    Simplify constraint handling
> > > >        2    Rename make_restrict_var_constraints to
> > > > make_param_constraints
> > > >        3    Add recursion to make_param_constraints
> > > >        4    Add handle_param parameter to create_variable_info_for_1
> > > >        5    Handle recursive restrict pointer in
> > > > create_variable_info_for_1
> > > >        6    Handle restrict struct fields recursively
> > > > 
> > > > Currently doing bootstrap and regtest on x86_64.
> > > > 
> > > > I'll repost the patch series in reply to this message.
> > > 
> > > This patch gets rid of this bit of code in intra_create_variable_infos:
> > > ...
> > >        if (restrict_pointer_p)
> > > 	make_constraint_from_global_restrict (p, "PARM_RESTRICT");
> > >        else
> > > ..
> > > 
> > > I already proposed to remove it here (
> > > https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02426.html ) but there is a
> > > problem with that approach: It can happen that restrict_pointer_p is true,
> > > but
> > > p->only_restrict_pointers is false. This happens with fipa-pta, when
> > > create_function_info_for created a varinfo for the parameter before
> > > intra_create_variable_infos was called.
> > > 
> > > The patch handles that case now by setting p->only_restrict_pointers.
> > 
> > Hmm, but ... restrict only has an effect in non-IPA mode.
> 
> Indeed, I also realized that by now.
> 
> I wrote attached patch to make that explicit and simplify fipa-pta.
> 
> OK for trunk if bootstrap and reg-test succeeds?

I don't see the patch simplifies anything but only removes spurious
settings by adding IMHO redundant checks.

> > That we
> > use intra_create_variable_infos in IPA mode is only done for correctness
> > (to set up incoming NONLOCAL) for functions we do not see all callers of.
> > 
> 
> Right.
> 
> > Maybe we should fix that (in intra_create_variable_infos properly
> > add constraints from NONLOCAL for all such functions).
> > 
> 
> Aren't we are adding those constraints currently in
> intra_create_variable_infos? Maybe you meant 'in ipa_pta_execute'?

I meant create_function_info_for.  Include all the effects of

      /* For externally visible or attribute used annotated functions use
         local constraints for their arguments.
         For local functions we see all callers and thus do not need 
initial
         constraints for parameters.  */
      if (node->used_from_other_partition
          || node->externally_visible
          || node->force_output
          || node->address_taken)
        {
          intra_create_variable_infos (func);

          /* We also need to make function return values escape.  Nothing
             escapes by returning from main though.  */
          if (!MAIN_NAME_P (DECL_NAME (node->decl)))
            {
              varinfo_t fi, rvi;
              fi = lookup_vi_for_tree (node->decl);
              rvi = first_vi_for_offset (fi, fi_result);
              if (rvi && rvi->offset == fi_result)
                {
                  struct constraint_expr includes;
                  struct constraint_expr var;
                  includes.var = escaped_id;
                  includes.offset = 0;
                  includes.type = SCALAR;
                  var.var = rvi->id;
                  var.offset = 0;
                  var.type = SCALAR;
                  process_constraint (new_constraint (includes, var));
                }
            }
        }

in it (just pass it a flag on whether the function is non-local).
The effect of intra_create_variable_infos above was supposed to
add a constraint from NONLOCAL on all vars (but the return value
which instead needs to escape as done above).

Richard.

> Thanks,
> - Tom
> 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 1/6] Simplify constraint handling
  2015-10-29 11:16               ` Richard Biener
@ 2015-10-29 12:31                 ` Tom de Vries
  2015-10-29 13:15                   ` Richard Biener
  0 siblings, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-10-29 12:31 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2985 bytes --]

On 29/10/15 12:13, Richard Biener wrote:
> On Wed, 28 Oct 2015, Tom de Vries wrote:
>
>> >On 28/10/15 16:35, Richard Biener wrote:
>>> > >On Tue, 27 Oct 2015, Tom de Vries wrote:
>>> > >
>>>> > > >On 27/10/15 13:24, Tom de Vries wrote:
>>>>> > > > >Thinking it over a bit more, I realized the constraint handling started
>>>>> > > > >to be messy. I've reworked the patch series to simplify that first.
>>>>> > > > >
>>>>> > > > >        1    Simplify constraint handling
>>>>> > > > >        2    Rename make_restrict_var_constraints to
>>>>> > > > >make_param_constraints
>>>>> > > > >        3    Add recursion to make_param_constraints
>>>>> > > > >        4    Add handle_param parameter to create_variable_info_for_1
>>>>> > > > >        5    Handle recursive restrict pointer in
>>>>> > > > >create_variable_info_for_1
>>>>> > > > >        6    Handle restrict struct fields recursively
>>>>> > > > >
>>>>> > > > >Currently doing bootstrap and regtest on x86_64.
>>>>> > > > >
>>>>> > > > >I'll repost the patch series in reply to this message.
>>>> > > >
>>>> > > >This patch gets rid of this bit of code in intra_create_variable_infos:
>>>> > > >...
>>>> > > >        if (restrict_pointer_p)
>>>> > > >	make_constraint_from_global_restrict (p, "PARM_RESTRICT");
>>>> > > >        else
>>>> > > >..
>>>> > > >
>>>> > > >I already proposed to remove it here (
>>>> > > >https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02426.html  ) but there is a
>>>> > > >problem with that approach: It can happen that restrict_pointer_p is true,
>>>> > > >but
>>>> > > >p->only_restrict_pointers is false. This happens with fipa-pta, when
>>>> > > >create_function_info_for created a varinfo for the parameter before
>>>> > > >intra_create_variable_infos was called.
>>>> > > >
>>>> > > >The patch handles that case now by setting p->only_restrict_pointers.
>>> > >
>>> > >Hmm, but ... restrict only has an effect in non-IPA mode.
>> >
>> >Indeed, I also realized that by now.
>> >
>> >I wrote attached patch to make that explicit and simplify fipa-pta.
>> >
>> >OK for trunk if bootstrap and reg-test succeeds?

First, there was an error in the patch, it tested for flag_ipa_pta (so 
it also affected ealias), but it was supposed to test for in_ipa mode. 
That is fixed in attached version.

 > I don't see the patch simplifies anything but only removes spurious
 > settings by adding IMHO redundant checks.

Consider testcase:
...
int __attribute__((noinline, noclone))
foo (int *__restrict__ a, int *__restrict__ b)
{
   *a = 1;
   *b = 2;
}

int __attribute__((noinline, noclone))
bar (int *a, int *b)
{
   foo (a, b);
}
...

The impact of this patch in the pta dump (focusing on the constraints 
bit) is:
...
  Generating constraints for foo (foo)

-foo.arg0 = &PARM_NOALIAS(20)
-PARM_NOALIAS(20) = NONLOCAL
-foo.arg1 = &PARM_NOALIAS(21)
-PARM_NOALIAS(21) = NONLOCAL
+foo.arg0 = &NONLOCAL
+foo.arg1 = &NONLOCAL
...

That's the kind of simplifications I'm trying to achieve.

Thanks,
- Tom


[-- Attachment #2: 0001-Don-t-interpret-restrict-in-ipa_pta_execute.patch --]
[-- Type: text/x-patch, Size: 2310 bytes --]

Don't interpret restrict in ipa_pta_execute

2015-10-28  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (push_fields_onto_fieldstack)
	(create_variable_info_for_1, create_variable_info_for)
	(intra_create_variable_infos): Ignore restrict if in_ipa_mode.
---
 gcc/tree-ssa-structalias.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 6f72dd3..66ff8cb 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5384,7 +5384,8 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *fieldstack,
 		e.must_have_pointers = must_have_pointers_p;
 		e.may_have_pointers = true;
 		e.only_restrict_pointers
-		  = (!has_unknown_size
+		  = (!in_ipa_mode
+		     && !has_unknown_size
 		     && POINTER_TYPE_P (field_type)
 		     && TYPE_RESTRICT (field_type));
 		fieldstack->safe_push (e);
@@ -5697,7 +5698,8 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       vi->fullsize = tree_to_uhwi (declsize);
       vi->size = vi->fullsize;
       vi->is_full_var = true;
-      if (POINTER_TYPE_P (TREE_TYPE (decl))
+      if (!in_ipa_mode
+	  && POINTER_TYPE_P (TREE_TYPE (decl))
 	  && TYPE_RESTRICT (TREE_TYPE (decl)))
 	vi->only_restrict_pointers = 1;
       fieldstack.release ();
@@ -5767,9 +5769,10 @@ create_variable_info_for (tree decl, const char *name, bool add_id)
 	continue;
 
       /* Mark global restrict qualified pointers.  */
-      if ((POINTER_TYPE_P (TREE_TYPE (decl))
-	   && TYPE_RESTRICT (TREE_TYPE (decl)))
-	  || vi->only_restrict_pointers)
+      if (!in_ipa_mode
+	  && ((POINTER_TYPE_P (TREE_TYPE (decl))
+	       && TYPE_RESTRICT (TREE_TYPE (decl)))
+	      || vi->only_restrict_pointers))
 	{
 	  varinfo_t rvi
 	    = make_constraint_from_global_restrict (vi, "GLOBAL_RESTRICT",
@@ -5886,7 +5889,8 @@ intra_create_variable_infos (struct function *fn)
      passed-by-reference argument.  */
   for (t = DECL_ARGUMENTS (fn->decl); t; t = DECL_CHAIN (t))
     {
-      bool restrict_pointer_p = (POINTER_TYPE_P (TREE_TYPE (t))
+      bool restrict_pointer_p = (!in_ipa_mode
+				 && POINTER_TYPE_P (TREE_TYPE (t))
 				 && TYPE_RESTRICT (TREE_TYPE (t)));
       bool recursive_restrict_p
 	= (restrict_pointer_p
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 1/6] Simplify constraint handling
  2015-10-29 12:31                 ` Tom de Vries
@ 2015-10-29 13:15                   ` Richard Biener
  2015-10-29 16:10                     ` Tom de Vries
  0 siblings, 1 reply; 35+ messages in thread
From: Richard Biener @ 2015-10-29 13:15 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Richard Biener, gcc-patches

On Thu, 29 Oct 2015, Tom de Vries wrote:

> On 29/10/15 12:13, Richard Biener wrote:
> > On Wed, 28 Oct 2015, Tom de Vries wrote:
> > 
> > > >On 28/10/15 16:35, Richard Biener wrote:
> > > > > >On Tue, 27 Oct 2015, Tom de Vries wrote:
> > > > > >
> > > > > > > >On 27/10/15 13:24, Tom de Vries wrote:
> > > > > > > > > >Thinking it over a bit more, I realized the constraint
> > > > > > handling started
> > > > > > > > > >to be messy. I've reworked the patch series to simplify that
> > > > > > first.
> > > > > > > > > >
> > > > > > > > > >        1    Simplify constraint handling
> > > > > > > > > >        2    Rename make_restrict_var_constraints to
> > > > > > > > > >make_param_constraints
> > > > > > > > > >        3    Add recursion to make_param_constraints
> > > > > > > > > >        4    Add handle_param parameter to
> > > > > > create_variable_info_for_1
> > > > > > > > > >        5    Handle recursive restrict pointer in
> > > > > > > > > >create_variable_info_for_1
> > > > > > > > > >        6    Handle restrict struct fields recursively
> > > > > > > > > >
> > > > > > > > > >Currently doing bootstrap and regtest on x86_64.
> > > > > > > > > >
> > > > > > > > > >I'll repost the patch series in reply to this message.
> > > > > > > >
> > > > > > > >This patch gets rid of this bit of code in
> > > > > intra_create_variable_infos:
> > > > > > > >...
> > > > > > > >        if (restrict_pointer_p)
> > > > > > > >	make_constraint_from_global_restrict (p, "PARM_RESTRICT");
> > > > > > > >        else
> > > > > > > >..
> > > > > > > >
> > > > > > > >I already proposed to remove it here (
> > > > > > > >https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02426.html  ) but
> > > > > there is a
> > > > > > > >problem with that approach: It can happen that restrict_pointer_p
> > > > > is true,
> > > > > > > >but
> > > > > > > >p->only_restrict_pointers is false. This happens with fipa-pta,
> > > > > when
> > > > > > > >create_function_info_for created a varinfo for the parameter
> > > > > before
> > > > > > > >intra_create_variable_infos was called.
> > > > > > > >
> > > > > > > >The patch handles that case now by setting
> > > > > p->only_restrict_pointers.
> > > > > >
> > > > > >Hmm, but ... restrict only has an effect in non-IPA mode.
> > > >
> > > >Indeed, I also realized that by now.
> > > >
> > > >I wrote attached patch to make that explicit and simplify fipa-pta.
> > > >
> > > >OK for trunk if bootstrap and reg-test succeeds?
> 
> First, there was an error in the patch, it tested for flag_ipa_pta (so it also
> affected ealias), but it was supposed to test for in_ipa mode. That is fixed
> in attached version.
> 
> > I don't see the patch simplifies anything but only removes spurious
> > settings by adding IMHO redundant checks.
> 
> Consider testcase:
> ...
> int __attribute__((noinline, noclone))
> foo (int *__restrict__ a, int *__restrict__ b)
> {
>   *a = 1;
>   *b = 2;
> }
> 
> int __attribute__((noinline, noclone))
> bar (int *a, int *b)
> {
>   foo (a, b);
> }
> ...
> 
> The impact of this patch in the pta dump (focusing on the constraints bit) is:
> ...
>  Generating constraints for foo (foo)
> 
> -foo.arg0 = &PARM_NOALIAS(20)
> -PARM_NOALIAS(20) = NONLOCAL
> -foo.arg1 = &PARM_NOALIAS(21)
> -PARM_NOALIAS(21) = NONLOCAL
> +foo.arg0 = &NONLOCAL
> +foo.arg1 = &NONLOCAL
> ...
> 
> That's the kind of simplifications I'm trying to achieve.

Yes, but as I said we should refactor things to avoid calling
the intra constraints generation from the IPA path.

Richard.

> Thanks,
> - Tom
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 1/6] Simplify constraint handling
  2015-10-29 13:15                   ` Richard Biener
@ 2015-10-29 16:10                     ` Tom de Vries
  2015-10-30  9:44                       ` Richard Biener
  0 siblings, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-10-29 16:10 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 4231 bytes --]

On 29/10/15 14:12, Richard Biener wrote:
> On Thu, 29 Oct 2015, Tom de Vries wrote:
>
>> >On 29/10/15 12:13, Richard Biener wrote:
>>> > >On Wed, 28 Oct 2015, Tom de Vries wrote:
>>> > >
>>>>> > > > >On 28/10/15 16:35, Richard Biener wrote:
>>>>>>> > > > > > >On Tue, 27 Oct 2015, Tom de Vries wrote:
>>>>>>> > > > > > >
>>>>>>>>> > > > > > > > >On 27/10/15 13:24, Tom de Vries wrote:
>>>>>>>>>>> > > > > > > > > > >Thinking it over a bit more, I realized the constraint
>>>>>>> > > > > > >handling started
>>>>>>>>>>> > > > > > > > > > >to be messy. I've reworked the patch series to simplify that
>>>>>>> > > > > > >first.
>>>>>>>>>>> > > > > > > > > > >
>>>>>>>>>>> > > > > > > > > > >        1    Simplify constraint handling
>>>>>>>>>>> > > > > > > > > > >        2    Rename make_restrict_var_constraints to
>>>>>>>>>>> > > > > > > > > > >make_param_constraints
>>>>>>>>>>> > > > > > > > > > >        3    Add recursion to make_param_constraints
>>>>>>>>>>> > > > > > > > > > >        4    Add handle_param parameter to
>>>>>>> > > > > > >create_variable_info_for_1
>>>>>>>>>>> > > > > > > > > > >        5    Handle recursive restrict pointer in
>>>>>>>>>>> > > > > > > > > > >create_variable_info_for_1
>>>>>>>>>>> > > > > > > > > > >        6    Handle restrict struct fields recursively
>>>>>>>>>>> > > > > > > > > > >
>>>>>>>>>>> > > > > > > > > > >Currently doing bootstrap and regtest on x86_64.
>>>>>>>>>>> > > > > > > > > > >
>>>>>>>>>>> > > > > > > > > > >I'll repost the patch series in reply to this message.
>>>>>>>>> > > > > > > > >
>>>>>>>>> > > > > > > > >This patch gets rid of this bit of code in
>>>>>> > > > > >intra_create_variable_infos:
>>>>>>>>> > > > > > > > >...
>>>>>>>>> > > > > > > > >        if (restrict_pointer_p)
>>>>>>>>> > > > > > > > >	make_constraint_from_global_restrict (p, "PARM_RESTRICT");
>>>>>>>>> > > > > > > > >        else
>>>>>>>>> > > > > > > > >..
>>>>>>>>> > > > > > > > >
>>>>>>>>> > > > > > > > >I already proposed to remove it here (
>>>>>>>>> > > > > > > > >https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02426.html   ) but
>>>>>> > > > > >there is a
>>>>>>>>> > > > > > > > >problem with that approach: It can happen that restrict_pointer_p
>>>>>> > > > > >is true,
>>>>>>>>> > > > > > > > >but
>>>>>>>>> > > > > > > > >p->only_restrict_pointers is false. This happens with fipa-pta,
>>>>>> > > > > >when
>>>>>>>>> > > > > > > > >create_function_info_for created a varinfo for the parameter
>>>>>> > > > > >before
>>>>>>>>> > > > > > > > >intra_create_variable_infos was called.
>>>>>>>>> > > > > > > > >
>>>>>>>>> > > > > > > > >The patch handles that case now by setting
>>>>>> > > > > >p->only_restrict_pointers.
>>>>>>> > > > > > >
>>>>>>> > > > > > >Hmm, but ... restrict only has an effect in non-IPA mode.
>>>>> > > > >
>>>>> > > > >Indeed, I also realized that by now.
>>>>> > > > >
>>>>> > > > >I wrote attached patch to make that explicit and simplify fipa-pta.
>>>>> > > > >
>>>>> > > > >OK for trunk if bootstrap and reg-test succeeds?
>> >
>> >First, there was an error in the patch, it tested for flag_ipa_pta (so it also
>> >affected ealias), but it was supposed to test for in_ipa mode. That is fixed
>> >in attached version.
>> >
>>> > >I don't see the patch simplifies anything but only removes spurious
>>> > >settings by adding IMHO redundant checks.
>> >
>> >Consider testcase:
>> >...
>> >int __attribute__((noinline, noclone))
>> >foo (int *__restrict__ a, int *__restrict__ b)
>> >{
>> >   *a = 1;
>> >   *b = 2;
>> >}
>> >
>> >int __attribute__((noinline, noclone))
>> >bar (int *a, int *b)
>> >{
>> >   foo (a, b);
>> >}
>> >...
>> >
>> >The impact of this patch in the pta dump (focusing on the constraints bit) is:
>> >...
>> >  Generating constraints for foo (foo)
>> >
>> >-foo.arg0 = &PARM_NOALIAS(20)
>> >-PARM_NOALIAS(20) = NONLOCAL
>> >-foo.arg1 = &PARM_NOALIAS(21)
>> >-PARM_NOALIAS(21) = NONLOCAL
>> >+foo.arg0 = &NONLOCAL
>> >+foo.arg1 = &NONLOCAL
>> >...
>> >
>> >That's the kind of simplifications I'm trying to achieve.
> Yes, but as I said we should refactor things to avoid calling
> the intra constraints generation from the IPA path.

Ah, I see.

So, like this? OK for trunk if bootstrap and reg-test succeeds?

Thanks,
- Tom


[-- Attachment #2: 0012-Add-initial-constraints-in-create_function_info_for.patch --]
[-- Type: text/x-patch, Size: 5084 bytes --]

Add initial constraints in create_function_info_for

2015-10-29  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (ipa_pta_execute): Add extra arg to call to
	create_function_info_for.  Dump constraints generated during
	create_function_info_for. Move intra_create_variable_infos call and
	function-return-values-escape bit to ...
	(create_function_info_for): ... here, and merge
	intra_create_variable_infos call with argument loop.  Add and handle
	nonlocal_p parameter.
---
 gcc/tree-ssa-structalias.c | 94 ++++++++++++++++++++++++++++------------------
 1 file changed, 58 insertions(+), 36 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 3bef607..3a0891c 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5421,10 +5421,12 @@ count_num_arguments (tree decl, bool *is_varargs)
 }
 
 /* Creation function node for DECL, using NAME, and return the index
-   of the variable we've created for the function.  */
+   of the variable we've created for the function.  If NONLOCAL_p, create
+   initial constraints.  */
 
 static varinfo_t
-create_function_info_for (tree decl, const char *name, bool add_id)
+create_function_info_for (tree decl, const char *name, bool add_id,
+			  bool nonlocal_p)
 {
   struct function *fn = DECL_STRUCT_FUNCTION (decl);
   varinfo_t vi, prev_vi;
@@ -5536,6 +5538,28 @@ create_function_info_for (tree decl, const char *name, bool add_id)
 	insert_vi_for_tree (DECL_RESULT (decl), resultvi);
     }
 
+  /* We also need to make function return values escape.  Nothing
+     escapes by returning from main though.  */
+  if (nonlocal_p
+      && !MAIN_NAME_P (DECL_NAME (decl)))
+    {
+      varinfo_t fi, rvi;
+      fi = lookup_vi_for_tree (decl);
+      rvi = first_vi_for_offset (fi, fi_result);
+      if (rvi && rvi->offset == fi_result)
+	{
+	  struct constraint_expr includes;
+	  struct constraint_expr var;
+	  includes.var = escaped_id;
+	  includes.offset = 0;
+	  includes.type = SCALAR;
+	  var.var = rvi->id;
+	  var.offset = 0;
+	  var.type = SCALAR;
+	  process_constraint (new_constraint (includes, var));
+	}
+    }
+
   /* Set up variables for each argument.  */
   arg = DECL_ARGUMENTS (decl);
   for (i = 0; i < num_args; i++)
@@ -5562,6 +5586,11 @@ create_function_info_for (tree decl, const char *name, bool add_id)
       gcc_assert (prev_vi->offset < argvi->offset);
       prev_vi->next = argvi->id;
       prev_vi = argvi;
+
+      if (nonlocal_p
+	  && argvi->may_have_pointers)
+	make_constraint_from (argvi, nonlocal_id);
+
       if (arg)
 	{
 	  insert_vi_for_tree (arg, argvi);
@@ -7317,8 +7346,34 @@ ipa_pta_execute (void)
 
       gcc_assert (!node->clone_of);
 
+      /* For externally visible or attribute used annotated functions use
+	 local constraints for their arguments.
+	 For local functions we see all callers and thus do not need initial
+	 constraints for parameters.  */
+      bool nonlocal_p = (node->used_from_other_partition
+			 || node->externally_visible
+			 || node->force_output
+			 || node->address_taken);
+
       vi = create_function_info_for (node->decl,
-				     alias_get_name (node->decl), false);
+				     alias_get_name (node->decl), false,
+				     nonlocal_p);
+      if (dump_file
+	  && from != constraints.length ())
+	{
+	  fprintf (dump_file,
+		   "Generating intial constraints for %s", node->name ());
+	  if (DECL_ASSEMBLER_NAME_SET_P (node->decl))
+	    fprintf (dump_file, " (%s)",
+		     IDENTIFIER_POINTER
+		       (DECL_ASSEMBLER_NAME (node->decl)));
+	  fprintf (dump_file, "\n\n");
+	  dump_constraints (dump_file, from);
+	  fprintf (dump_file, "\n");
+
+	  from = constraints.length ();
+	}
+
       node->call_for_symbol_thunks_and_aliases
 	(associate_varinfo_to_alias, vi, true);
     }
@@ -7365,39 +7420,6 @@ ipa_pta_execute (void)
       func = DECL_STRUCT_FUNCTION (node->decl);
       gcc_assert (cfun == NULL);
 
-      /* For externally visible or attribute used annotated functions use
-	 local constraints for their arguments.
-	 For local functions we see all callers and thus do not need initial
-	 constraints for parameters.  */
-      if (node->used_from_other_partition
-	  || node->externally_visible
-	  || node->force_output
-	  || node->address_taken)
-	{
-	  intra_create_variable_infos (func);
-
-	  /* We also need to make function return values escape.  Nothing
-	     escapes by returning from main though.  */
-	  if (!MAIN_NAME_P (DECL_NAME (node->decl)))
-	    {
-	      varinfo_t fi, rvi;
-	      fi = lookup_vi_for_tree (node->decl);
-	      rvi = first_vi_for_offset (fi, fi_result);
-	      if (rvi && rvi->offset == fi_result)
-		{
-		  struct constraint_expr includes;
-		  struct constraint_expr var;
-		  includes.var = escaped_id;
-		  includes.offset = 0;
-		  includes.type = SCALAR;
-		  var.var = rvi->id;
-		  var.offset = 0;
-		  var.type = SCALAR;
-		  process_constraint (new_constraint (includes, var));
-		}
-	    }
-	}
-
       /* Build constriants for the function body.  */
       FOR_EACH_BB_FN (bb, func)
 	{
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 1/6] Simplify constraint handling
  2015-10-29 16:10                     ` Tom de Vries
@ 2015-10-30  9:44                       ` Richard Biener
  2015-10-31  8:24                         ` Tom de Vries
  0 siblings, 1 reply; 35+ messages in thread
From: Richard Biener @ 2015-10-30  9:44 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Richard Biener, gcc-patches

On Thu, 29 Oct 2015, Tom de Vries wrote:

> On 29/10/15 14:12, Richard Biener wrote:
> > On Thu, 29 Oct 2015, Tom de Vries wrote:
> > 
> > > >On 29/10/15 12:13, Richard Biener wrote:
> > > > > >On Wed, 28 Oct 2015, Tom de Vries wrote:
> > > > > >
> > > > > > > > > >On 28/10/15 16:35, Richard Biener wrote:
> > > > > > > > > > > > > >On Tue, 27 Oct 2015, Tom de Vries wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > >On 27/10/15 13:24, Tom de Vries wrote:
> > > > > > > > > > > > > > > > > > > > > >Thinking it over a bit more, I
> > > > > > > > > > > > realized the constraint
> > > > > > > > > > > > > >handling started
> > > > > > > > > > > > > > > > > > > > > >to be messy. I've reworked the patch
> > > > > > > > > > > > series to simplify that
> > > > > > > > > > > > > >first.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >        1    Simplify constraint
> > > > > > > > > > > > handling
> > > > > > > > > > > > > > > > > > > > > >        2    Rename
> > > > > > > > > > > > make_restrict_var_constraints to
> > > > > > > > > > > > > > > > > > > > > >make_param_constraints
> > > > > > > > > > > > > > > > > > > > > >        3    Add recursion to
> > > > > > > > > > > > make_param_constraints
> > > > > > > > > > > > > > > > > > > > > >        4    Add handle_param
> > > > > > > > > > > > parameter to
> > > > > > > > > > > > > >create_variable_info_for_1
> > > > > > > > > > > > > > > > > > > > > >        5    Handle recursive
> > > > > > > > > > > > restrict pointer in
> > > > > > > > > > > > > > > > > > > > > >create_variable_info_for_1
> > > > > > > > > > > > > > > > > > > > > >        6    Handle restrict struct
> > > > > > > > > > > > fields recursively
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >Currently doing bootstrap and regtest
> > > > > > > > > > > > on x86_64.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >I'll repost the patch series in reply
> > > > > > > > > > > > to this message.
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > >This patch gets rid of this bit of code in
> > > > > > > > > > > >intra_create_variable_infos:
> > > > > > > > > > > > > > > > > >...
> > > > > > > > > > > > > > > > > >        if (restrict_pointer_p)
> > > > > > > > > > > > > > > > > >	make_constraint_from_global_restrict
> > > > > > > > > > (p, "PARM_RESTRICT");
> > > > > > > > > > > > > > > > > >        else
> > > > > > > > > > > > > > > > > >..
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > >I already proposed to remove it here (
> > > > > > > > > > > > > > > > >
> > > > > > > > > > >https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02426.html
> > > > > > > > > > ) but
> > > > > > > > > > > >there is a
> > > > > > > > > > > > > > > > > >problem with that approach: It can happen
> > > > > > > > > > that restrict_pointer_p
> > > > > > > > > > > >is true,
> > > > > > > > > > > > > > > > > >but
> > > > > > > > > > > > > > > > > >p->only_restrict_pointers is false. This
> > > > > > > > > > happens with fipa-pta,
> > > > > > > > > > > >when
> > > > > > > > > > > > > > > > > >create_function_info_for created a varinfo
> > > > > > > > > > for the parameter
> > > > > > > > > > > >before
> > > > > > > > > > > > > > > > > >intra_create_variable_infos was called.
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > >The patch handles that case now by setting
> > > > > > > > > > > >p->only_restrict_pointers.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >Hmm, but ... restrict only has an effect in non-IPA
> > > > > > > > mode.
> > > > > > > > > >
> > > > > > > > > >Indeed, I also realized that by now.
> > > > > > > > > >
> > > > > > > > > >I wrote attached patch to make that explicit and simplify
> > > > > > fipa-pta.
> > > > > > > > > >
> > > > > > > > > >OK for trunk if bootstrap and reg-test succeeds?
> > > >
> > > >First, there was an error in the patch, it tested for flag_ipa_pta (so it
> > > also
> > > >affected ealias), but it was supposed to test for in_ipa mode. That is
> > > fixed
> > > >in attached version.
> > > >
> > > > > >I don't see the patch simplifies anything but only removes spurious
> > > > > >settings by adding IMHO redundant checks.
> > > >
> > > >Consider testcase:
> > > >...
> > > >int __attribute__((noinline, noclone))
> > > >foo (int *__restrict__ a, int *__restrict__ b)
> > > >{
> > > >   *a = 1;
> > > >   *b = 2;
> > > >}
> > > >
> > > >int __attribute__((noinline, noclone))
> > > >bar (int *a, int *b)
> > > >{
> > > >   foo (a, b);
> > > >}
> > > >...
> > > >
> > > >The impact of this patch in the pta dump (focusing on the constraints
> > > bit) is:
> > > >...
> > > >  Generating constraints for foo (foo)
> > > >
> > > >-foo.arg0 = &PARM_NOALIAS(20)
> > > >-PARM_NOALIAS(20) = NONLOCAL
> > > >-foo.arg1 = &PARM_NOALIAS(21)
> > > >-PARM_NOALIAS(21) = NONLOCAL
> > > >+foo.arg0 = &NONLOCAL
> > > >+foo.arg1 = &NONLOCAL
> > > >...
> > > >
> > > >That's the kind of simplifications I'm trying to achieve.
> > Yes, but as I said we should refactor things to avoid calling
> > the intra constraints generation from the IPA path.
> 
> Ah, I see.
> 
> So, like this? OK for trunk if bootstrap and reg-test succeeds?

Yes, like this.  But you miss to apply the same to the static chain,
and the varargs "rest".

Ok with that change.

Richard.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 1/6] Simplify constraint handling
  2015-10-30  9:44                       ` Richard Biener
@ 2015-10-31  8:24                         ` Tom de Vries
  2015-10-31  9:44                           ` [committed, trivial] Don't expect existing varinfo for arguments in intra_create_variable_infos Tom de Vries
  0 siblings, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-10-31  8:24 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 484 bytes --]

On 30/10/15 10:33, Richard Biener wrote:
>>> Yes, but as I said we should refactor things to avoid calling
>>> > >the intra constraints generation from the IPA path.
>> >
>> >Ah, I see.
>> >
>> >So, like this? OK for trunk if bootstrap and reg-test succeeds?
> Yes, like this.  But you miss to apply the same to the static chain,
> and the varargs "rest".
>
> Ok with that change.

Updated patch, bootstrapped and reg-tested on x86_64.

Committed to trunk as attached.

Thanks,
- Tom

[-- Attachment #2: 0003-Add-initial-constraints-in-create_function_info_for.patch --]
[-- Type: text/x-patch, Size: 5352 bytes --]

Add initial constraints in create_function_info_for

2015-10-29  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (ipa_pta_execute): Add extra arg to call to
	create_function_info_for.  Dump constraints generated during
	create_function_info_for. Move intra_create_variable_infos call and
	function-return-values-escape bit to ...
	(create_function_info_for): ... here, and merge
	intra_create_variable_infos call with argument loop.  Add and handle
	nonlocal_p parameter.
---
 gcc/tree-ssa-structalias.c | 81 +++++++++++++++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 26 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 5195eb39..b361096 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5422,10 +5422,12 @@ count_num_arguments (tree decl, bool *is_varargs)
 }
 
 /* Creation function node for DECL, using NAME, and return the index
-   of the variable we've created for the function.  */
+   of the variable we've created for the function.  If NONLOCAL_p, create
+   initial constraints.  */
 
 static varinfo_t
-create_function_info_for (tree decl, const char *name, bool add_id)
+create_function_info_for (tree decl, const char *name, bool add_id,
+			  bool nonlocal_p)
 {
   struct function *fn = DECL_STRUCT_FUNCTION (decl);
   varinfo_t vi, prev_vi;
@@ -5506,6 +5508,10 @@ create_function_info_for (tree decl, const char *name, bool add_id)
 
       insert_vi_for_tree (fn->static_chain_decl, chainvi);
 
+      if (nonlocal_p
+	  && chainvi->may_have_pointers)
+	make_constraint_from (chainvi, nonlocal_id);
+
       gcc_assert (prev_vi->offset < chainvi->offset);
       prev_vi->next = chainvi->id;
       prev_vi = chainvi;
@@ -5543,6 +5549,18 @@ create_function_info_for (tree decl, const char *name, bool add_id)
       prev_vi = resultvi;
     }
 
+  /* We also need to make function return values escape.  Nothing
+     escapes by returning from main though.  */
+  if (nonlocal_p
+      && !MAIN_NAME_P (DECL_NAME (decl)))
+    {
+      varinfo_t fi, rvi;
+      fi = lookup_vi_for_tree (decl);
+      rvi = first_vi_for_offset (fi, fi_result);
+      if (rvi && rvi->offset == fi_result)
+	make_copy_constraint (get_varinfo (escaped_id), rvi->id);
+    }
+
   /* Set up variables for each argument.  */
   arg = DECL_ARGUMENTS (decl);
   for (i = 0; i < num_args; i++)
@@ -5570,6 +5588,10 @@ create_function_info_for (tree decl, const char *name, bool add_id)
       if (arg)
 	insert_vi_for_tree (arg, argvi);
 
+      if (nonlocal_p
+	  && argvi->may_have_pointers)
+	make_constraint_from (argvi, nonlocal_id);
+
       gcc_assert (prev_vi->offset < argvi->offset);
       prev_vi->next = argvi->id;
       prev_vi = argvi;
@@ -5599,6 +5621,10 @@ create_function_info_for (tree decl, const char *name, bool add_id)
       argvi->is_heap_var = true;
       argvi->fullsize = vi->fullsize;
 
+      if (nonlocal_p
+	  && argvi->may_have_pointers)
+	make_constraint_from (argvi, nonlocal_id);
+
       gcc_assert (prev_vi->offset < argvi->offset);
       prev_vi->next = argvi->id;
       prev_vi = argvi;
@@ -7325,8 +7351,34 @@ ipa_pta_execute (void)
 
       gcc_assert (!node->clone_of);
 
+      /* For externally visible or attribute used annotated functions use
+	 local constraints for their arguments.
+	 For local functions we see all callers and thus do not need initial
+	 constraints for parameters.  */
+      bool nonlocal_p = (node->used_from_other_partition
+			 || node->externally_visible
+			 || node->force_output
+			 || node->address_taken);
+
       vi = create_function_info_for (node->decl,
-				     alias_get_name (node->decl), false);
+				     alias_get_name (node->decl), false,
+				     nonlocal_p);
+      if (dump_file
+	  && from != constraints.length ())
+	{
+	  fprintf (dump_file,
+		   "Generating intial constraints for %s", node->name ());
+	  if (DECL_ASSEMBLER_NAME_SET_P (node->decl))
+	    fprintf (dump_file, " (%s)",
+		     IDENTIFIER_POINTER
+		       (DECL_ASSEMBLER_NAME (node->decl)));
+	  fprintf (dump_file, "\n\n");
+	  dump_constraints (dump_file, from);
+	  fprintf (dump_file, "\n");
+
+	  from = constraints.length ();
+	}
+
       node->call_for_symbol_thunks_and_aliases
 	(associate_varinfo_to_alias, vi, true);
     }
@@ -7373,29 +7425,6 @@ ipa_pta_execute (void)
       func = DECL_STRUCT_FUNCTION (node->decl);
       gcc_assert (cfun == NULL);
 
-      /* For externally visible or attribute used annotated functions use
-	 local constraints for their arguments.
-	 For local functions we see all callers and thus do not need initial
-	 constraints for parameters.  */
-      if (node->used_from_other_partition
-	  || node->externally_visible
-	  || node->force_output
-	  || node->address_taken)
-	{
-	  intra_create_variable_infos (func);
-
-	  /* We also need to make function return values escape.  Nothing
-	     escapes by returning from main though.  */
-	  if (!MAIN_NAME_P (DECL_NAME (node->decl)))
-	    {
-	      varinfo_t fi, rvi;
-	      fi = lookup_vi_for_tree (node->decl);
-	      rvi = first_vi_for_offset (fi, fi_result);
-	      if (rvi && rvi->offset == fi_result)
-		make_copy_constraint (get_varinfo (escaped_id), rvi->id);
-	    }
-	}
-
       /* Build constriants for the function body.  */
       FOR_EACH_BB_FN (bb, func)
 	{
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [committed, trivial] Don't expect existing varinfo for arguments in intra_create_variable_infos
  2015-10-31  8:24                         ` Tom de Vries
@ 2015-10-31  9:44                           ` Tom de Vries
  0 siblings, 0 replies; 35+ messages in thread
From: Tom de Vries @ 2015-10-31  9:44 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 812 bytes --]

[ was: Re: [PATCH, 1/6] Simplify constraint handling ]

On 31/10/15 09:19, Tom de Vries wrote:
> On 30/10/15 10:33, Richard Biener wrote:
>>>> Yes, but as I said we should refactor things to avoid calling
>>>> > >the intra constraints generation from the IPA path.
>>> >
>>> >Ah, I see.
>>> >
>>> >So, like this? OK for trunk if bootstrap and reg-test succeeds?
>> Yes, like this.  But you miss to apply the same to the static chain,
>> and the varargs "rest".
>>
>> Ok with that change.
>
> Updated patch, bootstrapped and reg-tested on x86_64.
>
> Committed to trunk as attached.
>

And now that we don't call intra_create_variable_infos anymore during 
ipa_pta_execute, we can simplify intra_create_variable_infos a bit.

Bootstrapped and reg-tested on x86_64.

Committed to trunk as trivial.

Thanks,
- Tom


[-- Attachment #2: 0004-Don-t-expect-existing-varinfo-for-arguments-in-intra.patch --]
[-- Type: text/x-patch, Size: 1114 bytes --]

Don't expect existing varinfo for arguments in intra_create_variable_infos

2015-10-30  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (intra_create_variable_infos): Don't expect
	existing varinfo for arguments.
---
 gcc/tree-ssa-structalias.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index b361096..d409727 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5925,12 +5925,8 @@ intra_create_variable_infos (struct function *fn)
       bool recursive_restrict_p
 	= (restrict_pointer_p
 	   && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (t))));
-      varinfo_t p = lookup_vi_for_tree (t);
-      if (p == NULL)
-	{
-	  p = create_variable_info_for_1 (t, alias_get_name (t), false);
-	  insert_vi_for_tree (t, p);
-	}
+      varinfo_t p = create_variable_info_for_1 (t, alias_get_name (t), false);
+      insert_vi_for_tree (t, p);
 
       /* For restrict qualified pointers build a representative for
 	 the pointed-to object.  Note that this ends up handling
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 3/6] Add recursion to make_param_constraints
  2015-10-27 13:04         ` [PATCH, 3/6] Add recursion " Tom de Vries
@ 2015-11-01 18:04           ` Tom de Vries
  2015-11-01 18:13             ` Tom de Vries
  2015-11-01 18:20             ` [PATCH, 2/2] Handle recursive restrict in function parameter Tom de Vries
  0 siblings, 2 replies; 35+ messages in thread
From: Tom de Vries @ 2015-11-01 18:04 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

On 27/10/15 13:56, Tom de Vries wrote:
> On 27/10/15 13:24, Tom de Vries wrote:
>> Thinking it over a bit more, I realized the constraint handling started
>> to be messy. I've reworked the patch series to simplify that first.
>>
>>       1    Simplify constraint handling
>>       2    Rename make_restrict_var_constraints to make_param_constraints
>>       3    Add recursion to make_param_constraints
>>       4    Add handle_param parameter to create_variable_info_for_1
>>       5    Handle recursive restrict pointer in
>> create_variable_info_for_1
>>       6    Handle restrict struct fields recursively
>>
>> Currently doing bootstrap and regtest on x86_64.
>>
>> I'll repost the patch series in reply to this message.
>>
>
> This patch:
> - registers the connection between a restrict pointer var and a
>    restrict var in a new hash_map restrict_pointed_var.
> - move the restrict pointer constraint handling from
>    intra_create_variable_infos to make_param_constraints
>
> The result of this and the two preceding patches is that the constraint
> handling for params in intra_create_variable_infos is reduced to a
> single call to make_param_constraints.

I've managed to eliminate this patch from the patch series, at the cost 
of having to merge patches 4-6 into a single patch, rather than having a 
more stepwise approach.

So, the new patch series is:

      1	Rename make_restrict_var_constraints to make_param_constraints
      2	Handle recursive restrict in function parameter

I'll repost in reply to this message.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 3/6] Add recursion to make_param_constraints
  2015-11-01 18:04           ` Tom de Vries
@ 2015-11-01 18:13             ` Tom de Vries
  2015-11-02 15:25               ` Richard Biener
  2015-11-01 18:20             ` [PATCH, 2/2] Handle recursive restrict in function parameter Tom de Vries
  1 sibling, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-11-01 18:13 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

On 01/11/15 19:03, Tom de Vries wrote:
>
> So, the new patch series is:
>
>       1    Rename make_restrict_var_constraints to make_param_constraints
>       2    Handle recursive restrict in function parameter
>
> I'll repost in reply to this message.

This no-functional-changes patch:
- moves the one constraint handling loop left in
    intra_create_variable_infos to make_restrict_var_constraints
- renames make_restrict_var_constraints to make_param_constraints
- adds a parameter toplevel to make_param_constraints to distinguish
    between the two calling contexts
- adds a parmeter restrict_name that allows to pass in the name of
   restrict vars

This patch was posted before at 
https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03111.html .

Thanks,
- Tom


[-- Attachment #2: 0001-Rename-make_restrict_var_constraints-to-make_param_c.patch --]
[-- Type: text/x-patch, Size: 2484 bytes --]

Rename make_restrict_var_constraints to make_param_constraints

2015-10-27  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (make_restrict_var_constraints): Rename to ...
	(make_param_constraints): ... this.  Add toplevel and restrict_name
	parameter.
	(intra_create_variable_infos): Use make_param_constraints.
---
 gcc/tree-ssa-structalias.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index d409727..ea34764 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5892,19 +5892,28 @@ debug_solution_for_var (unsigned int var)
   dump_solution_for_var (stderr, var);
 }
 
-/* Register the constraints for restrict var VI.  */
+/* Register the constraints for VI.  If TOPLEVEL then VI is a function
+   parameter, otherwise VI is part of a function parameter.  Use RESTRICT_NAME
+   as the base name of created restrict vars.  */
 
 static void
-make_restrict_var_constraints (varinfo_t vi)
+make_param_constraints (varinfo_t vi, bool toplevel, const char *restrict_name)
 {
   for (; vi; vi = vi_next (vi))
-    if (vi->may_have_pointers)
-      {
-	if (vi->only_restrict_pointers)
-	  make_constraint_from_global_restrict (vi, "GLOBAL_RESTRICT", true);
-	else
-	  make_copy_constraint (vi, nonlocal_id);
-      }
+    {
+      if (vi->only_restrict_pointers)
+	make_constraint_from_global_restrict (vi, restrict_name, true);
+      else if (vi->may_have_pointers)
+	{
+	  if (toplevel)
+	    make_constraint_from (vi, nonlocal_id);
+	  else
+	    make_copy_constraint (vi, nonlocal_id);
+	}
+
+    if (vi->is_full_var)
+      break;
+    }
 }
 
 /* Create varinfo structures for all of the variables in the
@@ -5941,19 +5950,11 @@ intra_create_variable_infos (struct function *fn)
 	  vi->is_restrict_var = 1;
 	  insert_vi_for_tree (heapvar, vi);
 	  make_constraint_from (p, vi->id);
-	  make_restrict_var_constraints (vi);
+	  make_param_constraints (vi, false, "GLOBAL_RESTRICT");
 	  continue;
 	}
 
-      for (; p; p = vi_next (p))
-	{
-	  if (p->only_restrict_pointers)
-	    make_constraint_from_global_restrict (p, "PARM_RESTRICT", true);
-	  else if (p->may_have_pointers)
-	    make_constraint_from (p, nonlocal_id);
-	  if (p->is_full_var)
-	    break;
-	}
+      make_param_constraints (p, true, "PARM_RESTRICT");
     }
 
   /* Add a constraint for a result decl that is passed by reference.  */
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH, 2/2] Handle recursive restrict in function parameter
  2015-11-01 18:04           ` Tom de Vries
  2015-11-01 18:13             ` Tom de Vries
@ 2015-11-01 18:20             ` Tom de Vries
  2015-11-03 13:47               ` Tom de Vries
  1 sibling, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-11-01 18:20 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 364 bytes --]

On 01/11/15 19:03, Tom de Vries wrote:
> So, the new patch series is:
>
>       1    Rename make_restrict_var_constraints to make_param_constraints
>       2    Handle 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


[-- Attachment #2: 0002-Handle-recursive-restrict-in-function-parameter.patch --]
[-- Type: text/x-patch, Size: 9758 bytes --]

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 0000000..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 0000000..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> 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<fieldoff_s> *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<fieldoff_s> *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<fieldoff_s> *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<fieldoff_s> *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;
+		  }
 		fieldstack->safe_push (e);
 	      }
 	  }
@@ -5655,10 +5674,11 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)
 
 /* 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.  */
+   of DECL.  DECL is a function parameter if HANDLE_PARAM is set.  */
 
 static varinfo_t
-create_variable_info_for_1 (tree decl, const char *name, bool add_id)
+create_variable_info_for_1 (tree decl, const char *name, bool add_id,
+			    bool handle_param)
 {
   varinfo_t vi, newvi;
   tree decl_type = TREE_TYPE (decl);
@@ -5692,7 +5712,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       bool notokay = false;
       unsigned int i;
 
-      push_fields_onto_fieldstack (decl_type, &fieldstack, 0);
+      push_fields_onto_fieldstack (decl_type, &fieldstack, 0, handle_param);
 
       for (i = 0; !notokay && fieldstack.iterate (i, &fo); i++)
 	if (fo->has_unknown_size
@@ -5734,6 +5754,19 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       if (POINTER_TYPE_P (TREE_TYPE (decl))
 	  && TYPE_RESTRICT (TREE_TYPE (decl)))
 	vi->only_restrict_pointers = 1;
+      if (vi->only_restrict_pointers
+	  && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (decl)))
+	  && handle_param)
+	{
+	  varinfo_t rvi;
+	  tree heapvar = build_fake_var_decl (TREE_TYPE (decl_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);
+	  make_constraint_from (vi, rvi->id);
+	  make_param_constraints (rvi, false);
+	}
       fieldstack.release ();
       return vi;
     }
@@ -5771,6 +5804,13 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       newvi->fullsize = vi->fullsize;
       newvi->may_have_pointers = fo->may_have_pointers;
       newvi->only_restrict_pointers = fo->only_restrict_pointers;
+      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, false);
+	}
       if (i + 1 < fieldstack.length ())
 	{
 	  varinfo_t tem = new_var_info (decl, name, false);
@@ -5785,7 +5825,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
 static unsigned int
 create_variable_info_for (tree decl, const char *name, bool add_id)
 {
-  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id);
+  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id, false);
   unsigned int id = vi->id;
 
   insert_vi_for_tree (decl, vi);
@@ -5897,12 +5937,12 @@ debug_solution_for_var (unsigned int var)
    as the base name of created restrict vars.  */
 
 static void
-make_param_constraints (varinfo_t vi, bool toplevel, const char *restrict_name)
+make_param_constraints (varinfo_t vi, bool toplevel)
 {
   for (; vi; vi = vi_next (vi))
     {
       if (vi->only_restrict_pointers)
-	make_constraint_from_global_restrict (vi, restrict_name, true);
+	;
       else if (vi->may_have_pointers)
 	{
 	  if (toplevel)
@@ -5929,32 +5969,11 @@ intra_create_variable_infos (struct function *fn)
      passed-by-reference argument.  */
   for (t = DECL_ARGUMENTS (fn->decl); t; t = DECL_CHAIN (t))
     {
-      bool restrict_pointer_p = (POINTER_TYPE_P (TREE_TYPE (t))
-				 && TYPE_RESTRICT (TREE_TYPE (t)));
-      bool recursive_restrict_p
-	= (restrict_pointer_p
-	   && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (t))));
-      varinfo_t p = create_variable_info_for_1 (t, alias_get_name (t), false);
+      varinfo_t p
+	= create_variable_info_for_1 (t, alias_get_name (t), false, true);
       insert_vi_for_tree (t, p);
 
-      /* For restrict qualified pointers build a representative for
-	 the pointed-to object.  Note that this ends up handling
-	 out-of-bound references conservatively by aggregating them
-	 in the first/last subfield of the object.  */
-      if (recursive_restrict_p)
-	{
-	  varinfo_t vi;
-	  tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
-	  DECL_EXTERNAL (heapvar) = 1;
-	  vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", true);
-	  vi->is_restrict_var = 1;
-	  insert_vi_for_tree (heapvar, vi);
-	  make_constraint_from (p, vi->id);
-	  make_param_constraints (vi, false, "GLOBAL_RESTRICT");
-	  continue;
-	}
-
-      make_param_constraints (p, true, "PARM_RESTRICT");
+      make_param_constraints (p, true);
     }
 
   /* Add a constraint for a result decl that is passed by reference.  */
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 3/6] Add recursion to make_param_constraints
  2015-11-01 18:13             ` Tom de Vries
@ 2015-11-02 15:25               ` Richard Biener
  2015-11-02 23:29                 ` Tom de Vries
  0 siblings, 1 reply; 35+ messages in thread
From: Richard Biener @ 2015-11-02 15:25 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Richard Biener, gcc-patches

On Sun, 1 Nov 2015, Tom de Vries wrote:

> On 01/11/15 19:03, Tom de Vries wrote:
> > 
> > So, the new patch series is:
> > 
> >       1    Rename make_restrict_var_constraints to make_param_constraints
> >       2    Handle recursive restrict in function parameter
> > 
> > I'll repost in reply to this message.
> 
> This no-functional-changes patch:
> - moves the one constraint handling loop left in
>    intra_create_variable_infos to make_restrict_var_constraints
> - renames make_restrict_var_constraints to make_param_constraints
> - adds a parameter toplevel to make_param_constraints to distinguish
>    between the two calling contexts
> - adds a parmeter restrict_name that allows to pass in the name of
>   restrict vars
> 
> This patch was posted before at
> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03111.html .

+         if (toplevel)
+           make_constraint_from (vi, nonlocal_id);
+         else
+           make_copy_constraint (vi, nonlocal_id);

I think make_constraint_from is what we want in both cases.

Ok with this change (thus drop the toplevel parameter).

Richard.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 3/6] Add recursion to make_param_constraints
  2015-11-02 15:25               ` Richard Biener
@ 2015-11-02 23:29                 ` Tom de Vries
  0 siblings, 0 replies; 35+ messages in thread
From: Tom de Vries @ 2015-11-02 23:29 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1313 bytes --]

On 02/11/15 16:25, Richard Biener wrote:
> On Sun, 1 Nov 2015, Tom de Vries wrote:
>
>> >On 01/11/15 19:03, Tom de Vries wrote:
>>> > >
>>> > >So, the new patch series is:
>>> > >
>>> > >       1    Rename make_restrict_var_constraints to make_param_constraints
>>> > >       2    Handle recursive restrict in function parameter
>>> > >
>>> > >I'll repost in reply to this message.
>> >
>> >This no-functional-changes patch:
>> >- moves the one constraint handling loop left in
>> >    intra_create_variable_infos to make_restrict_var_constraints
>> >- renames make_restrict_var_constraints to make_param_constraints
>> >- adds a parameter toplevel to make_param_constraints to distinguish
>> >    between the two calling contexts
>> >- adds a parmeter restrict_name that allows to pass in the name of
>> >   restrict vars
>> >
>> >This patch was posted before at
>> >https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03111.html  .
> +         if (toplevel)
> +           make_constraint_from (vi, nonlocal_id);
> +         else
> +           make_copy_constraint (vi, nonlocal_id);
>
> I think make_constraint_from is what we want in both cases.
>

Committed as separate patch, as attached (1st patch).

> Ok with this change (thus drop the toplevel parameter).
>

Committed as attached (2nd patch).

Thanks,
- Tom


[-- Attachment #2: 0001-Replace-make_copy_constraint-with-make_constraint_fr.patch --]
[-- Type: text/x-patch, Size: 810 bytes --]

Replace make_copy_constraint with make_constraint_from in make_restrict_var_constraints

2015-11-02  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (make_restrict_var_constraints): Replace
	make_copy_constraint call with make_constraint_from call.
---
 gcc/tree-ssa-structalias.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 55f72a2..773731d 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5890,7 +5890,7 @@ make_restrict_var_constraints (varinfo_t vi)
 	if (vi->only_restrict_pointers)
 	  make_constraint_from_global_restrict (vi, "GLOBAL_RESTRICT", true);
 	else
-	  make_copy_constraint (vi, nonlocal_id);
+	  make_constraint_from (vi, nonlocal_id);
       }
 }
 
-- 
1.9.1


[-- Attachment #3: 0002-Rename-make_restrict_var_constraints-to-make_param_c.patch --]
[-- Type: text/x-patch, Size: 2317 bytes --]

Rename make_restrict_var_constraints to make_param_constraints

2015-10-27  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-structalias.c (make_restrict_var_constraints): Rename to ...
	(make_param_constraints): ... this.  Add and handle restrict_name
	parameter.  Handle is_full_var case.
	(intra_create_variable_infos): Use make_param_constraints.
---
 gcc/tree-ssa-structalias.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 773731d..ded5a1e 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5879,19 +5879,22 @@ debug_solution_for_var (unsigned int var)
   dump_solution_for_var (stderr, var);
 }
 
-/* Register the constraints for restrict var VI.  */
+/* Register the constraints for function parameter related VI.  Use RESTRICT_NAME
+   as the base name of created restrict vars.  */
 
 static void
-make_restrict_var_constraints (varinfo_t vi)
+make_param_constraints (varinfo_t vi, const char *restrict_name)
 {
   for (; vi; vi = vi_next (vi))
-    if (vi->may_have_pointers)
-      {
-	if (vi->only_restrict_pointers)
-	  make_constraint_from_global_restrict (vi, "GLOBAL_RESTRICT", true);
-	else
-	  make_constraint_from (vi, nonlocal_id);
-      }
+    {
+      if (vi->only_restrict_pointers)
+	make_constraint_from_global_restrict (vi, restrict_name, true);
+      else if (vi->may_have_pointers)
+	make_constraint_from (vi, nonlocal_id);
+
+      if (vi->is_full_var)
+	break;
+    }
 }
 
 /* Create varinfo structures for all of the variables in the
@@ -5928,19 +5931,11 @@ intra_create_variable_infos (struct function *fn)
 	  vi->is_restrict_var = 1;
 	  insert_vi_for_tree (heapvar, vi);
 	  make_constraint_from (p, vi->id);
-	  make_restrict_var_constraints (vi);
+	  make_param_constraints (vi, "GLOBAL_RESTRICT");
 	  continue;
 	}
 
-      for (; p; p = vi_next (p))
-	{
-	  if (p->only_restrict_pointers)
-	    make_constraint_from_global_restrict (p, "PARM_RESTRICT", true);
-	  else if (p->may_have_pointers)
-	    make_constraint_from (p, nonlocal_id);
-	  if (p->is_full_var)
-	    break;
-	}
+      make_param_constraints (p, "PARM_RESTRICT");
     }
 
   /* Add a constraint for a result decl that is passed by reference.  */
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 2/2] Handle recursive restrict in function parameter
  2015-11-01 18:20             ` [PATCH, 2/2] Handle recursive restrict in function parameter Tom de Vries
@ 2015-11-03 13:47               ` Tom de Vries
  2015-11-03 13:59                 ` [gomp4,committed] " Tom de Vries
  2015-11-03 15:08                 ` [PATCH, 2/2] " Richard Biener
  0 siblings, 2 replies; 35+ messages in thread
From: Tom de Vries @ 2015-11-03 13:47 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

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:
>>
>>       1    Rename make_restrict_var_constraints to make_param_constraints
>>       2    Handle 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


[-- Attachment #2: 0001-Handle-recursive-restrict-in-function-parameter.patch --]
[-- Type: text/x-patch, Size: 9906 bytes --]

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 0000000..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 0000000..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> 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<fieldoff_s> *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<fieldoff_s> *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<fieldoff_s> *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, vec<fieldoff_s> *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;
+		  }
 		fieldstack->safe_push (e);
 	      }
 	  }
@@ -5642,10 +5661,11 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)
 
 /* 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.  */
+   of DECL.  DECL is a function parameter if HANDLE_PARAM is set.  */
 
 static varinfo_t
-create_variable_info_for_1 (tree decl, const char *name, bool add_id)
+create_variable_info_for_1 (tree decl, const char *name, bool add_id,
+			    bool handle_param)
 {
   varinfo_t vi, newvi;
   tree decl_type = TREE_TYPE (decl);
@@ -5679,7 +5699,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       bool notokay = false;
       unsigned int i;
 
-      push_fields_onto_fieldstack (decl_type, &fieldstack, 0);
+      push_fields_onto_fieldstack (decl_type, &fieldstack, 0, handle_param);
 
       for (i = 0; !notokay && fieldstack.iterate (i, &fo); i++)
 	if (fo->has_unknown_size
@@ -5721,6 +5741,19 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       if (POINTER_TYPE_P (TREE_TYPE (decl))
 	  && TYPE_RESTRICT (TREE_TYPE (decl)))
 	vi->only_restrict_pointers = 1;
+      if (vi->only_restrict_pointers
+	  && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (decl)))
+	  && handle_param)
+	{
+	  varinfo_t rvi;
+	  tree heapvar = build_fake_var_decl (TREE_TYPE (decl_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);
+	  make_constraint_from (vi, rvi->id);
+	  make_param_constraints (rvi);
+	}
       fieldstack.release ();
       return vi;
     }
@@ -5758,6 +5791,13 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       newvi->fullsize = vi->fullsize;
       newvi->may_have_pointers = fo->may_have_pointers;
       newvi->only_restrict_pointers = fo->only_restrict_pointers;
+      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);
+	}
       if (i + 1 < fieldstack.length ())
 	{
 	  varinfo_t tem = new_var_info (decl, name, false);
@@ -5772,7 +5812,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
 static unsigned int
 create_variable_info_for (tree decl, const char *name, bool add_id)
 {
-  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id);
+  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id, false);
   unsigned int id = vi->id;
 
   insert_vi_for_tree (decl, vi);
@@ -5879,16 +5919,15 @@ debug_solution_for_var (unsigned int var)
   dump_solution_for_var (stderr, var);
 }
 
-/* Register the constraints for function parameter related VI.  Use RESTRICT_NAME
-   as the base name of created restrict vars.  */
+/* Register the constraints for function parameter related VI.  */
 
 static void
-make_param_constraints (varinfo_t vi, const char *restrict_name)
+make_param_constraints (varinfo_t vi)
 {
   for (; vi; vi = vi_next (vi))
     {
       if (vi->only_restrict_pointers)
-	make_constraint_from_global_restrict (vi, restrict_name, true);
+	;
       else if (vi->may_have_pointers)
 	make_constraint_from (vi, nonlocal_id);
 
@@ -5910,32 +5949,11 @@ intra_create_variable_infos (struct function *fn)
      passed-by-reference argument.  */
   for (t = DECL_ARGUMENTS (fn->decl); t; t = DECL_CHAIN (t))
     {
-      bool restrict_pointer_p = (POINTER_TYPE_P (TREE_TYPE (t))
-				 && TYPE_RESTRICT (TREE_TYPE (t)));
-      bool recursive_restrict_p
-	= (restrict_pointer_p
-	   && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (t))));
-      varinfo_t p = create_variable_info_for_1 (t, alias_get_name (t), false);
+      varinfo_t p
+	= create_variable_info_for_1 (t, alias_get_name (t), false, true);
       insert_vi_for_tree (t, p);
 
-      /* For restrict qualified pointers build a representative for
-	 the pointed-to object.  Note that this ends up handling
-	 out-of-bound references conservatively by aggregating them
-	 in the first/last subfield of the object.  */
-      if (recursive_restrict_p)
-	{
-	  varinfo_t vi;
-	  tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
-	  DECL_EXTERNAL (heapvar) = 1;
-	  vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", true);
-	  vi->is_restrict_var = 1;
-	  insert_vi_for_tree (heapvar, vi);
-	  make_constraint_from (p, vi->id);
-	  make_param_constraints (vi, "GLOBAL_RESTRICT");
-	  continue;
-	}
-
-      make_param_constraints (p, "PARM_RESTRICT");
+      make_param_constraints (p);
     }
 
   /* Add a constraint for a result decl that is passed by reference.  */
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [gomp4,committed] Handle recursive restrict in function parameter
  2015-11-03 13:47               ` Tom de Vries
@ 2015-11-03 13:59                 ` Tom de Vries
  2015-11-04 17:01                   ` Tom de Vries
  2015-11-03 15:08                 ` [PATCH, 2/2] " Richard Biener
  1 sibling, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-11-03 13:59 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches, Jakub Jelinek

[ was: Re: [PATCH, 2/2] Handle recursive restrict in function parameter ]

On 03/11/15 14:46, 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:
>>>
>>>       1    Rename make_restrict_var_constraints to
>>> make_param_constraints
>>>       2    Handle 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.
>

And committed to gomp-4_0-branch.

Thanks,
- Tom

> 0001-Handle-recursive-restrict-in-function-parameter.patch
>
>
> 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 0000000..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 0000000..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> 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<fieldoff_s> *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<fieldoff_s> *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<fieldoff_s> *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, vec<fieldoff_s> *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;
> +		  }
>   		fieldstack->safe_push (e);
>   	      }
>   	  }
> @@ -5642,10 +5661,11 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)
>
>   /* 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.  */
> +   of DECL.  DECL is a function parameter if HANDLE_PARAM is set.  */
>
>   static varinfo_t
> -create_variable_info_for_1 (tree decl, const char *name, bool add_id)
> +create_variable_info_for_1 (tree decl, const char *name, bool add_id,
> +			    bool handle_param)
>   {
>     varinfo_t vi, newvi;
>     tree decl_type = TREE_TYPE (decl);
> @@ -5679,7 +5699,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
>         bool notokay = false;
>         unsigned int i;
>
> -      push_fields_onto_fieldstack (decl_type, &fieldstack, 0);
> +      push_fields_onto_fieldstack (decl_type, &fieldstack, 0, handle_param);
>
>         for (i = 0; !notokay && fieldstack.iterate (i, &fo); i++)
>   	if (fo->has_unknown_size
> @@ -5721,6 +5741,19 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
>         if (POINTER_TYPE_P (TREE_TYPE (decl))
>   	  && TYPE_RESTRICT (TREE_TYPE (decl)))
>   	vi->only_restrict_pointers = 1;
> +      if (vi->only_restrict_pointers
> +	  && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (decl)))
> +	  && handle_param)
> +	{
> +	  varinfo_t rvi;
> +	  tree heapvar = build_fake_var_decl (TREE_TYPE (decl_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);
> +	  make_constraint_from (vi, rvi->id);
> +	  make_param_constraints (rvi);
> +	}
>         fieldstack.release ();
>         return vi;
>       }
> @@ -5758,6 +5791,13 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
>         newvi->fullsize = vi->fullsize;
>         newvi->may_have_pointers = fo->may_have_pointers;
>         newvi->only_restrict_pointers = fo->only_restrict_pointers;
> +      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);
> +	}
>         if (i + 1 < fieldstack.length ())
>   	{
>   	  varinfo_t tem = new_var_info (decl, name, false);
> @@ -5772,7 +5812,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
>   static unsigned int
>   create_variable_info_for (tree decl, const char *name, bool add_id)
>   {
> -  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id);
> +  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id, false);
>     unsigned int id = vi->id;
>
>     insert_vi_for_tree (decl, vi);
> @@ -5879,16 +5919,15 @@ debug_solution_for_var (unsigned int var)
>     dump_solution_for_var (stderr, var);
>   }
>
> -/* Register the constraints for function parameter related VI.  Use RESTRICT_NAME
> -   as the base name of created restrict vars.  */
> +/* Register the constraints for function parameter related VI.  */
>
>   static void
> -make_param_constraints (varinfo_t vi, const char *restrict_name)
> +make_param_constraints (varinfo_t vi)
>   {
>     for (; vi; vi = vi_next (vi))
>       {
>         if (vi->only_restrict_pointers)
> -	make_constraint_from_global_restrict (vi, restrict_name, true);
> +	;
>         else if (vi->may_have_pointers)
>   	make_constraint_from (vi, nonlocal_id);
>
> @@ -5910,32 +5949,11 @@ intra_create_variable_infos (struct function *fn)
>        passed-by-reference argument.  */
>     for (t = DECL_ARGUMENTS (fn->decl); t; t = DECL_CHAIN (t))
>       {
> -      bool restrict_pointer_p = (POINTER_TYPE_P (TREE_TYPE (t))
> -				 && TYPE_RESTRICT (TREE_TYPE (t)));
> -      bool recursive_restrict_p
> -	= (restrict_pointer_p
> -	   && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (t))));
> -      varinfo_t p = create_variable_info_for_1 (t, alias_get_name (t), false);
> +      varinfo_t p
> +	= create_variable_info_for_1 (t, alias_get_name (t), false, true);
>         insert_vi_for_tree (t, p);
>
> -      /* For restrict qualified pointers build a representative for
> -	 the pointed-to object.  Note that this ends up handling
> -	 out-of-bound references conservatively by aggregating them
> -	 in the first/last subfield of the object.  */
> -      if (recursive_restrict_p)
> -	{
> -	  varinfo_t vi;
> -	  tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
> -	  DECL_EXTERNAL (heapvar) = 1;
> -	  vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", true);
> -	  vi->is_restrict_var = 1;
> -	  insert_vi_for_tree (heapvar, vi);
> -	  make_constraint_from (p, vi->id);
> -	  make_param_constraints (vi, "GLOBAL_RESTRICT");
> -	  continue;
> -	}
> -
> -      make_param_constraints (p, "PARM_RESTRICT");
> +      make_param_constraints (p);
>       }
>
>     /* Add a constraint for a result decl that is passed by reference.  */
> -- 1.9.1
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 2/2] Handle recursive restrict in function parameter
  2015-11-03 13:47               ` Tom de Vries
  2015-11-03 13:59                 ` [gomp4,committed] " Tom de Vries
@ 2015-11-03 15:08                 ` Richard Biener
  2015-11-03 16:44                   ` Tom de Vries
  1 sibling, 1 reply; 35+ messages in thread
From: Richard Biener @ 2015-11-03 15:08 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Richard Biener, gcc-patches

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:
> > > 
> > >       1    Rename make_restrict_var_constraints to make_param_constraints
> > >       2    Handle 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<fieldoff_s> *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.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 2/2] Handle recursive restrict in function parameter
  2015-11-03 15:08                 ` [PATCH, 2/2] " Richard Biener
@ 2015-11-03 16:44                   ` Tom de Vries
  2015-11-04  9:28                     ` Richard Biener
  0 siblings, 1 reply; 35+ messages in thread
From: Tom de Vries @ 2015-11-03 16:44 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 3319 bytes --]

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:
>>>>
>>>>        1    Rename make_restrict_var_constraints to make_param_constraints
>>>>        2    Handle 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<fieldoff_s> *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

[-- Attachment #2: 0001-Handle-recursive-restrict-in-function-parameter.patch --]
[-- Type: text/x-patch, Size: 9923 bytes --]

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 0000000..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 0000000..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..9e15a3c 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> 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;
+
+  unsigned 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<fieldoff_s> *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<fieldoff_s> *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<fieldoff_s> *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, 0};
 		pair = fieldstack->safe_push (e);
 	      }
 
@@ -5374,6 +5380,19 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *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->id;
+		  }
 		fieldstack->safe_push (e);
 	      }
 	  }
@@ -5642,10 +5661,11 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)
 
 /* 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.  */
+   of DECL.  DECL is a function parameter if HANDLE_PARAM is set.  */
 
 static varinfo_t
-create_variable_info_for_1 (tree decl, const char *name, bool add_id)
+create_variable_info_for_1 (tree decl, const char *name, bool add_id,
+			    bool handle_param)
 {
   varinfo_t vi, newvi;
   tree decl_type = TREE_TYPE (decl);
@@ -5679,7 +5699,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       bool notokay = false;
       unsigned int i;
 
-      push_fields_onto_fieldstack (decl_type, &fieldstack, 0);
+      push_fields_onto_fieldstack (decl_type, &fieldstack, 0, handle_param);
 
       for (i = 0; !notokay && fieldstack.iterate (i, &fo); i++)
 	if (fo->has_unknown_size
@@ -5721,6 +5741,20 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       if (POINTER_TYPE_P (TREE_TYPE (decl))
 	  && TYPE_RESTRICT (TREE_TYPE (decl)))
 	vi->only_restrict_pointers = 1;
+      if (vi->only_restrict_pointers
+	  && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (decl)))
+	  && handle_param)
+	{
+	  varinfo_t rvi;
+	  tree heapvar = build_fake_var_decl (TREE_TYPE (decl_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);
+	  make_constraint_from (vi, rvi->id);
+	  make_param_constraints (rvi);
+	}
       fieldstack.release ();
       return vi;
     }
@@ -5758,6 +5792,13 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       newvi->fullsize = vi->fullsize;
       newvi->may_have_pointers = fo->may_have_pointers;
       newvi->only_restrict_pointers = fo->only_restrict_pointers;
+      if (handle_param
+	  && newvi->only_restrict_pointers
+	  && fo->restrict_var != 0)
+	{
+	  make_constraint_from (newvi, fo->restrict_var);
+	  make_param_constraints (get_varinfo (fo->restrict_var));
+	}
       if (i + 1 < fieldstack.length ())
 	{
 	  varinfo_t tem = new_var_info (decl, name, false);
@@ -5772,7 +5813,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
 static unsigned int
 create_variable_info_for (tree decl, const char *name, bool add_id)
 {
-  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id);
+  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id, false);
   unsigned int id = vi->id;
 
   insert_vi_for_tree (decl, vi);
@@ -5879,16 +5920,15 @@ debug_solution_for_var (unsigned int var)
   dump_solution_for_var (stderr, var);
 }
 
-/* Register the constraints for function parameter related VI.  Use RESTRICT_NAME
-   as the base name of created restrict vars.  */
+/* Register the constraints for function parameter related VI.  */
 
 static void
-make_param_constraints (varinfo_t vi, const char *restrict_name)
+make_param_constraints (varinfo_t vi)
 {
   for (; vi; vi = vi_next (vi))
     {
       if (vi->only_restrict_pointers)
-	make_constraint_from_global_restrict (vi, restrict_name, true);
+	;
       else if (vi->may_have_pointers)
 	make_constraint_from (vi, nonlocal_id);
 
@@ -5910,32 +5950,11 @@ intra_create_variable_infos (struct function *fn)
      passed-by-reference argument.  */
   for (t = DECL_ARGUMENTS (fn->decl); t; t = DECL_CHAIN (t))
     {
-      bool restrict_pointer_p = (POINTER_TYPE_P (TREE_TYPE (t))
-				 && TYPE_RESTRICT (TREE_TYPE (t)));
-      bool recursive_restrict_p
-	= (restrict_pointer_p
-	   && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (t))));
-      varinfo_t p = create_variable_info_for_1 (t, alias_get_name (t), false);
+      varinfo_t p
+	= create_variable_info_for_1 (t, alias_get_name (t), false, true);
       insert_vi_for_tree (t, p);
 
-      /* For restrict qualified pointers build a representative for
-	 the pointed-to object.  Note that this ends up handling
-	 out-of-bound references conservatively by aggregating them
-	 in the first/last subfield of the object.  */
-      if (recursive_restrict_p)
-	{
-	  varinfo_t vi;
-	  tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
-	  DECL_EXTERNAL (heapvar) = 1;
-	  vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", true);
-	  vi->is_restrict_var = 1;
-	  insert_vi_for_tree (heapvar, vi);
-	  make_constraint_from (p, vi->id);
-	  make_param_constraints (vi, "GLOBAL_RESTRICT");
-	  continue;
-	}
-
-      make_param_constraints (p, "PARM_RESTRICT");
+      make_param_constraints (p);
     }
 
   /* Add a constraint for a result decl that is passed by reference.  */
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH, 2/2] Handle recursive restrict in function parameter
  2015-11-03 16:44                   ` Tom de Vries
@ 2015-11-04  9:28                     ` Richard Biener
  2015-11-04 14:25                       ` [committed] " Tom de Vries
  0 siblings, 1 reply; 35+ messages in thread
From: Richard Biener @ 2015-11-04  9:28 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Richard Biener, gcc-patches

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:
> > > > > 
> > > > >        1    Rename make_restrict_var_constraints to
> > > > > make_param_constraints
> > > > >        2    Handle 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<fieldoff_s> *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.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [committed] Handle recursive restrict in function parameter
  2015-11-04  9:28                     ` Richard Biener
@ 2015-11-04 14:25                       ` Tom de Vries
  0 siblings, 0 replies; 35+ messages in thread
From: Tom de Vries @ 2015-11-04 14:25 UTC (permalink / raw)
  To: Richard Biener; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 788 bytes --]

On 04/11/15 10:28, Richard Biener wrote:
>> 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.

Updated patch accordingly.

Bootstrapped and reg-tested on x86_64.

Committed to trunk as attached.

Thanks,
- Tom

[-- Attachment #2: 0002-Handle-recursive-restrict-in-function-parameter.patch --]
[-- Type: text/x-patch, Size: 8408 bytes --]

Handle recursive restrict in function parameter

2015-11-04  Tom de Vries  <tom@codesourcery.com>

	PR tree-optimization/67742
	* tree-ssa-structalias.c (struct fieldoff): Add restrict_pointed_type
	field.
	(push_fields_onto_fieldstack): Handle restrict_pointed_type field.
	(create_variable_info_for_1): Add and handle handle_param parameter.
	Add restrict handling.
	(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                 | 78 +++++++++++++++++-------------
 3 files changed, 74 insertions(+), 33 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 0000000..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 0000000..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 98b5f16..52a35f6 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> variable_info_pool
@@ -393,7 +394,6 @@ new_var_info (tree t, const char *name, bool add_id)
   return ret;
 }
 
-
 /* A map mapping call statements to per-stmt variables for uses
    and clobbers specific to the call.  */
 static hash_map<gimple *, varinfo_t> *call_stmt_vars;
@@ -5195,6 +5195,8 @@ struct fieldoff
   unsigned may_have_pointers : 1;
 
   unsigned only_restrict_pointers : 1;
+
+  tree restrict_pointed_type;
 };
 typedef struct fieldoff fieldoff_s;
 
@@ -5340,7 +5342,8 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *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_TREE};
 		pair = fieldstack->safe_push (e);
 	      }
 
@@ -5374,6 +5377,8 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *fieldstack,
 		  = (!has_unknown_size
 		     && POINTER_TYPE_P (field_type)
 		     && TYPE_RESTRICT (field_type));
+		if (e.only_restrict_pointers)
+		  e.restrict_pointed_type = TREE_TYPE (field_type);
 		fieldstack->safe_push (e);
 	      }
 	  }
@@ -5642,10 +5647,11 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)
 
 /* 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.  */
+   of DECL.  DECL is a function parameter if HANDLE_PARAM is set.  */
 
 static varinfo_t
-create_variable_info_for_1 (tree decl, const char *name, bool add_id)
+create_variable_info_for_1 (tree decl, const char *name, bool add_id,
+			    bool handle_param)
 {
   varinfo_t vi, newvi;
   tree decl_type = TREE_TYPE (decl);
@@ -5721,6 +5727,20 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       if (POINTER_TYPE_P (decl_type)
 	  && TYPE_RESTRICT (decl_type))
 	vi->only_restrict_pointers = 1;
+      if (vi->only_restrict_pointers
+	  && !type_contains_placeholder_p (TREE_TYPE (decl_type))
+	  && handle_param)
+	{
+	  varinfo_t rvi;
+	  tree heapvar = build_fake_var_decl (TREE_TYPE (decl_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);
+	  make_constraint_from (vi, rvi->id);
+	  make_param_constraints (rvi);
+	}
       fieldstack.release ();
       return vi;
     }
@@ -5758,6 +5778,20 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       newvi->fullsize = vi->fullsize;
       newvi->may_have_pointers = fo->may_have_pointers;
       newvi->only_restrict_pointers = fo->only_restrict_pointers;
+      if (handle_param
+	  && newvi->only_restrict_pointers
+	  && !type_contains_placeholder_p (fo->restrict_pointed_type))
+	{
+	  varinfo_t rvi;
+	  tree heapvar = build_fake_var_decl (fo->restrict_pointed_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);
+	  make_constraint_from (newvi, rvi->id);
+	  make_param_constraints (rvi);
+	}
       if (i + 1 < fieldstack.length ())
 	{
 	  varinfo_t tem = new_var_info (decl, name, false);
@@ -5772,7 +5806,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
 static unsigned int
 create_variable_info_for (tree decl, const char *name, bool add_id)
 {
-  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id);
+  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id, false);
   unsigned int id = vi->id;
 
   insert_vi_for_tree (decl, vi);
@@ -5879,16 +5913,15 @@ debug_solution_for_var (unsigned int var)
   dump_solution_for_var (stderr, var);
 }
 
-/* Register the constraints for function parameter related VI.  Use RESTRICT_NAME
-   as the base name of created restrict vars.  */
+/* Register the constraints for function parameter related VI.  */
 
 static void
-make_param_constraints (varinfo_t vi, const char *restrict_name)
+make_param_constraints (varinfo_t vi)
 {
   for (; vi; vi = vi_next (vi))
     {
       if (vi->only_restrict_pointers)
-	make_constraint_from_global_restrict (vi, restrict_name, true);
+	;
       else if (vi->may_have_pointers)
 	make_constraint_from (vi, nonlocal_id);
 
@@ -5910,32 +5943,11 @@ intra_create_variable_infos (struct function *fn)
      passed-by-reference argument.  */
   for (t = DECL_ARGUMENTS (fn->decl); t; t = DECL_CHAIN (t))
     {
-      bool restrict_pointer_p = (POINTER_TYPE_P (TREE_TYPE (t))
-				 && TYPE_RESTRICT (TREE_TYPE (t)));
-      bool recursive_restrict_p
-	= (restrict_pointer_p
-	   && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (t))));
-      varinfo_t p = create_variable_info_for_1 (t, alias_get_name (t), false);
+      varinfo_t p
+	= create_variable_info_for_1 (t, alias_get_name (t), false, true);
       insert_vi_for_tree (t, p);
 
-      /* For restrict qualified pointers build a representative for
-	 the pointed-to object.  Note that this ends up handling
-	 out-of-bound references conservatively by aggregating them
-	 in the first/last subfield of the object.  */
-      if (recursive_restrict_p)
-	{
-	  varinfo_t vi;
-	  tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
-	  DECL_EXTERNAL (heapvar) = 1;
-	  vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", true);
-	  vi->is_restrict_var = 1;
-	  insert_vi_for_tree (heapvar, vi);
-	  make_constraint_from (p, vi->id);
-	  make_param_constraints (vi, "GLOBAL_RESTRICT");
-	  continue;
-	}
-
-      make_param_constraints (p, "PARM_RESTRICT");
+      make_param_constraints (p);
     }
 
   /* Add a constraint for a result decl that is passed by reference.  */
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [gomp4,committed] Handle recursive restrict in function parameter
  2015-11-03 13:59                 ` [gomp4,committed] " Tom de Vries
@ 2015-11-04 17:01                   ` Tom de Vries
  0 siblings, 0 replies; 35+ messages in thread
From: Tom de Vries @ 2015-11-04 17:01 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 282 bytes --]

On 03/11/15 14:58, Tom de Vries wrote:
>>> This patch adds handling of all the restrict qualifiers in the type of a
>>> function parameter.

> And committed to gomp-4_0-branch.

I've reverted this patch, and backported the version from trunk.

Committed as attached.

Thanks,
- Tom

[-- Attachment #2: 0001-Revert-Handle-recursive-restrict-in-function-paramet.patch --]
[-- Type: text/x-patch, Size: 10045 bytes --]

[PATCH 1/2] Revert "Handle recursive restrict in function parameter"

2015-11-04  Tom de Vries  <tom@codesourcery.com>

	revert:
	2015-10-03  Tom de Vries  <tom@codesourcery.com>

	* 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, 36 insertions(+), 83 deletions(-)
 delete mode 100644 gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c
 delete 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
deleted file mode 100644
index f7a68c7..0000000
--- a/gcc/testsuite/gcc.dg/tree-ssa/restrict-7.c
+++ /dev/null
@@ -1,12 +0,0 @@
-/* { 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
deleted file mode 100644
index b0ab164..0000000
--- a/gcc/testsuite/gcc.dg/tree-ssa/restrict-8.c
+++ /dev/null
@@ -1,17 +0,0 @@
-/* { 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 074285c..f4c875f 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -320,7 +320,6 @@ 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> variable_info_pool
@@ -407,7 +406,6 @@ 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.  */
@@ -5210,8 +5208,6 @@ struct fieldoff
   unsigned may_have_pointers : 1;
 
   unsigned only_restrict_pointers : 1;
-
-  varinfo_t restrict_var;
 };
 typedef struct fieldoff fieldoff_s;
 
@@ -5306,12 +5302,11 @@ 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.  If HANDLE_PARAM is set, we're handling part of a function
-   parameter.  */
+   recursed for.  */
 
 static bool
 push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *fieldstack,
-			     HOST_WIDE_INT offset, bool handle_param)
+			     HOST_WIDE_INT offset)
 {
   tree field;
   bool empty_p = true;
@@ -5337,7 +5332,7 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *fieldstack,
 	    || TREE_CODE (field_type) == UNION_TYPE)
 	  push = true;
 	else if (!push_fields_onto_fieldstack
-		    (field_type, fieldstack, offset + foff, handle_param)
+		    (field_type, fieldstack, offset + foff)
 		 && (DECL_SIZE (field)
 		     && !integer_zerop (DECL_SIZE (field))))
 	  /* Empty structures may have actual size, like in C++.  So
@@ -5358,8 +5353,7 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *fieldstack,
 	    if (!pair
 		&& offset + foff != 0)
 	      {
-		fieldoff_s e = {0, offset + foff, false, false, false, false,
-				NULL};
+		fieldoff_s e = {0, offset + foff, false, false, false, false};
 		pair = fieldstack->safe_push (e);
 	      }
 
@@ -5393,19 +5387,6 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *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;
-		  }
 		fieldstack->safe_push (e);
 	      }
 	  }
@@ -5674,11 +5655,10 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)
 
 /* 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.  */
+   of DECL.  */
 
 static varinfo_t
-create_variable_info_for_1 (tree decl, const char *name, bool add_id,
-			    bool handle_param)
+create_variable_info_for_1 (tree decl, const char *name, bool add_id)
 {
   varinfo_t vi, newvi;
   tree decl_type = TREE_TYPE (decl);
@@ -5712,7 +5692,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id,
       bool notokay = false;
       unsigned int i;
 
-      push_fields_onto_fieldstack (decl_type, &fieldstack, 0, handle_param);
+      push_fields_onto_fieldstack (decl_type, &fieldstack, 0);
 
       for (i = 0; !notokay && fieldstack.iterate (i, &fo); i++)
 	if (fo->has_unknown_size
@@ -5754,19 +5734,6 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id,
       if (POINTER_TYPE_P (TREE_TYPE (decl))
 	  && TYPE_RESTRICT (TREE_TYPE (decl)))
 	vi->only_restrict_pointers = 1;
-      if (vi->only_restrict_pointers
-	  && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (decl)))
-	  && handle_param)
-	{
-	  varinfo_t rvi;
-	  tree heapvar = build_fake_var_decl (TREE_TYPE (decl_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);
-	  make_constraint_from (vi, rvi->id);
-	  make_param_constraints (rvi);
-	}
       fieldstack.release ();
       return vi;
     }
@@ -5804,13 +5771,6 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id,
       newvi->fullsize = vi->fullsize;
       newvi->may_have_pointers = fo->may_have_pointers;
       newvi->only_restrict_pointers = fo->only_restrict_pointers;
-      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);
-	}
       if (i + 1 < fieldstack.length ())
 	{
 	  varinfo_t tem = new_var_info (decl, name, false);
@@ -5825,7 +5785,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id,
 static unsigned int
 create_variable_info_for (tree decl, const char *name, bool add_id)
 {
-  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id, false);
+  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id);
   unsigned int id = vi->id;
 
   insert_vi_for_tree (decl, vi);
@@ -5932,15 +5892,16 @@ debug_solution_for_var (unsigned int var)
   dump_solution_for_var (stderr, var);
 }
 
-/* Register the constraints for function parameter related VI.  */
+/* Register the constraints for function parameter related VI.  Use RESTRICT_NAME
+   as the base name of created restrict vars.  */
 
 static void
-make_param_constraints (varinfo_t vi)
+make_param_constraints (varinfo_t vi, const char *restrict_name)
 {
   for (; vi; vi = vi_next (vi))
     {
       if (vi->only_restrict_pointers)
-	;
+	make_constraint_from_global_restrict (vi, restrict_name, true);
       else if (vi->may_have_pointers)
 	make_constraint_from (vi, nonlocal_id);
 
@@ -5962,11 +5923,32 @@ intra_create_variable_infos (struct function *fn)
      passed-by-reference argument.  */
   for (t = DECL_ARGUMENTS (fn->decl); t; t = DECL_CHAIN (t))
     {
-      varinfo_t p
-	= create_variable_info_for_1 (t, alias_get_name (t), false, true);
+      bool restrict_pointer_p = (POINTER_TYPE_P (TREE_TYPE (t))
+				 && TYPE_RESTRICT (TREE_TYPE (t)));
+      bool recursive_restrict_p
+	= (restrict_pointer_p
+	   && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (t))));
+      varinfo_t p = create_variable_info_for_1 (t, alias_get_name (t), false);
       insert_vi_for_tree (t, p);
 
-      make_param_constraints (p);
+      /* For restrict qualified pointers build a representative for
+	 the pointed-to object.  Note that this ends up handling
+	 out-of-bound references conservatively by aggregating them
+	 in the first/last subfield of the object.  */
+      if (recursive_restrict_p)
+	{
+	  varinfo_t vi;
+	  tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
+	  DECL_EXTERNAL (heapvar) = 1;
+	  vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", true);
+	  vi->is_restrict_var = 1;
+	  insert_vi_for_tree (heapvar, vi);
+	  make_constraint_from (p, vi->id);
+	  make_param_constraints (vi, "GLOBAL_RESTRICT");
+	  continue;
+	}
+
+      make_param_constraints (p, "PARM_RESTRICT");
     }
 
   /* Add a constraint for a result decl that is passed by reference.  */
-- 
1.9.1


[-- Attachment #3: 0002-Backport-Handle-recursive-restrict-in-function-param.patch --]
[-- Type: text/x-patch, Size: 8782 bytes --]

[PATCH 2/2] Backport "Handle recursive restrict in function parameter"

2015-11-04  Tom de Vries  <tom@codesourcery.com>

	backport from trunk:
	2015-11-04  Tom de Vries  <tom@codesourcery.com>

	PR tree-optimization/67742
	* tree-ssa-structalias.c (struct fieldoff): Add restrict_pointed_type
	field.
	(push_fields_onto_fieldstack): Handle restrict_pointed_type field.
	(create_variable_info_for_1): Add and handle handle_param parameter.
	Add restrict handling.
	(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.

	* tree-ssa-structalias.c (create_variable_info_for_1): Use decl_type
	variable.
---
 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                 | 82 +++++++++++++++++-------------
 3 files changed, 76 insertions(+), 35 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 0000000..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 0000000..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 f4c875f..f681b17 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);
 
 /* Pool of variable info structures.  */
 static object_allocator<variable_info> variable_info_pool
@@ -406,7 +407,6 @@ new_var_info (tree t, const char *name, bool add_id)
   return ret;
 }
 
-
 /* A map mapping call statements to per-stmt variables for uses
    and clobbers specific to the call.  */
 static hash_map<gimple *, varinfo_t> *call_stmt_vars;
@@ -5208,6 +5208,8 @@ struct fieldoff
   unsigned may_have_pointers : 1;
 
   unsigned only_restrict_pointers : 1;
+
+  tree restrict_pointed_type;
 };
 typedef struct fieldoff fieldoff_s;
 
@@ -5353,7 +5355,8 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *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_TREE};
 		pair = fieldstack->safe_push (e);
 	      }
 
@@ -5387,6 +5390,8 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *fieldstack,
 		  = (!has_unknown_size
 		     && POINTER_TYPE_P (field_type)
 		     && TYPE_RESTRICT (field_type));
+		if (e.only_restrict_pointers)
+		  e.restrict_pointed_type = TREE_TYPE (field_type);
 		fieldstack->safe_push (e);
 	      }
 	  }
@@ -5655,10 +5660,11 @@ check_for_overlaps (vec<fieldoff_s> fieldstack)
 
 /* 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.  */
+   of DECL.  DECL is a function parameter if HANDLE_PARAM is set.  */
 
 static varinfo_t
-create_variable_info_for_1 (tree decl, const char *name, bool add_id)
+create_variable_info_for_1 (tree decl, const char *name, bool add_id,
+			    bool handle_param)
 {
   varinfo_t vi, newvi;
   tree decl_type = TREE_TYPE (decl);
@@ -5731,9 +5737,23 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       vi->fullsize = tree_to_uhwi (declsize);
       vi->size = vi->fullsize;
       vi->is_full_var = true;
-      if (POINTER_TYPE_P (TREE_TYPE (decl))
-	  && TYPE_RESTRICT (TREE_TYPE (decl)))
+      if (POINTER_TYPE_P (decl_type)
+	  && TYPE_RESTRICT (decl_type))
 	vi->only_restrict_pointers = 1;
+      if (vi->only_restrict_pointers
+	  && !type_contains_placeholder_p (TREE_TYPE (decl_type))
+	  && handle_param)
+	{
+	  varinfo_t rvi;
+	  tree heapvar = build_fake_var_decl (TREE_TYPE (decl_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);
+	  make_constraint_from (vi, rvi->id);
+	  make_param_constraints (rvi);
+	}
       fieldstack.release ();
       return vi;
     }
@@ -5771,6 +5791,20 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
       newvi->fullsize = vi->fullsize;
       newvi->may_have_pointers = fo->may_have_pointers;
       newvi->only_restrict_pointers = fo->only_restrict_pointers;
+      if (handle_param
+	  && newvi->only_restrict_pointers
+	  && !type_contains_placeholder_p (fo->restrict_pointed_type))
+	{
+	  varinfo_t rvi;
+	  tree heapvar = build_fake_var_decl (fo->restrict_pointed_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);
+	  make_constraint_from (newvi, rvi->id);
+	  make_param_constraints (rvi);
+	}
       if (i + 1 < fieldstack.length ())
 	{
 	  varinfo_t tem = new_var_info (decl, name, false);
@@ -5785,7 +5819,7 @@ create_variable_info_for_1 (tree decl, const char *name, bool add_id)
 static unsigned int
 create_variable_info_for (tree decl, const char *name, bool add_id)
 {
-  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id);
+  varinfo_t vi = create_variable_info_for_1 (decl, name, add_id, false);
   unsigned int id = vi->id;
 
   insert_vi_for_tree (decl, vi);
@@ -5892,16 +5926,15 @@ debug_solution_for_var (unsigned int var)
   dump_solution_for_var (stderr, var);
 }
 
-/* Register the constraints for function parameter related VI.  Use RESTRICT_NAME
-   as the base name of created restrict vars.  */
+/* Register the constraints for function parameter related VI.  */
 
 static void
-make_param_constraints (varinfo_t vi, const char *restrict_name)
+make_param_constraints (varinfo_t vi)
 {
   for (; vi; vi = vi_next (vi))
     {
       if (vi->only_restrict_pointers)
-	make_constraint_from_global_restrict (vi, restrict_name, true);
+	;
       else if (vi->may_have_pointers)
 	make_constraint_from (vi, nonlocal_id);
 
@@ -5923,32 +5956,11 @@ intra_create_variable_infos (struct function *fn)
      passed-by-reference argument.  */
   for (t = DECL_ARGUMENTS (fn->decl); t; t = DECL_CHAIN (t))
     {
-      bool restrict_pointer_p = (POINTER_TYPE_P (TREE_TYPE (t))
-				 && TYPE_RESTRICT (TREE_TYPE (t)));
-      bool recursive_restrict_p
-	= (restrict_pointer_p
-	   && !type_contains_placeholder_p (TREE_TYPE (TREE_TYPE (t))));
-      varinfo_t p = create_variable_info_for_1 (t, alias_get_name (t), false);
+      varinfo_t p
+	= create_variable_info_for_1 (t, alias_get_name (t), false, true);
       insert_vi_for_tree (t, p);
 
-      /* For restrict qualified pointers build a representative for
-	 the pointed-to object.  Note that this ends up handling
-	 out-of-bound references conservatively by aggregating them
-	 in the first/last subfield of the object.  */
-      if (recursive_restrict_p)
-	{
-	  varinfo_t vi;
-	  tree heapvar = build_fake_var_decl (TREE_TYPE (TREE_TYPE (t)));
-	  DECL_EXTERNAL (heapvar) = 1;
-	  vi = create_variable_info_for_1 (heapvar, "PARM_NOALIAS", true);
-	  vi->is_restrict_var = 1;
-	  insert_vi_for_tree (heapvar, vi);
-	  make_constraint_from (p, vi->id);
-	  make_param_constraints (vi, "GLOBAL_RESTRICT");
-	  continue;
-	}
-
-      make_param_constraints (p, "PARM_RESTRICT");
+      make_param_constraints (p);
     }
 
   /* Add a constraint for a result decl that is passed by reference.  */
-- 
1.9.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2015-11-04 17:01 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-26 11:23 [PATCH, 1/2] Add handle_param parameter to create_variable_info_for_1 Tom de Vries
2015-10-26 13:46 ` Richard Biener
2015-10-26 16:26   ` Tom de Vries
2015-10-27  7:59     ` [PATCH, PR67742] Handle restrict struct fields recursively Tom de Vries
2015-10-27 12:26       ` Tom de Vries
2015-10-27 12:49         ` [PATCH, 1/6] Simplify constraint handling Tom de Vries
2015-10-28 15:38           ` Richard Biener
2015-10-28 16:06             ` Tom de Vries
2015-10-28 21:32               ` Tom de Vries
2015-10-29 11:16               ` Richard Biener
2015-10-29 12:31                 ` Tom de Vries
2015-10-29 13:15                   ` Richard Biener
2015-10-29 16:10                     ` Tom de Vries
2015-10-30  9:44                       ` Richard Biener
2015-10-31  8:24                         ` Tom de Vries
2015-10-31  9:44                           ` [committed, trivial] Don't expect existing varinfo for arguments in intra_create_variable_infos Tom de Vries
2015-10-27 12:50         ` [PATCH, 2/6] Rename make_restrict_var_constraints to make_param_constraints Tom de Vries
2015-10-28 15:45           ` Richard Biener
2015-10-28 21:56             ` Tom de Vries
2015-10-27 13:04         ` [PATCH, 3/6] Add recursion " Tom de Vries
2015-11-01 18:04           ` Tom de Vries
2015-11-01 18:13             ` Tom de Vries
2015-11-02 15:25               ` Richard Biener
2015-11-02 23:29                 ` Tom de Vries
2015-11-01 18:20             ` [PATCH, 2/2] Handle recursive restrict in function parameter Tom de Vries
2015-11-03 13:47               ` Tom de Vries
2015-11-03 13:59                 ` [gomp4,committed] " Tom de Vries
2015-11-04 17:01                   ` Tom de Vries
2015-11-03 15:08                 ` [PATCH, 2/2] " Richard Biener
2015-11-03 16:44                   ` Tom de Vries
2015-11-04  9:28                     ` Richard Biener
2015-11-04 14:25                       ` [committed] " Tom de Vries
2015-10-27 13:06         ` [PATCH, 4/6] Add handle_param parameter to create_variable_info_for_1 Tom de Vries
2015-10-27 13:12         ` [PATCH, 5/6] Handle recursive restrict pointer in create_variable_info_for_1 Tom de Vries
2015-10-27 13:17         ` [PATCH, 6/6] Handle restrict struct fields recursively Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).