Set nonnull attribute to ptr_info_def based on VRP

Set nonnull attribute to ptr_info_def based on VRP

[kugan]

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

Hi,

This patch tries to add nonnull_p attribute to ptr_info_def. As it 
stands, range_info_def is bigger in size than ptr_info_def. Therefore 
adding  nonnull_p should not change the size.

I am setting this based on the VRP analysis. Right there is no uses for 
this. But the idea is, this can be used in IPA-VRP such that redundant 
check for NULL can be eliminated.

Bootstrapped and regression tested on x86_64-linux-gnu. Is this OK for 
trunk.

Thanks,
Kugan


gcc/ChangeLog:

2016-08-08  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* tree-ssanames.c (set_ptr_nonnull): New.
	(get_ptr_nonnull): Likewise.
	* tree-ssanames.h (struct ptr_info_def): Add nonnull_p attribute.
	* tree-vrp.c (vrp_finalize): Call set_ptr_nonnull.

[-- Attachment #2: p2.txt --]
[-- Type: text/plain, Size: 3219 bytes --]

diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 91a8f97..19c9027 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -374,6 +374,27 @@ get_range_info (const_tree name, wide_int *min, wide_int *max)
   return SSA_NAME_RANGE_TYPE (name);
 }
 
+/* Set nonnull attribute to pointer NAME.  */
+
+void
+set_ptr_nonnull (tree name, bool nonnull_p)
+{
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+  struct ptr_info_def *pi = get_ptr_info (name);
+  pi->nonnull_p = nonnull_p;;
+}
+
+/* Return nonnull attribute of pointer NAME.  */
+bool
+get_ptr_nonnull (tree name)
+{
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+  struct ptr_info_def *pi = SSA_NAME_PTR_INFO (name);
+  if (pi == NULL)
+    return false;
+  return pi->nonnull_p;
+}
+
 /* Change non-zero bits bitmask of NAME.  */
 
 void
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index c81b1a1..6e34433 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -43,6 +43,9 @@ struct GTY(()) ptr_info_def
      above alignment.  Access only through the same helper functions as align
      above.  */
   unsigned int misalign;
+  /* When this pointer is knonw to be nnonnull this would be true otherwise
+     false.  */
+  bool  nonnull_p;
 };
 
 /* Value range information for SSA_NAMEs representing non-pointer variables.  */
@@ -89,6 +92,8 @@ extern void set_ptr_info_alignment (struct ptr_info_def *, unsigned int,
 extern void adjust_ptr_info_misalignment (struct ptr_info_def *,
 					  unsigned int);
 extern struct ptr_info_def *get_ptr_info (tree);
+extern void set_ptr_nonnull (tree name, bool nonnull_p);
+extern bool get_ptr_nonnull (tree name);
 
 extern tree copy_ssa_name_fn (struct function *, tree, gimple *);
 extern void duplicate_ssa_name_ptr_info (tree, struct ptr_info_def *);
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 7c7ad91..40c4d48 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -10352,18 +10352,24 @@ vrp_finalize (bool warn_array_bounds_p)
       {
 	tree name = ssa_name (i);
 
-      if (!name
-	  || POINTER_TYPE_P (TREE_TYPE (name))
-	  || (vr_value[i]->type == VR_VARYING)
-	  || (vr_value[i]->type == VR_UNDEFINED))
-	continue;
+	if (!name
+	    || (vr_value[i]->type == VR_VARYING)
+	    || (vr_value[i]->type == VR_UNDEFINED)
+	    || (TREE_CODE (vr_value[i]->min) != INTEGER_CST)
+	    || (TREE_CODE (vr_value[i]->max) != INTEGER_CST))
+	  continue;
 
-      if ((TREE_CODE (vr_value[i]->min) == INTEGER_CST)
-	  && (TREE_CODE (vr_value[i]->max) == INTEGER_CST)
-	  && (vr_value[i]->type == VR_RANGE
-	      || vr_value[i]->type == VR_ANTI_RANGE))
-	set_range_info (name, vr_value[i]->type, vr_value[i]->min,
-			vr_value[i]->max);
+	if (POINTER_TYPE_P (TREE_TYPE (name))
+	    && ((vr_value[i]->type == VR_RANGE
+		 && range_includes_zero_p (vr_value[i]->min,
+					   vr_value[i]->max) == 0)
+		|| (vr_value[i]->type == VR_ANTI_RANGE
+		    && range_includes_zero_p (vr_value[i]->min,
+					      vr_value[i]->max) == 1)))
+	  set_ptr_nonnull (name, true);
+	else if (!POINTER_TYPE_P (TREE_TYPE (name)))
+	  set_range_info (name, vr_value[i]->type, vr_value[i]->min,
+			  vr_value[i]->max);
       }
 
   substitute_and_fold (op_with_constant_singleton_value_range,

Re: Set nonnull attribute to ptr_info_def based on VRP

[Jakub Jelinek]

On Mon, Aug 08, 2016 at 01:36:51PM +1000, kugan wrote:
> diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
> index c81b1a1..6e34433 100644
> --- a/gcc/tree-ssanames.h
> +++ b/gcc/tree-ssanames.h
> @@ -43,6 +43,9 @@ struct GTY(()) ptr_info_def
>       above alignment.  Access only through the same helper functions as align
>       above.  */
>    unsigned int misalign;
> +  /* When this pointer is knonw to be nnonnull this would be true otherwise
> +     false.  */
> +  bool  nonnull_p;
>  };

Why do you need this?  Doesn't the pt.null bit represent that already?
Also, formatting and spelling:
s/knonw/known/
s/nnon/non/
s/bool  /bool /

	Jakub

Re: Set nonnull attribute to ptr_info_def based on VRP

[kugan]

Hi Jakub,

Thanks for the review.

On 08/08/16 16:40, Jakub Jelinek wrote:
> On Mon, Aug 08, 2016 at 01:36:51PM +1000, kugan wrote:
>> diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
>> index c81b1a1..6e34433 100644
>> --- a/gcc/tree-ssanames.h
>> +++ b/gcc/tree-ssanames.h
>> @@ -43,6 +43,9 @@ struct GTY(()) ptr_info_def
>>       above alignment.  Access only through the same helper functions as align
>>       above.  */
>>    unsigned int misalign;
>> +  /* When this pointer is knonw to be nnonnull this would be true otherwise
>> +     false.  */
>> +  bool  nonnull_p;
>>  };
>
> Why do you need this?  Doesn't the pt.null bit represent that already?

It looks like I can use this. As in gcc/tree-ssa-alias.h:

   /* Nonzero if the points-to set includes 'nothing', the points-to set
      includes memory at address NULL.  */
   unsigned int null : 1;

But in gcc/tree-ssa-alias.c, ptrs_compare_unequal has the following 
comment which says:

/* ???  We'd like to handle ptr1 != NULL and ptr1 != ptr2
      but those require pt.null to be conservatively correct.  */

Does that means it can be wrong at times? I haven't looked it in detail 
yet but if it is, it would be a problem.

> Also, formatting and spelling:
> s/knonw/known/
> s/nnon/non/
> s/bool  /bool /

I will change this.

Thanks,
Kugan
>
> 	Jakub
>

Re: Set nonnull attribute to ptr_info_def based on VRP

[Richard Biener]

On Tue, Aug 9, 2016 at 12:58 AM, kugan
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Jakub,
>
> Thanks for the review.
>
> On 08/08/16 16:40, Jakub Jelinek wrote:
>>
>> On Mon, Aug 08, 2016 at 01:36:51PM +1000, kugan wrote:
>>>
>>> diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
>>> index c81b1a1..6e34433 100644
>>> --- a/gcc/tree-ssanames.h
>>> +++ b/gcc/tree-ssanames.h
>>> @@ -43,6 +43,9 @@ struct GTY(()) ptr_info_def
>>>       above alignment.  Access only through the same helper functions as
>>> align
>>>       above.  */
>>>    unsigned int misalign;
>>> +  /* When this pointer is knonw to be nnonnull this would be true
>>> otherwise
>>> +     false.  */
>>> +  bool  nonnull_p;
>>>  };
>>
>>
>> Why do you need this?  Doesn't the pt.null bit represent that already?
>
>
> It looks like I can use this. As in gcc/tree-ssa-alias.h:
>
>   /* Nonzero if the points-to set includes 'nothing', the points-to set
>      includes memory at address NULL.  */
>   unsigned int null : 1;
>
> But in gcc/tree-ssa-alias.c, ptrs_compare_unequal has the following comment
> which says:
>
> /* ???  We'd like to handle ptr1 != NULL and ptr1 != ptr2
>      but those require pt.null to be conservatively correct.  */
>
> Does that means it can be wrong at times? I haven't looked it in detail yet
> but if it is, it would be a problem.

Yes, this bit is not correctly computed for all cases (well, it works
for trivial
ones but for example misses weaks and maybe some other corner-cases).

Currently there are no consumers of this bit so the incorrectness
doesn't matter.

I suggest you simply use that bit but set it conservatively from PTA (to true)
for now and adjust it from VRP only.

I do have some patches for fixinig some of the .nonnull_p issues in PTA but
they come at a cost of more constraints.

Richard.

>> Also, formatting and spelling:
>> s/knonw/known/
>> s/nnon/non/
>> s/bool  /bool /
>
>
> I will change this.
>
> Thanks,
> Kugan
>>
>>
>>         Jakub
>>
>

Re: Set nonnull attribute to ptr_info_def based on VRP

[kugan]

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

Hi Richard,

Thanks for the review.

On 09/08/16 18:58, Richard Biener wrote:
> On Tue, Aug 9, 2016 at 12:58 AM, kugan
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi Jakub,
>>
>> Thanks for the review.
>>
>> On 08/08/16 16:40, Jakub Jelinek wrote:
>>>
>>> On Mon, Aug 08, 2016 at 01:36:51PM +1000, kugan wrote:
>>>>
>>>> diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
>>>> index c81b1a1..6e34433 100644
>>>> --- a/gcc/tree-ssanames.h
>>>> +++ b/gcc/tree-ssanames.h
>>>> @@ -43,6 +43,9 @@ struct GTY(()) ptr_info_def
>>>>       above alignment.  Access only through the same helper functions as
>>>> align
>>>>       above.  */
>>>>    unsigned int misalign;
>>>> +  /* When this pointer is knonw to be nnonnull this would be true
>>>> otherwise
>>>> +     false.  */
>>>> +  bool  nonnull_p;
>>>>  };
>>>
>>>
>>> Why do you need this?  Doesn't the pt.null bit represent that already?
>>
>>
>> It looks like I can use this. As in gcc/tree-ssa-alias.h:
>>
>>   /* Nonzero if the points-to set includes 'nothing', the points-to set
>>      includes memory at address NULL.  */
>>   unsigned int null : 1;
>>
>> But in gcc/tree-ssa-alias.c, ptrs_compare_unequal has the following comment
>> which says:
>>
>> /* ???  We'd like to handle ptr1 != NULL and ptr1 != ptr2
>>      but those require pt.null to be conservatively correct.  */
>>
>> Does that means it can be wrong at times? I haven't looked it in detail yet
>> but if it is, it would be a problem.
>
> Yes, this bit is not correctly computed for all cases (well, it works
> for trivial
> ones but for example misses weaks and maybe some other corner-cases).
>
> Currently there are no consumers of this bit so the incorrectness
> doesn't matter.
>
> I suggest you simply use that bit but set it conservatively from PTA (to true)
> for now and adjust it from VRP only.
>
> I do have some patches for fixinig some of the .nonnull_p issues in PTA but
> they come at a cost of more constraints.

Here is a version of the patch that does this. I have a follow up patch 
to use this in ipa-vrp. I will send it for review once this is OK.

Bootstrapped and regression tested on x86_64-linux-gnu with no new 
regressions. Is this OK for trunk?


Thanks,
Kugan

gcc/ChangeLog:

2016-10-07  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* tree-ssa-alias.c (dump_points_to_solution): Dump non null as it
	  is set to null conservatively.
	* tree-ssa-structalias.c (find_what_var_points_to): Set to null
	  conservatively.
	(pt_solution_singleton_p): Dont check null.
	* tree-ssanames.c (set_ptr_nonnull): New.
	(get_ptr_nonnull): Likewise.
	* tree-vrp.c (vrp_finalize): Set ptr that are nonnull.
	(evrp_dom_walker::before_dom_children): Likewise.
>
> Richard.
>
>>> Also, formatting and spelling:
>>> s/knonw/known/
>>> s/nnon/non/
>>> s/bool  /bool /
>>
>>
>> I will change this.
>>
>> Thanks,
>> Kugan
>>>
>>>
>>>         Jakub
>>>
>>

[-- Attachment #2: 0003-Add-nonnull-to-pointer-from-VRP.patch --]
[-- Type: text/x-patch, Size: 6016 bytes --]

From 20ef028b2e38e3d91874ba873be10e5dfdac4d05 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Fri, 30 Sep 2016 09:00:08 +1000
Subject: [PATCH 3/5] Add nonnull to pointer from VRP

---
 gcc/tree-ssa-alias.c       |  4 ++--
 gcc/tree-ssa-structalias.c |  6 ++++--
 gcc/tree-ssanames.c        | 21 +++++++++++++++++++++
 gcc/tree-ssanames.h        |  2 ++
 gcc/tree-vrp.c             | 44 +++++++++++++++++++++++++++++---------------
 5 files changed, 58 insertions(+), 19 deletions(-)

diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 30de461..ccad68d 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -515,8 +515,8 @@ dump_points_to_solution (FILE *file, struct pt_solution *pt)
   if (pt->ipa_escaped)
     fprintf (file, ", points-to unit escaped");
 
-  if (pt->null)
-    fprintf (file, ", points-to NULL");
+  if (!pt->null)
+    fprintf (file, ", points-to non NULL");
 
   if (pt->vars)
     {
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index be892fd..99b373f8 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -6349,6 +6349,8 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)
 
   *slot = pt = XOBNEW (&final_solutions_obstack, struct pt_solution);
   memset (pt, 0, sizeof (struct pt_solution));
+  /* Conservatively set to NULL from PTA (to true). */
+  pt->null = 1;
 
   /* Translate artificial variables into SSA_NAME_PTR_INFO
      attributes.  */
@@ -6359,7 +6361,7 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)
       if (vi->is_artificial_var)
 	{
 	  if (vi->id == nothing_id)
-	    pt->null = 1;
+	    ;
 	  else if (vi->id == escaped_id)
 	    {
 	      if (in_ipa_mode)
@@ -6569,7 +6571,7 @@ bool
 pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid)
 {
   if (pt->anything || pt->nonlocal || pt->escaped || pt->ipa_escaped
-      || pt->null || pt->vars == NULL
+      || pt->vars == NULL
       || !bitmap_single_bit_set_p (pt->vars))
     return false;
 
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 91a8f97..6b6619f 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -374,6 +374,27 @@ get_range_info (const_tree name, wide_int *min, wide_int *max)
   return SSA_NAME_RANGE_TYPE (name);
 }
 
+/* Set nonnull attribute to pointer NAME.  */
+
+void
+set_ptr_nonnull (tree name)
+{
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+  struct ptr_info_def *pi = get_ptr_info (name);
+  pi->pt.null = 0;
+}
+
+/* Return nonnull attribute of pointer NAME.  */
+bool
+get_ptr_nonnull (const_tree name)
+{
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+  struct ptr_info_def *pi = SSA_NAME_PTR_INFO (name);
+  if (pi == NULL)
+    return false;
+  return !pi->pt.null;
+}
+
 /* Change non-zero bits bitmask of NAME.  */
 
 void
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index 4496e1d..d39cc9d 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -88,6 +88,8 @@ extern void set_ptr_info_alignment (struct ptr_info_def *, unsigned int,
 extern void adjust_ptr_info_misalignment (struct ptr_info_def *,
 					  unsigned int);
 extern struct ptr_info_def *get_ptr_info (tree);
+extern void set_ptr_nonnull (tree);
+extern bool get_ptr_nonnull (const_tree);
 
 extern tree copy_ssa_name_fn (struct function *, tree, gimple *);
 extern void duplicate_ssa_name_ptr_info (tree, struct ptr_info_def *);
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 0892709..80b7302 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -10593,18 +10593,24 @@ vrp_finalize (bool warn_array_bounds_p)
       {
 	tree name = ssa_name (i);
 
-      if (!name
-	  || POINTER_TYPE_P (TREE_TYPE (name))
-	  || (vr_value[i]->type == VR_VARYING)
-	  || (vr_value[i]->type == VR_UNDEFINED))
-	continue;
+	if (!name
+	    || (vr_value[i]->type == VR_VARYING)
+	    || (vr_value[i]->type == VR_UNDEFINED)
+	    || (TREE_CODE (vr_value[i]->min) != INTEGER_CST)
+	    || (TREE_CODE (vr_value[i]->max) != INTEGER_CST))
+	  continue;
 
-      if ((TREE_CODE (vr_value[i]->min) == INTEGER_CST)
-	  && (TREE_CODE (vr_value[i]->max) == INTEGER_CST)
-	  && (vr_value[i]->type == VR_RANGE
-	      || vr_value[i]->type == VR_ANTI_RANGE))
-	set_range_info (name, vr_value[i]->type, vr_value[i]->min,
-			vr_value[i]->max);
+	if (POINTER_TYPE_P (TREE_TYPE (name))
+	    && ((vr_value[i]->type == VR_RANGE
+		 && range_includes_zero_p (vr_value[i]->min,
+					   vr_value[i]->max) == 0)
+		|| (vr_value[i]->type == VR_ANTI_RANGE
+		    && range_includes_zero_p (vr_value[i]->min,
+					      vr_value[i]->max) == 1)))
+	  set_ptr_nonnull (name);
+	else if (!POINTER_TYPE_P (TREE_TYPE (name)))
+	  set_range_info (name, vr_value[i]->type, vr_value[i]->min,
+			  vr_value[i]->max);
       }
 
   substitute_and_fold (op_with_constant_singleton_value_range,
@@ -10808,17 +10814,25 @@ evrp_dom_walker::before_dom_children (basic_block bb)
 	  def_operand_p def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
 	  /* Set the SSA with the value range.  */
 	  if (def_p
-	      && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME
-	      && INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p))))
+	      && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME)
 	    {
 	      tree def = DEF_FROM_PTR (def_p);
 	      value_range *vr = get_value_range (def);
 
-	      if ((vr->type == VR_RANGE
-		   || vr->type == VR_ANTI_RANGE)
+	      if (INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p)))
+		  && (vr->type == VR_RANGE
+		      || vr->type == VR_ANTI_RANGE)
 		  && (TREE_CODE (vr->min) == INTEGER_CST)
 		  && (TREE_CODE (vr->max) == INTEGER_CST))
 		set_range_info (def, vr->type, vr->min, vr->max);
+	      else if (POINTER_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p)))
+		       && ((vr->type == VR_RANGE
+			    && range_includes_zero_p (vr->min,
+						      vr->max) == 0)
+			   || (vr->type == VR_ANTI_RANGE
+			       && range_includes_zero_p (vr->min,
+							 vr->max) == 1)))
+		set_ptr_nonnull (def);
 	    }
 	}
       else
-- 
2.7.4

Re: Set nonnull attribute to ptr_info_def based on VRP

[Richard Biener]

On Fri, Oct 7, 2016 at 2:53 AM, kugan <kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
> Thanks for the review.
>
>
> On 09/08/16 18:58, Richard Biener wrote:
>>
>> On Tue, Aug 9, 2016 at 12:58 AM, kugan
>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>
>>> Hi Jakub,
>>>
>>> Thanks for the review.
>>>
>>> On 08/08/16 16:40, Jakub Jelinek wrote:
>>>>
>>>>
>>>> On Mon, Aug 08, 2016 at 01:36:51PM +1000, kugan wrote:
>>>>>
>>>>>
>>>>> diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
>>>>> index c81b1a1..6e34433 100644
>>>>> --- a/gcc/tree-ssanames.h
>>>>> +++ b/gcc/tree-ssanames.h
>>>>> @@ -43,6 +43,9 @@ struct GTY(()) ptr_info_def
>>>>>       above alignment.  Access only through the same helper functions
>>>>> as
>>>>> align
>>>>>       above.  */
>>>>>    unsigned int misalign;
>>>>> +  /* When this pointer is knonw to be nnonnull this would be true
>>>>> otherwise
>>>>> +     false.  */
>>>>> +  bool  nonnull_p;
>>>>>  };
>>>>
>>>>
>>>>
>>>> Why do you need this?  Doesn't the pt.null bit represent that already?
>>>
>>>
>>>
>>> It looks like I can use this. As in gcc/tree-ssa-alias.h:
>>>
>>>   /* Nonzero if the points-to set includes 'nothing', the points-to set
>>>      includes memory at address NULL.  */
>>>   unsigned int null : 1;
>>>
>>> But in gcc/tree-ssa-alias.c, ptrs_compare_unequal has the following
>>> comment
>>> which says:
>>>
>>> /* ???  We'd like to handle ptr1 != NULL and ptr1 != ptr2
>>>      but those require pt.null to be conservatively correct.  */
>>>
>>> Does that means it can be wrong at times? I haven't looked it in detail
>>> yet
>>> but if it is, it would be a problem.
>>
>>
>> Yes, this bit is not correctly computed for all cases (well, it works
>> for trivial
>> ones but for example misses weaks and maybe some other corner-cases).
>>
>> Currently there are no consumers of this bit so the incorrectness
>> doesn't matter.
>>
>> I suggest you simply use that bit but set it conservatively from PTA (to
>> true)
>> for now and adjust it from VRP only.
>>
>> I do have some patches for fixinig some of the .nonnull_p issues in PTA
>> but
>> they come at a cost of more constraints.
>
>
> Here is a version of the patch that does this. I have a follow up patch to
> use this in ipa-vrp. I will send it for review once this is OK.
>
> Bootstrapped and regression tested on x86_64-linux-gnu with no new
> regressions. Is this OK for trunk?

@@ -6359,7 +6361,7 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)
       if (vi->is_artificial_var)
        {
          if (vi->id == nothing_id)
-           pt->null = 1;
+           ;

please leave this here.

 pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid)
 {
   if (pt->anything || pt->nonlocal || pt->escaped || pt->ipa_escaped
-      || pt->null || pt->vars == NULL
+      || pt->vars == NULL
       || !bitmap_single_bit_set_p (pt->vars))
     return false;

humm... this is a correctness problem I guess.  A latent one currently
depending on who relies on it.
There is a single use of the above predicate which looks for the
points-to solution of a __builtin_alloca call.  We should somehow
arrange for its solution to not include ->null.  Or, simpler, change
the predicates name to pt_solution_singleton_or_null_p () as
the use does not care for pt->null.

+void
+set_ptr_nonnull (tree name)
+{
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+  struct ptr_info_def *pi = get_ptr_info (name);
+  pi->pt.null = 0;

 = false;

+}

There is the question on how pt.null semantics should be
with respect to pt.anything, pt.escaped and pt.nonlocal.
I think get_ptr_nonnull needs to return false if any
of those are set.

Richard.

>
> Thanks,
> Kugan
>
> gcc/ChangeLog:
>
> 2016-10-07  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>         * tree-ssa-alias.c (dump_points_to_solution): Dump non null as it
>           is set to null conservatively.
>         * tree-ssa-structalias.c (find_what_var_points_to): Set to null
>           conservatively.
>         (pt_solution_singleton_p): Dont check null.
>         * tree-ssanames.c (set_ptr_nonnull): New.
>         (get_ptr_nonnull): Likewise.
>         * tree-vrp.c (vrp_finalize): Set ptr that are nonnull.
>         (evrp_dom_walker::before_dom_children): Likewise.
>
>>
>> Richard.
>>
>>>> Also, formatting and spelling:
>>>> s/knonw/known/
>>>> s/nnon/non/
>>>> s/bool  /bool /
>>>
>>>
>>>
>>> I will change this.
>>>
>>> Thanks,
>>> Kugan
>>>>
>>>>
>>>>
>>>>         Jakub
>>>>
>>>
>

Re: Set nonnull attribute to ptr_info_def based on VRP

[kugan]

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

Hi Richard,


On 07/10/16 21:03, Richard Biener wrote:
> On Fri, Oct 7, 2016 at 2:53 AM, kugan <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi Richard,
>>
>> Thanks for the review.
>>
>>
>> On 09/08/16 18:58, Richard Biener wrote:
>>>
>>> On Tue, Aug 9, 2016 at 12:58 AM, kugan
>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>
>>>> Hi Jakub,
>>>>
>>>> Thanks for the review.
>>>>
>>>> On 08/08/16 16:40, Jakub Jelinek wrote:
>>>>>
>>>>>
>>>>> On Mon, Aug 08, 2016 at 01:36:51PM +1000, kugan wrote:
>>>>>>
>>>>>>
>>>>>> diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
>>>>>> index c81b1a1..6e34433 100644
>>>>>> --- a/gcc/tree-ssanames.h
>>>>>> +++ b/gcc/tree-ssanames.h
>>>>>> @@ -43,6 +43,9 @@ struct GTY(()) ptr_info_def
>>>>>>       above alignment.  Access only through the same helper functions
>>>>>> as
>>>>>> align
>>>>>>       above.  */
>>>>>>    unsigned int misalign;
>>>>>> +  /* When this pointer is knonw to be nnonnull this would be true
>>>>>> otherwise
>>>>>> +     false.  */
>>>>>> +  bool  nonnull_p;
>>>>>>  };
>>>>>
>>>>>
>>>>>
>>>>> Why do you need this?  Doesn't the pt.null bit represent that already?
>>>>
>>>>
>>>>
>>>> It looks like I can use this. As in gcc/tree-ssa-alias.h:
>>>>
>>>>   /* Nonzero if the points-to set includes 'nothing', the points-to set
>>>>      includes memory at address NULL.  */
>>>>   unsigned int null : 1;
>>>>
>>>> But in gcc/tree-ssa-alias.c, ptrs_compare_unequal has the following
>>>> comment
>>>> which says:
>>>>
>>>> /* ???  We'd like to handle ptr1 != NULL and ptr1 != ptr2
>>>>      but those require pt.null to be conservatively correct.  */
>>>>
>>>> Does that means it can be wrong at times? I haven't looked it in detail
>>>> yet
>>>> but if it is, it would be a problem.
>>>
>>>
>>> Yes, this bit is not correctly computed for all cases (well, it works
>>> for trivial
>>> ones but for example misses weaks and maybe some other corner-cases).
>>>
>>> Currently there are no consumers of this bit so the incorrectness
>>> doesn't matter.
>>>
>>> I suggest you simply use that bit but set it conservatively from PTA (to
>>> true)
>>> for now and adjust it from VRP only.
>>>
>>> I do have some patches for fixinig some of the .nonnull_p issues in PTA
>>> but
>>> they come at a cost of more constraints.
>>
>>
>> Here is a version of the patch that does this. I have a follow up patch to
>> use this in ipa-vrp. I will send it for review once this is OK.
>>
>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>> regressions. Is this OK for trunk?
>
> @@ -6359,7 +6361,7 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)
>        if (vi->is_artificial_var)
>         {
>           if (vi->id == nothing_id)
> -           pt->null = 1;
> +           ;
>
> please leave this here.
Done.

>
>  pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid)
>  {
>    if (pt->anything || pt->nonlocal || pt->escaped || pt->ipa_escaped
> -      || pt->null || pt->vars == NULL
> +      || pt->vars == NULL
>        || !bitmap_single_bit_set_p (pt->vars))
>      return false;
>
> humm... this is a correctness problem I guess.  A latent one currently
> depending on who relies on it.
> There is a single use of the above predicate which looks for the
> points-to solution of a __builtin_alloca call.  We should somehow
> arrange for its solution to not include ->null.  Or, simpler, change
> the predicates name to pt_solution_singleton_or_null_p () as
> the use does not care for pt->null.

I have changed the name.

>
> +void
> +set_ptr_nonnull (tree name)
> +{
> +  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
> +  struct ptr_info_def *pi = get_ptr_info (name);
> +  pi->pt.null = 0;
>
>  = false;
>
> +}
>
> There is the question on how pt.null semantics should be
> with respect to pt.anything, pt.escaped and pt.nonlocal.
> I think get_ptr_nonnull needs to return false if any
> of those are set.

I have added pt.anything.

I am assuming if pt.escaped, it cannot become null?
My intention is to eliminate for pt.nonlocal too. Like in:

static __attribute__((noinline, noclone))
int foo (int *p)
{
   if (!p)
     return 0;
   *p = 1;
}

struct st
{
   int a;
   int b;
};

int bar (struct st *s)
{

   if (!s)
     return 0;
   foo (&s->a);
   foo (&s->b);
}

And also in find_what_p_points_to we can overwrite pt.null. So I have 
changed that too.

Please find the updated patch attached. Does this look better?

Thanks,
Kugan

> Richard.
>
>>
>> Thanks,
>> Kugan
>>
>> gcc/ChangeLog:
>>
>> 2016-10-07  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>>         * tree-ssa-alias.c (dump_points_to_solution): Dump non null as it
>>           is set to null conservatively.
>>         * tree-ssa-structalias.c (find_what_var_points_to): Set to null
>>           conservatively.
>>         (pt_solution_singleton_p): Dont check null.
>>         * tree-ssanames.c (set_ptr_nonnull): New.
>>         (get_ptr_nonnull): Likewise.
>>         * tree-vrp.c (vrp_finalize): Set ptr that are nonnull.
>>         (evrp_dom_walker::before_dom_children): Likewise.
>>
>>>
>>> Richard.
>>>
>>>>> Also, formatting and spelling:
>>>>> s/knonw/known/
>>>>> s/nnon/non/
>>>>> s/bool  /bool /
>>>>
>>>>
>>>>
>>>> I will change this.
>>>>
>>>> Thanks,
>>>> Kugan
>>>>>
>>>>>
>>>>>
>>>>>         Jakub
>>>>>
>>>>
>>

[-- Attachment #2: 0001-Add-nonnull-to-pointer-from-VRP.patch --]
[-- Type: text/x-patch, Size: 7924 bytes --]

From a2c163ca9e7d3569cc2d58bc188fcd466dfe2ff7 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Wed, 12 Oct 2016 13:48:07 +1100
Subject: [PATCH 1/3] Add-nonnull-to-pointer-from-VRP

---
 gcc/tree-ssa-alias.c       |  4 ++--
 gcc/tree-ssa-alias.h       |  2 +-
 gcc/tree-ssa-ccp.c         |  2 +-
 gcc/tree-ssa-structalias.c | 13 ++++++++-----
 gcc/tree-ssanames.c        | 22 ++++++++++++++++++++++
 gcc/tree-ssanames.h        |  2 ++
 gcc/tree-vrp.c             | 44 +++++++++++++++++++++++++++++---------------
 7 files changed, 65 insertions(+), 24 deletions(-)

diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 01bef17..dff11b1 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -515,8 +515,8 @@ dump_points_to_solution (FILE *file, struct pt_solution *pt)
   if (pt->ipa_escaped)
     fprintf (file, ", points-to unit escaped");
 
-  if (pt->null)
-    fprintf (file, ", points-to NULL");
+  if (!pt->null)
+    fprintf (file, ", points-to non NULL");
 
   if (pt->vars)
     {
diff --git a/gcc/tree-ssa-alias.h b/gcc/tree-ssa-alias.h
index 6680cc0..27a06fc 100644
--- a/gcc/tree-ssa-alias.h
+++ b/gcc/tree-ssa-alias.h
@@ -146,7 +146,7 @@ extern void dump_alias_stats (FILE *);
 /* In tree-ssa-structalias.c  */
 extern unsigned int compute_may_aliases (void);
 extern bool pt_solution_empty_p (struct pt_solution *);
-extern bool pt_solution_singleton_p (struct pt_solution *, unsigned *);
+extern bool pt_solution_singleton_or_null_p (struct pt_solution *, unsigned *);
 extern bool pt_solution_includes_global (struct pt_solution *);
 extern bool pt_solution_includes (struct pt_solution *, const_tree);
 extern bool pt_solutions_intersect (struct pt_solution *, struct pt_solution *);
diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index fe9a313..61754d8 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -2135,7 +2135,7 @@ fold_builtin_alloca_with_align (gimple *stmt)
       {
 	bool singleton_p;
 	unsigned uid;
-	singleton_p = pt_solution_singleton_p (&pi->pt, &uid);
+	singleton_p = pt_solution_singleton_or_null_p (&pi->pt, &uid);
 	gcc_assert (singleton_p);
 	SET_DECL_PT_UID (var, uid);
       }
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 2a4ab2f..d451ba2 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -6382,6 +6382,8 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)
 
   *slot = pt = XOBNEW (&final_solutions_obstack, struct pt_solution);
   memset (pt, 0, sizeof (struct pt_solution));
+  /* Conservatively set to NULL from PTA (to true). */
+  pt->null = 1;
 
   /* Translate artificial variables into SSA_NAME_PTR_INFO
      attributes.  */
@@ -6391,9 +6393,7 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)
 
       if (vi->is_artificial_var)
 	{
-	  if (vi->id == nothing_id)
-	    pt->null = 1;
-	  else if (vi->id == escaped_id)
+	  if (vi->id == escaped_id)
 	    {
 	      if (in_ipa_mode)
 		pt->ipa_escaped = 1;
@@ -6451,6 +6451,7 @@ find_what_p_points_to (tree fndecl, tree p)
   struct ptr_info_def *pi;
   tree lookup_p = p;
   varinfo_t vi;
+  bool nonnull = get_ptr_nonnull (p);
 
   /* For parameters, get at the points-to set for the actual parm
      decl.  */
@@ -6466,6 +6467,8 @@ find_what_p_points_to (tree fndecl, tree p)
 
   pi = get_ptr_info (p);
   pi->pt = find_what_var_points_to (fndecl, vi);
+  if (nonnull)
+    set_ptr_nonnull (p);
 }
 
 
@@ -6599,10 +6602,10 @@ pt_solution_empty_p (struct pt_solution *pt)
    return the var uid in *UID.  */
 
 bool
-pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid)
+pt_solution_singleton_or_null_p (struct pt_solution *pt, unsigned *uid)
 {
   if (pt->anything || pt->nonlocal || pt->escaped || pt->ipa_escaped
-      || pt->null || pt->vars == NULL
+      || pt->vars == NULL
       || !bitmap_single_bit_set_p (pt->vars))
     return false;
 
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 64ab13a..1d2ec01 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -374,6 +374,28 @@ get_range_info (const_tree name, wide_int *min, wide_int *max)
   return SSA_NAME_RANGE_TYPE (name);
 }
 
+/* Set nonnull attribute to pointer NAME.  */
+
+void
+set_ptr_nonnull (tree name)
+{
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+  struct ptr_info_def *pi = get_ptr_info (name);
+  pi->pt.null = 0;
+}
+
+/* Return nonnull attribute of pointer NAME.  */
+bool
+get_ptr_nonnull (const_tree name)
+{
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+  struct ptr_info_def *pi = SSA_NAME_PTR_INFO (name);
+  if (pi == NULL
+      || pi->pt.anything)
+    return false;
+  return !pi->pt.null;
+}
+
 /* Change non-zero bits bitmask of NAME.  */
 
 void
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index 4496e1d..d39cc9d 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -88,6 +88,8 @@ extern void set_ptr_info_alignment (struct ptr_info_def *, unsigned int,
 extern void adjust_ptr_info_misalignment (struct ptr_info_def *,
 					  unsigned int);
 extern struct ptr_info_def *get_ptr_info (tree);
+extern void set_ptr_nonnull (tree);
+extern bool get_ptr_nonnull (const_tree);
 
 extern tree copy_ssa_name_fn (struct function *, tree, gimple *);
 extern void duplicate_ssa_name_ptr_info (tree, struct ptr_info_def *);
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 8a129c6..7e4f947 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -10603,18 +10603,24 @@ vrp_finalize (bool warn_array_bounds_p)
       {
 	tree name = ssa_name (i);
 
-      if (!name
-	  || POINTER_TYPE_P (TREE_TYPE (name))
-	  || (vr_value[i]->type == VR_VARYING)
-	  || (vr_value[i]->type == VR_UNDEFINED))
-	continue;
+	if (!name
+	    || (vr_value[i]->type == VR_VARYING)
+	    || (vr_value[i]->type == VR_UNDEFINED)
+	    || (TREE_CODE (vr_value[i]->min) != INTEGER_CST)
+	    || (TREE_CODE (vr_value[i]->max) != INTEGER_CST))
+	  continue;
 
-      if ((TREE_CODE (vr_value[i]->min) == INTEGER_CST)
-	  && (TREE_CODE (vr_value[i]->max) == INTEGER_CST)
-	  && (vr_value[i]->type == VR_RANGE
-	      || vr_value[i]->type == VR_ANTI_RANGE))
-	set_range_info (name, vr_value[i]->type, vr_value[i]->min,
-			vr_value[i]->max);
+	if (POINTER_TYPE_P (TREE_TYPE (name))
+	    && ((vr_value[i]->type == VR_RANGE
+		 && range_includes_zero_p (vr_value[i]->min,
+					   vr_value[i]->max) == 0)
+		|| (vr_value[i]->type == VR_ANTI_RANGE
+		    && range_includes_zero_p (vr_value[i]->min,
+					      vr_value[i]->max) == 1)))
+	  set_ptr_nonnull (name);
+	else if (!POINTER_TYPE_P (TREE_TYPE (name)))
+	  set_range_info (name, vr_value[i]->type, vr_value[i]->min,
+			  vr_value[i]->max);
       }
 
   substitute_and_fold (op_with_constant_singleton_value_range,
@@ -10817,17 +10823,25 @@ evrp_dom_walker::before_dom_children (basic_block bb)
 	  def_operand_p def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
 	  /* Set the SSA with the value range.  */
 	  if (def_p
-	      && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME
-	      && INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p))))
+	      && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME)
 	    {
 	      tree def = DEF_FROM_PTR (def_p);
 	      value_range *vr = get_value_range (def);
 
-	      if ((vr->type == VR_RANGE
-		   || vr->type == VR_ANTI_RANGE)
+	      if (INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p)))
+		  && (vr->type == VR_RANGE
+		      || vr->type == VR_ANTI_RANGE)
 		  && (TREE_CODE (vr->min) == INTEGER_CST)
 		  && (TREE_CODE (vr->max) == INTEGER_CST))
 		set_range_info (def, vr->type, vr->min, vr->max);
+	      else if (POINTER_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p)))
+		       && ((vr->type == VR_RANGE
+			    && range_includes_zero_p (vr->min,
+						      vr->max) == 0)
+			   || (vr->type == VR_ANTI_RANGE
+			       && range_includes_zero_p (vr->min,
+							 vr->max) == 1)))
+		set_ptr_nonnull (def);
 	    }
 	}
       else
-- 
2.7.4

[ipa-vrp] Use get/set_ptr_nonnull in ipa-vrp

[kugan]

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

Hi,

This patch uses the get/set_ptr_nonnull so that ipa-vrp also propagates 
nonnull ranges for pinter.

Bootstrapped and regression tested this with other patched without any 
new regressions on x86_64-linux-gnu.

Is this OK for trunk?

Thanks,
Kugan




gcc/ChangeLog:

2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>

	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Set value range
	  for pointer type too.
	(ipcp_update_vr): set_ptr_nonnull for pointer.

gcc/testsuite/ChangeLog:

2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>

	* gcc.dg/ipa/vrp4.c: New test.


[-- Attachment #2: 0002-Set-nonnull-range-for-pointer-type.patch --]
[-- Type: text/x-patch, Size: 3596 bytes --]

From f773226855968cc652fa6f2b2d9c70d2a5d7acdb Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Wed, 12 Oct 2016 13:54:34 +1100
Subject: [PATCH 2/3] Set-nonnull-range-for-pointer-type

---
 gcc/ipa-prop.c                  | 57 +++++++++++++++++++++++++++++------------
 gcc/testsuite/gcc.dg/ipa/vrp4.c | 27 +++++++++++++++++++
 2 files changed, 68 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/vrp4.c

diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index a1d7619..353b638 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -1668,7 +1668,22 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
 	    useful_context = true;
 	}
 
-      if (!POINTER_TYPE_P (TREE_TYPE (arg)))
+      if (POINTER_TYPE_P (TREE_TYPE (arg)))
+	{
+	  if (TREE_CODE (arg) == SSA_NAME
+	      && param_type
+	      && get_ptr_nonnull (arg))
+	    {
+	      jfunc->vr_known = true;
+	      jfunc->m_vr.type = VR_ANTI_RANGE;
+	      jfunc->m_vr.min = build_int_cst (TREE_TYPE (arg), 0);
+	      jfunc->m_vr.max = build_int_cst (TREE_TYPE (arg), 0);
+	      jfunc->m_vr.equiv = NULL;
+	    }
+	  else
+	    gcc_assert (!jfunc->vr_known);
+	}
+      else
 	{
 	  wide_int min, max;
 	  value_range_type type;
@@ -5602,27 +5617,37 @@ ipcp_update_vr (struct cgraph_node *node)
 	continue;
 
       if (vr[i].known
-	  && INTEGRAL_TYPE_P (TREE_TYPE (ddef))
-	  && !POINTER_TYPE_P (TREE_TYPE (ddef))
 	  && (vr[i].type == VR_RANGE || vr[i].type == VR_ANTI_RANGE))
 	{
 	  tree type = TREE_TYPE (ddef);
 	  unsigned prec = TYPE_PRECISION (type);
-	  if (dump_file)
+	  if (INTEGRAL_TYPE_P (TREE_TYPE (ddef)))
+	    {
+	      if (dump_file)
+		{
+		  fprintf (dump_file, "Setting value range of param %u ", i);
+		  fprintf (dump_file, "%s[",
+			   (vr[i].type == VR_ANTI_RANGE) ? "~" : "");
+		  print_decs (vr[i].min, dump_file);
+		  fprintf (dump_file, ", ");
+		  print_decs (vr[i].max, dump_file);
+		  fprintf (dump_file, "]\n");
+		}
+	      set_range_info (ddef, vr[i].type,
+			      wide_int_storage::from (vr[i].min, prec,
+						      TYPE_SIGN (type)),
+			      wide_int_storage::from (vr[i].max, prec,
+						      TYPE_SIGN (type)));
+	    }
+	  else if (POINTER_TYPE_P (TREE_TYPE (ddef))
+		   && vr[i].type == VR_ANTI_RANGE
+		   && wi::eq_p (vr[i].min, 0)
+		   && wi::eq_p (vr[i].max, 0))
 	    {
-	      fprintf (dump_file, "Setting value range of param %u ", i);
-	      fprintf (dump_file, "%s[",
-		       (vr[i].type == VR_ANTI_RANGE) ? "~" : "");
-	      print_decs (vr[i].min, dump_file);
-	      fprintf (dump_file, ", ");
-	      print_decs (vr[i].max, dump_file);
-	      fprintf (dump_file, "]\n");
+	      if (dump_file)
+		fprintf (dump_file, "Setting nonnull for %u\n", i);
+	      set_ptr_nonnull (ddef);
 	    }
-	  set_range_info (ddef, vr[i].type,
-			  wide_int_storage::from (vr[i].min, prec,
-						  TYPE_SIGN (type)),
-			  wide_int_storage::from (vr[i].max, prec,
-						  TYPE_SIGN (type)));
 	}
     }
 }
diff --git a/gcc/testsuite/gcc.dg/ipa/vrp4.c b/gcc/testsuite/gcc.dg/ipa/vrp4.c
new file mode 100644
index 0000000..d7e1f26
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/vrp4.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-cp-details" } */
+
+static __attribute__((noinline, noclone))
+int foo (int *p)
+{
+  if (!p)
+    return 0;
+  *p = 1;
+}
+
+struct st
+{
+  int a;
+  int b;
+};
+
+int bar (struct st *s)
+{
+
+  if (!s)
+    return 0;
+  foo (&s->a);
+  foo (&s->b);
+}
+
+/* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */
-- 
2.7.4

[vrp] use get_ptr_nonnull in tree-vrp

[kugan]

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

Hi,

This patch uses get_ptr_nonnull in tree-vrp.

Bootstrapped and regression tested this with other patched without any
new regressions on x86_64-linux-gnu.

Is this OK for trunk?

Thanks,
Kugan

gcc/testsuite/ChangeLog:

2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>

	* gcc.dg/ipa/vrp4.c: Adjust testcase.

gcc/ChangeLog:

2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>

	* tree-vrp.c (get_value_range): Check get_ptr_nonnull.

[-- Attachment #2: 0003-Teach-vrp-to-use-ptr-nonnull.patch --]
[-- Type: text/x-patch, Size: 1484 bytes --]

From 56e57ed72e44f22285d1f9de204ff8f98f6c08d2 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Wed, 12 Oct 2016 13:54:58 +1100
Subject: [PATCH 3/3] Teach-vrp-to-use-ptr-nonnull

---
 gcc/testsuite/gcc.dg/ipa/vrp4.c | 3 ++-
 gcc/tree-vrp.c                  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/ipa/vrp4.c b/gcc/testsuite/gcc.dg/ipa/vrp4.c
index d7e1f26..941f80e 100644
--- a/gcc/testsuite/gcc.dg/ipa/vrp4.c
+++ b/gcc/testsuite/gcc.dg/ipa/vrp4.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-ipa-cp-details" } */
+/* { dg-options "-O2 -fdump-ipa-cp-details -fdump-tree-vrp1" } */
 
 static __attribute__((noinline, noclone))
 int foo (int *p)
@@ -25,3 +25,4 @@ int bar (struct st *s)
 }
 
 /* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */
+/* { dg-final { scan-tree-dump-times "if" 1 "vrp1" } } */
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 7e4f947..f9e5936 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -685,7 +685,8 @@ get_value_range (const_tree var)
 	     anti-ranges for pointers.  Note that this is only valid with
 	     default definitions of PARM_DECLs.  */
 	  if (POINTER_TYPE_P (TREE_TYPE (sym))
-	      && nonnull_arg_p (sym))
+	      && (nonnull_arg_p (sym)
+		  || get_ptr_nonnull (var)))
 	    set_value_range_to_nonnull (vr, TREE_TYPE (sym));
 	  else if (INTEGRAL_TYPE_P (TREE_TYPE (sym)))
 	    {
-- 
2.7.4

Re: [ipa-vrp] Use get/set_ptr_nonnull in ipa-vrp

[Jan Hubicka]

> Hi,
> 
> This patch uses the get/set_ptr_nonnull so that ipa-vrp also
> propagates nonnull ranges for pinter.
> 
> Bootstrapped and regression tested this with other patched without
> any new regressions on x86_64-linux-gnu.
> 
> Is this OK for trunk?
> 
> Thanks,
> Kugan
> 
> 
> 
> 
> gcc/ChangeLog:
> 
> 2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
> 
> 	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Set value range
> 	  for pointer type too.
> 	(ipcp_update_vr): set_ptr_nonnull for pointer.
> 
> gcc/testsuite/ChangeLog:
> 
> 2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
> 
> 	* gcc.dg/ipa/vrp4.c: New test.
OK, thank you!
We should be able to derive a lot of (useful) non-null information from the
fact that the pointers are dereferenced either prior the function call or in a
statement that postdominate the function entry.  I guess we could also give (semi)
useful -Wmissing-attribute=nonnull hints in that case.

Honza

> 

> >From f773226855968cc652fa6f2b2d9c70d2a5d7acdb Mon Sep 17 00:00:00 2001
> From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
> Date: Wed, 12 Oct 2016 13:54:34 +1100
> Subject: [PATCH 2/3] Set-nonnull-range-for-pointer-type
> 
> ---
>  gcc/ipa-prop.c                  | 57 +++++++++++++++++++++++++++++------------
>  gcc/testsuite/gcc.dg/ipa/vrp4.c | 27 +++++++++++++++++++
>  2 files changed, 68 insertions(+), 16 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/vrp4.c
> 
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index a1d7619..353b638 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -1668,7 +1668,22 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
>  	    useful_context = true;
>  	}
>  
> -      if (!POINTER_TYPE_P (TREE_TYPE (arg)))
> +      if (POINTER_TYPE_P (TREE_TYPE (arg)))
> +	{
> +	  if (TREE_CODE (arg) == SSA_NAME
> +	      && param_type
> +	      && get_ptr_nonnull (arg))
> +	    {
> +	      jfunc->vr_known = true;
> +	      jfunc->m_vr.type = VR_ANTI_RANGE;
> +	      jfunc->m_vr.min = build_int_cst (TREE_TYPE (arg), 0);
> +	      jfunc->m_vr.max = build_int_cst (TREE_TYPE (arg), 0);
> +	      jfunc->m_vr.equiv = NULL;
> +	    }
> +	  else
> +	    gcc_assert (!jfunc->vr_known);
> +	}
> +      else
>  	{
>  	  wide_int min, max;
>  	  value_range_type type;
> @@ -5602,27 +5617,37 @@ ipcp_update_vr (struct cgraph_node *node)
>  	continue;
>  
>        if (vr[i].known
> -	  && INTEGRAL_TYPE_P (TREE_TYPE (ddef))
> -	  && !POINTER_TYPE_P (TREE_TYPE (ddef))
>  	  && (vr[i].type == VR_RANGE || vr[i].type == VR_ANTI_RANGE))
>  	{
>  	  tree type = TREE_TYPE (ddef);
>  	  unsigned prec = TYPE_PRECISION (type);
> -	  if (dump_file)
> +	  if (INTEGRAL_TYPE_P (TREE_TYPE (ddef)))
> +	    {
> +	      if (dump_file)
> +		{
> +		  fprintf (dump_file, "Setting value range of param %u ", i);
> +		  fprintf (dump_file, "%s[",
> +			   (vr[i].type == VR_ANTI_RANGE) ? "~" : "");
> +		  print_decs (vr[i].min, dump_file);
> +		  fprintf (dump_file, ", ");
> +		  print_decs (vr[i].max, dump_file);
> +		  fprintf (dump_file, "]\n");
> +		}
> +	      set_range_info (ddef, vr[i].type,
> +			      wide_int_storage::from (vr[i].min, prec,
> +						      TYPE_SIGN (type)),
> +			      wide_int_storage::from (vr[i].max, prec,
> +						      TYPE_SIGN (type)));
> +	    }
> +	  else if (POINTER_TYPE_P (TREE_TYPE (ddef))
> +		   && vr[i].type == VR_ANTI_RANGE
> +		   && wi::eq_p (vr[i].min, 0)
> +		   && wi::eq_p (vr[i].max, 0))
>  	    {
> -	      fprintf (dump_file, "Setting value range of param %u ", i);
> -	      fprintf (dump_file, "%s[",
> -		       (vr[i].type == VR_ANTI_RANGE) ? "~" : "");
> -	      print_decs (vr[i].min, dump_file);
> -	      fprintf (dump_file, ", ");
> -	      print_decs (vr[i].max, dump_file);
> -	      fprintf (dump_file, "]\n");
> +	      if (dump_file)
> +		fprintf (dump_file, "Setting nonnull for %u\n", i);
> +	      set_ptr_nonnull (ddef);
>  	    }
> -	  set_range_info (ddef, vr[i].type,
> -			  wide_int_storage::from (vr[i].min, prec,
> -						  TYPE_SIGN (type)),
> -			  wide_int_storage::from (vr[i].max, prec,
> -						  TYPE_SIGN (type)));
>  	}
>      }
>  }
> diff --git a/gcc/testsuite/gcc.dg/ipa/vrp4.c b/gcc/testsuite/gcc.dg/ipa/vrp4.c
> new file mode 100644
> index 0000000..d7e1f26
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/vrp4.c
> @@ -0,0 +1,27 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-ipa-cp-details" } */
> +
> +static __attribute__((noinline, noclone))
> +int foo (int *p)
> +{
> +  if (!p)
> +    return 0;
> +  *p = 1;
> +}
> +
> +struct st
> +{
> +  int a;
> +  int b;
> +};
> +
> +int bar (struct st *s)
> +{
> +
> +  if (!s)
> +    return 0;
> +  foo (&s->a);
> +  foo (&s->b);
> +}
> +
> +/* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */
> -- 
> 2.7.4
> 

Re: Set nonnull attribute to ptr_info_def based on VRP

[Richard Biener]

On Wed, Oct 12, 2016 at 8:46 AM, kugan
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
>
>
> On 07/10/16 21:03, Richard Biener wrote:
>>
>> On Fri, Oct 7, 2016 at 2:53 AM, kugan <kugan.vivekanandarajah@linaro.org>
>> wrote:
>>>
>>> Hi Richard,
>>>
>>> Thanks for the review.
>>>
>>>
>>> On 09/08/16 18:58, Richard Biener wrote:
>>>>
>>>>
>>>> On Tue, Aug 9, 2016 at 12:58 AM, kugan
>>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>>
>>>>>
>>>>> Hi Jakub,
>>>>>
>>>>> Thanks for the review.
>>>>>
>>>>> On 08/08/16 16:40, Jakub Jelinek wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Mon, Aug 08, 2016 at 01:36:51PM +1000, kugan wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
>>>>>>> index c81b1a1..6e34433 100644
>>>>>>> --- a/gcc/tree-ssanames.h
>>>>>>> +++ b/gcc/tree-ssanames.h
>>>>>>> @@ -43,6 +43,9 @@ struct GTY(()) ptr_info_def
>>>>>>>       above alignment.  Access only through the same helper functions
>>>>>>> as
>>>>>>> align
>>>>>>>       above.  */
>>>>>>>    unsigned int misalign;
>>>>>>> +  /* When this pointer is knonw to be nnonnull this would be true
>>>>>>> otherwise
>>>>>>> +     false.  */
>>>>>>> +  bool  nonnull_p;
>>>>>>>  };
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Why do you need this?  Doesn't the pt.null bit represent that already?
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> It looks like I can use this. As in gcc/tree-ssa-alias.h:
>>>>>
>>>>>   /* Nonzero if the points-to set includes 'nothing', the points-to set
>>>>>      includes memory at address NULL.  */
>>>>>   unsigned int null : 1;
>>>>>
>>>>> But in gcc/tree-ssa-alias.c, ptrs_compare_unequal has the following
>>>>> comment
>>>>> which says:
>>>>>
>>>>> /* ???  We'd like to handle ptr1 != NULL and ptr1 != ptr2
>>>>>      but those require pt.null to be conservatively correct.  */
>>>>>
>>>>> Does that means it can be wrong at times? I haven't looked it in detail
>>>>> yet
>>>>> but if it is, it would be a problem.
>>>>
>>>>
>>>>
>>>> Yes, this bit is not correctly computed for all cases (well, it works
>>>> for trivial
>>>> ones but for example misses weaks and maybe some other corner-cases).
>>>>
>>>> Currently there are no consumers of this bit so the incorrectness
>>>> doesn't matter.
>>>>
>>>> I suggest you simply use that bit but set it conservatively from PTA (to
>>>> true)
>>>> for now and adjust it from VRP only.
>>>>
>>>> I do have some patches for fixinig some of the .nonnull_p issues in PTA
>>>> but
>>>> they come at a cost of more constraints.
>>>
>>>
>>>
>>> Here is a version of the patch that does this. I have a follow up patch
>>> to
>>> use this in ipa-vrp. I will send it for review once this is OK.
>>>
>>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>>> regressions. Is this OK for trunk?
>>
>>
>> @@ -6359,7 +6361,7 @@ find_what_var_points_to (tree fndecl, varinfo_t
>> orig_vi)
>>        if (vi->is_artificial_var)
>>         {
>>           if (vi->id == nothing_id)
>> -           pt->null = 1;
>> +           ;
>>
>> please leave this here.
>
> Done.
>
>>
>>  pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid)
>>  {
>>    if (pt->anything || pt->nonlocal || pt->escaped || pt->ipa_escaped
>> -      || pt->null || pt->vars == NULL
>> +      || pt->vars == NULL
>>        || !bitmap_single_bit_set_p (pt->vars))
>>      return false;
>>
>> humm... this is a correctness problem I guess.  A latent one currently
>> depending on who relies on it.
>> There is a single use of the above predicate which looks for the
>> points-to solution of a __builtin_alloca call.  We should somehow
>> arrange for its solution to not include ->null.  Or, simpler, change
>> the predicates name to pt_solution_singleton_or_null_p () as
>> the use does not care for pt->null.
>
>
> I have changed the name.
>
>>
>> +void
>> +set_ptr_nonnull (tree name)
>> +{
>> +  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
>> +  struct ptr_info_def *pi = get_ptr_info (name);
>> +  pi->pt.null = 0;
>>
>>  = false;
>>
>> +}
>>
>> There is the question on how pt.null semantics should be
>> with respect to pt.anything, pt.escaped and pt.nonlocal.
>> I think get_ptr_nonnull needs to return false if any
>> of those are set.
>
>
> I have added pt.anything.
>
> I am assuming if pt.escaped, it cannot become null?

Sure can it become null.

> My intention is to eliminate for pt.nonlocal too. Like in:
>
> static __attribute__((noinline, noclone))
> int foo (int *p)
> {
>   if (!p)
>     return 0;
>   *p = 1;
> }
>
> struct st
> {
>   int a;
>   int b;
> };
>
> int bar (struct st *s)
> {
>
>   if (!s)
>     return 0;
>   foo (&s->a);
>   foo (&s->b);
> }

I don't think this is sth PTA handles.

> And also in find_what_p_points_to we can overwrite pt.null. So I have
> changed that too.
>
> Please find the updated patch attached. Does this look better?

@@ -515,8 +515,8 @@ dump_points_to_solution (FILE *file, struct pt_solution *pt)
   if (pt->ipa_escaped)
     fprintf (file, ", points-to unit escaped");

-  if (pt->null)
-    fprintf (file, ", points-to NULL");
+  if (!pt->null)
+    fprintf (file, ", points-to non NULL");

please keep the original dumping.  !pt->null doesn't mean it points to
sth not NULL.
It might point to nothing.  And as I said pt->nonlocal / pt->escaped
include NULL.

@@ -6391,9 +6393,7 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)

       if (vi->is_artificial_var)
        {
-         if (vi->id == nothing_id)
-           pt->null = 1;
-         else if (vi->id == escaped_id)
+         if (vi->id == escaped_id)
            {
              if (in_ipa_mode)
                pt->ipa_escaped = 1;

please keep this as well.

@@ -6451,6 +6451,7 @@ find_what_p_points_to (tree fndecl, tree p)
   struct ptr_info_def *pi;
   tree lookup_p = p;
   varinfo_t vi;
+  bool nonnull = get_ptr_nonnull (p);

   /* For parameters, get at the points-to set for the actual parm
      decl.  */
@@ -6466,6 +6467,8 @@ find_what_p_points_to (tree fndecl, tree p)

   pi = get_ptr_info (p);
   pi->pt = find_what_var_points_to (fndecl, vi);
+  if (nonnull)
+    set_ptr_nonnull (p);
 }

what does this try to do?  Preserve info VRP computed across PTA?

I think we didn't yet sort out the nonlocal/escaped vs. null handling properly
(or how PTA should handle get_ptr_nonnull).  The way you are using it
asks for pt.null to be orthogonal to nonlocal/escaped and thus having
nonlocal or escaped would also require setting ptr.null in PTA.  It then
would be also more canonical to set it for pt.anything as well.  Which
means conservatively handling it would be equivalent to flipping its
semantic and changing its name to pt.nonnull.

That said, you seem to be simply "reserving" the bit for VRP, keeping it
conservatively true when "not computed".  I guess I'm fine with this for now
but it should be documented in the header file that way.

Richard.

> Thanks,
> Kugan
>
>
>> Richard.
>>
>>>
>>> Thanks,
>>> Kugan
>>>
>>> gcc/ChangeLog:
>>>
>>> 2016-10-07  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>
>>>         * tree-ssa-alias.c (dump_points_to_solution): Dump non null as it
>>>           is set to null conservatively.
>>>         * tree-ssa-structalias.c (find_what_var_points_to): Set to null
>>>           conservatively.
>>>         (pt_solution_singleton_p): Dont check null.
>>>         * tree-ssanames.c (set_ptr_nonnull): New.
>>>         (get_ptr_nonnull): Likewise.
>>>         * tree-vrp.c (vrp_finalize): Set ptr that are nonnull.
>>>         (evrp_dom_walker::before_dom_children): Likewise.
>>>
>>>>
>>>> Richard.
>>>>
>>>>>> Also, formatting and spelling:
>>>>>> s/knonw/known/
>>>>>> s/nnon/non/
>>>>>> s/bool  /bool /
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> I will change this.
>>>>>
>>>>> Thanks,
>>>>> Kugan
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>         Jakub
>>>>>>
>>>>>
>>>
>

Re: [vrp] use get_ptr_nonnull in tree-vrp

[Richard Biener]

On Wed, Oct 12, 2016 at 8:56 AM, kugan
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi,
>
> This patch uses get_ptr_nonnull in tree-vrp.
>
> Bootstrapped and regression tested this with other patched without any
> new regressions on x86_64-linux-gnu.
>
> Is this OK for trunk?

Um.  Doesn't make much sense given nothing provides this info before EVRP?
And if it makes sense then it makes sense not only for PARM_DECL SSA names.

Richard.

> Thanks,
> Kugan
>
> gcc/testsuite/ChangeLog:
>
> 2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
>
>         * gcc.dg/ipa/vrp4.c: Adjust testcase.
>
> gcc/ChangeLog:
>
> 2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
>
>         * tree-vrp.c (get_value_range): Check get_ptr_nonnull.

Re: [vrp] use get_ptr_nonnull in tree-vrp

[kugan]

Hi Richard,

On 12/10/16 23:24, Richard Biener wrote:
> On Wed, Oct 12, 2016 at 8:56 AM, kugan
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi,
>>
>> This patch uses get_ptr_nonnull in tree-vrp.
>>
>> Bootstrapped and regression tested this with other patched without any
>> new regressions on x86_64-linux-gnu.
>>
>> Is this OK for trunk?
>
> Um.  Doesn't make much sense given nothing provides this info before EVRP?
> And if it makes sense then it makes sense not only for PARM_DECL SSA names.

Not before EVRP. But when in TREE-VRP, EVRP + IPA-VRP should provide this.

I am not sure if this is the question?

Thanks,
Kugan
>
> Richard.
>
>> Thanks,
>> Kugan
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
>>
>>         * gcc.dg/ipa/vrp4.c: Adjust testcase.
>>
>> gcc/ChangeLog:
>>
>> 2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
>>
>>         * tree-vrp.c (get_value_range): Check get_ptr_nonnull.

Re: [vrp] use get_ptr_nonnull in tree-vrp

[kugan]

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

Hi Richard,


On 13/10/16 05:53, kugan wrote:
> Hi Richard,
>
> On 12/10/16 23:24, Richard Biener wrote:
>> On Wed, Oct 12, 2016 at 8:56 AM, kugan
>> <kugan.vivekanandarajah@linaro.org> wrote:
>>> Hi,
>>>
>>> This patch uses get_ptr_nonnull in tree-vrp.
>>>
>>> Bootstrapped and regression tested this with other patched without any
>>> new regressions on x86_64-linux-gnu.
>>>
>>> Is this OK for trunk?
>>
>> Um.  Doesn't make much sense given nothing provides this info before EVRP?
>> And if it makes sense then it makes sense not only for PARM_DECL SSA names.
> Not before EVRP. But when in TREE-VRP, EVRP + IPA-VRP should provide this.

My primary intention was to pass it for PARM_DECL SSA names which comes 
from ipa-vrp. I have changed this now.

Thanks,
Kugan

> I am not sure if this is the question?
>
> Thanks,
> Kugan
>>
>> Richard.
>>
>>> Thanks,
>>> Kugan
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
>>>
>>>         * gcc.dg/ipa/vrp4.c: Adjust testcase.
>>>
>>> gcc/ChangeLog:
>>>
>>> 2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
>>>
>>>         * tree-vrp.c (get_value_range): Check get_ptr_nonnull.

[-- Attachment #2: 0003-Teach-vrp-to-use-ptr-nonnull.patch --]
[-- Type: text/x-patch, Size: 1547 bytes --]

From c033af7161bcf54f50262688854cb32e3e8eeb54 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Wed, 12 Oct 2016 13:54:58 +1100
Subject: [PATCH 3/3] Teach-vrp-to-use-ptr-nonnull

---
 gcc/testsuite/gcc.dg/ipa/vrp4.c | 3 ++-
 gcc/tree-vrp.c                  | 5 ++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/ipa/vrp4.c b/gcc/testsuite/gcc.dg/ipa/vrp4.c
index d7e1f26..941f80e 100644
--- a/gcc/testsuite/gcc.dg/ipa/vrp4.c
+++ b/gcc/testsuite/gcc.dg/ipa/vrp4.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-ipa-cp-details" } */
+/* { dg-options "-O2 -fdump-ipa-cp-details -fdump-tree-vrp1" } */
 
 static __attribute__((noinline, noclone))
 int foo (int *p)
@@ -25,3 +25,4 @@ int bar (struct st *s)
 }
 
 /* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */
+/* { dg-final { scan-tree-dump-times "if" 1 "vrp1" } } */
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 7e4f947..bc53d07 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -679,7 +679,10 @@ get_value_range (const_tree var)
   if (SSA_NAME_IS_DEFAULT_DEF (var))
     {
       sym = SSA_NAME_VAR (var);
-      if (TREE_CODE (sym) == PARM_DECL)
+      if (POINTER_TYPE_P (TREE_TYPE (sym))
+	  && get_ptr_nonnull (var))
+	set_value_range_to_nonnull (vr, TREE_TYPE (sym));
+      else if (TREE_CODE (sym) == PARM_DECL)
 	{
 	  /* Try to use the "nonnull" attribute to create ~[0, 0]
 	     anti-ranges for pointers.  Note that this is only valid with
-- 
2.7.4

Re: Set nonnull attribute to ptr_info_def based on VRP

[kugan]

Hi Richard,

>
> what does this try to do?  Preserve info VRP computed across PTA?
>
> I think we didn't yet sort out the nonlocal/escaped vs. null handling properly
> (or how PTA should handle get_ptr_nonnull).  The way you are using it
> asks for pt.null to be orthogonal to nonlocal/escaped and thus having
> nonlocal or escaped would also require setting ptr.null in PTA.  It then
> would be also more canonical to set it for pt.anything as well.  Which
> means conservatively handling it would be equivalent to flipping its
> semantic and changing its name to pt.nonnull.
>
> That said, you seem to be simply "reserving" the bit for VRP, keeping it
> conservatively true when "not computed".  I guess I'm fine with this for now
> but it should be documented in the header file that way.
>

Thanks for the comments.

To summarize, currently I am not relying on PTA analysis at all. Just 
saving null from VRP (or rather nonnull) and preserving it across PTA. 
Primary intention is to pass it for PARM_DECL SSA names (from ipa-vrp).

In this case, using  pt.anything/nonlocal/escaped will only make the 
result more pessimistic.

Ideally, we should improve pt.null within PTA but for now as you said, I 
will document it.

When we start using pt.null from PTA analysis, we would also have to 
take into account pt.anything/nonlocal/escaped.

Does that make sense?

Thanks,
Kugan

Re: [vrp] use get_ptr_nonnull in tree-vrp

[Richard Biener]

On Thu, Oct 13, 2016 at 6:38 AM, kugan
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
>
> On 13/10/16 05:53, kugan wrote:
>>
>> Hi Richard,
>>
>> On 12/10/16 23:24, Richard Biener wrote:
>>>
>>> On Wed, Oct 12, 2016 at 8:56 AM, kugan
>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>
>>>> Hi,
>>>>
>>>> This patch uses get_ptr_nonnull in tree-vrp.
>>>>
>>>> Bootstrapped and regression tested this with other patched without any
>>>> new regressions on x86_64-linux-gnu.
>>>>
>>>> Is this OK for trunk?
>>>
>>>
>>> Um.  Doesn't make much sense given nothing provides this info before
>>> EVRP?
>>> And if it makes sense then it makes sense not only for PARM_DECL SSA
>>> names.
>>
>> Not before EVRP. But when in TREE-VRP, EVRP + IPA-VRP should provide this.
>
>
> My primary intention was to pass it for PARM_DECL SSA names which comes from
> ipa-vrp. I have changed this now.

Ok I see.  The new patch still has it inside the SSA_NAME_IS_DEFAULT_DEF () if
so isn't any better in this regard.  To handle non-default-defs you'd
have to intersect
with non-NULL in update_value_range where we also intersect with get_range_info
info for integer types.

Your original patch was better so that is ok (once the prerequesites
are approved).

We can decide later if it's worth handling get_ptr_nonnull in
update_value_range.

Thanks,
Richard.

> Thanks,
> Kugan
>
>
>> I am not sure if this is the question?
>>
>> Thanks,
>> Kugan
>>>
>>>
>>> Richard.
>>>
>>>> Thanks,
>>>> Kugan
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
>>>>
>>>>         * gcc.dg/ipa/vrp4.c: Adjust testcase.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
>>>>
>>>>         * tree-vrp.c (get_value_range): Check get_ptr_nonnull.

Re: Set nonnull attribute to ptr_info_def based on VRP

[Richard Biener]

On Thu, Oct 13, 2016 at 6:49 AM, kugan
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
>>
>> what does this try to do?  Preserve info VRP computed across PTA?
>>
>> I think we didn't yet sort out the nonlocal/escaped vs. null handling
>> properly
>> (or how PTA should handle get_ptr_nonnull).  The way you are using it
>> asks for pt.null to be orthogonal to nonlocal/escaped and thus having
>> nonlocal or escaped would also require setting ptr.null in PTA.  It then
>> would be also more canonical to set it for pt.anything as well.  Which
>> means conservatively handling it would be equivalent to flipping its
>> semantic and changing its name to pt.nonnull.
>>
>> That said, you seem to be simply "reserving" the bit for VRP, keeping it
>> conservatively true when "not computed".  I guess I'm fine with this for
>> now
>> but it should be documented in the header file that way.
>>
>
> Thanks for the comments.
>
> To summarize, currently I am not relying on PTA analysis at all. Just saving
> null from VRP (or rather nonnull) and preserving it across PTA. Primary
> intention is to pass it for PARM_DECL SSA names (from ipa-vrp).
>
> In this case, using  pt.anything/nonlocal/escaped will only make the result
> more pessimistic.
>
> Ideally, we should improve pt.null within PTA but for now as you said, I
> will document it.
>
> When we start using pt.null from PTA analysis, we would also have to take
> into account pt.anything/nonlocal/escaped.
>
> Does that make sense?

Yes.

Thanks,
Richard.

> Thanks,
> Kugan
>

Re: Set nonnull attribute to ptr_info_def based on VRP

[kugan]

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

Hi Richard,

On 13/10/16 20:44, Richard Biener wrote:
> On Thu, Oct 13, 2016 at 6:49 AM, kugan
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi Richard,
>>
>>>
>>> what does this try to do?  Preserve info VRP computed across PTA?
>>>
>>> I think we didn't yet sort out the nonlocal/escaped vs. null handling
>>> properly
>>> (or how PTA should handle get_ptr_nonnull).  The way you are using it
>>> asks for pt.null to be orthogonal to nonlocal/escaped and thus having
>>> nonlocal or escaped would also require setting ptr.null in PTA.  It then
>>> would be also more canonical to set it for pt.anything as well.  Which
>>> means conservatively handling it would be equivalent to flipping its
>>> semantic and changing its name to pt.nonnull.
>>>
>>> That said, you seem to be simply "reserving" the bit for VRP, keeping it
>>> conservatively true when "not computed".  I guess I'm fine with this for
>>> now
>>> but it should be documented in the header file that way.
>>>
>>
>> Thanks for the comments.
>>
>> To summarize, currently I am not relying on PTA analysis at all. Just saving
>> null from VRP (or rather nonnull) and preserving it across PTA. Primary
>> intention is to pass it for PARM_DECL SSA names (from ipa-vrp).
>>
>> In this case, using  pt.anything/nonlocal/escaped will only make the result
>> more pessimistic.
>>
>> Ideally, we should improve pt.null within PTA but for now as you said, I
>> will document it.
>>
>> When we start using pt.null from PTA analysis, we would also have to take
>> into account pt.anything/nonlocal/escaped.
>>
>> Does that make sense?
>
> Yes.


Here is the revised patch based on the review. I also had to adjust two 
testcases since we set pt.null conservatively and dumps that too.

Thanks,
Kugan

gcc/ChangeLog:

2016-10-14  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* tree-ssa-alias.h (pt_solution_singleton_or_null_p): Renamed from
	pt_solution_singleton_p.
	* tree-ssa-ccp.c (fold_builtin_alloca_with_align): Use renamed
	pt_solution_singleton_or_null_p from pt_solution_singleton_p.
	* tree-ssa-structalias.c (find_what_var_points_to): Conservatively set
	pt.null to 1.
	(find_what_p_points_to): Preserve pointer nonnull computed by VRP.
	(pt_solution_singleton_or_null_p): Renamed from
	pt_solution_singleton_p.
	* tree-ssanames.h (set_ptr_nonnull): Declare.
	(get_ptr_nonnull): Likewise.
	* tree-ssanames.c (set_ptr_nonnull): New.
	(get_ptr_nonnull): Likewise.
	* tree-vrp.c (vrp_finalize): Set ptr that are nonnull.
	(evrp_dom_walker::before_dom_children): Likewise.


gcc/testsuite/ChangeLog:

2016-10-14  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* gcc.dg/torture/pr39074-2.c: Adjust testcase.
	* gcc.dg/torture/pr39074.c: Likewise.

[-- Attachment #2: 0001-Add-nonnull-to-pointer-from-VRP.patch --]
[-- Type: text/x-patch, Size: 8825 bytes --]

From 0bc1c2efb600854148889bfdd9d121a3edc2841b Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Wed, 12 Oct 2016 13:48:07 +1100
Subject: [PATCH 1/3] Add-nonnull-to-pointer-from-VRP

---
 gcc/testsuite/gcc.dg/torture/pr39074-2.c |  2 +-
 gcc/testsuite/gcc.dg/torture/pr39074.c   |  2 +-
 gcc/tree-ssa-alias.h                     |  2 +-
 gcc/tree-ssa-ccp.c                       |  2 +-
 gcc/tree-ssa-structalias.c               | 11 ++++++--
 gcc/tree-ssanames.c                      | 29 +++++++++++++++++++++
 gcc/tree-ssanames.h                      |  2 ++
 gcc/tree-vrp.c                           | 44 +++++++++++++++++++++-----------
 8 files changed, 73 insertions(+), 21 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/torture/pr39074-2.c b/gcc/testsuite/gcc.dg/torture/pr39074-2.c
index 740b463..0693f2d 100644
--- a/gcc/testsuite/gcc.dg/torture/pr39074-2.c
+++ b/gcc/testsuite/gcc.dg/torture/pr39074-2.c
@@ -31,4 +31,4 @@ int main()
 }
 
 /* { dg-final { scan-tree-dump "y.._. = { i }" "alias" } } */
-/* { dg-final { scan-tree-dump "y.._., points-to vars: { D..... }" "alias" } } */
+/* { dg-final { scan-tree-dump "y.._., points-to NULL, points-to vars: { D..... }" "alias" } } */
diff --git a/gcc/testsuite/gcc.dg/torture/pr39074.c b/gcc/testsuite/gcc.dg/torture/pr39074.c
index 31ed499..54c444e 100644
--- a/gcc/testsuite/gcc.dg/torture/pr39074.c
+++ b/gcc/testsuite/gcc.dg/torture/pr39074.c
@@ -30,4 +30,4 @@ int main()
 }
 
 /* { dg-final { scan-tree-dump "y.._. = { i }" "alias" } } */
-/* { dg-final { scan-tree-dump "y.._., points-to vars: { D..... }" "alias" } } */
+/* { dg-final { scan-tree-dump "y.._., points-to NULL, points-to vars: { D..... }" "alias" } } */
diff --git a/gcc/tree-ssa-alias.h b/gcc/tree-ssa-alias.h
index 6680cc0..27a06fc 100644
--- a/gcc/tree-ssa-alias.h
+++ b/gcc/tree-ssa-alias.h
@@ -146,7 +146,7 @@ extern void dump_alias_stats (FILE *);
 /* In tree-ssa-structalias.c  */
 extern unsigned int compute_may_aliases (void);
 extern bool pt_solution_empty_p (struct pt_solution *);
-extern bool pt_solution_singleton_p (struct pt_solution *, unsigned *);
+extern bool pt_solution_singleton_or_null_p (struct pt_solution *, unsigned *);
 extern bool pt_solution_includes_global (struct pt_solution *);
 extern bool pt_solution_includes (struct pt_solution *, const_tree);
 extern bool pt_solutions_intersect (struct pt_solution *, struct pt_solution *);
diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index fe9a313..61754d8 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -2135,7 +2135,7 @@ fold_builtin_alloca_with_align (gimple *stmt)
       {
 	bool singleton_p;
 	unsigned uid;
-	singleton_p = pt_solution_singleton_p (&pi->pt, &uid);
+	singleton_p = pt_solution_singleton_or_null_p (&pi->pt, &uid);
 	gcc_assert (singleton_p);
 	SET_DECL_PT_UID (var, uid);
       }
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 2a4ab2f..ad65c94 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -6382,6 +6382,8 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)
 
   *slot = pt = XOBNEW (&final_solutions_obstack, struct pt_solution);
   memset (pt, 0, sizeof (struct pt_solution));
+  /* Conservatively set to NULL from PTA (to true). */
+  pt->null = 1;
 
   /* Translate artificial variables into SSA_NAME_PTR_INFO
      attributes.  */
@@ -6451,6 +6453,7 @@ find_what_p_points_to (tree fndecl, tree p)
   struct ptr_info_def *pi;
   tree lookup_p = p;
   varinfo_t vi;
+  bool nonnull = get_ptr_nonnull (p);
 
   /* For parameters, get at the points-to set for the actual parm
      decl.  */
@@ -6466,6 +6469,10 @@ find_what_p_points_to (tree fndecl, tree p)
 
   pi = get_ptr_info (p);
   pi->pt = find_what_var_points_to (fndecl, vi);
+  /* Preserve pointer nonnull computed by VRP.  See get_ptr_nonnull
+     in gcc/tree-ssaname.c for more information.  */
+  if (nonnull)
+    set_ptr_nonnull (p);
 }
 
 
@@ -6599,10 +6606,10 @@ pt_solution_empty_p (struct pt_solution *pt)
    return the var uid in *UID.  */
 
 bool
-pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid)
+pt_solution_singleton_or_null_p (struct pt_solution *pt, unsigned *uid)
 {
   if (pt->anything || pt->nonlocal || pt->escaped || pt->ipa_escaped
-      || pt->null || pt->vars == NULL
+      || pt->vars == NULL
       || !bitmap_single_bit_set_p (pt->vars))
     return false;
 
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 64ab13a..fe6a0f7 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -374,6 +374,35 @@ get_range_info (const_tree name, wide_int *min, wide_int *max)
   return SSA_NAME_RANGE_TYPE (name);
 }
 
+/* Set nonnull attribute to pointer NAME.  */
+
+void
+set_ptr_nonnull (tree name)
+{
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+  struct ptr_info_def *pi = get_ptr_info (name);
+  pi->pt.null = 0;
+}
+
+/* Return nonnull attribute of pointer NAME.  */
+bool
+get_ptr_nonnull (const_tree name)
+{
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+  struct ptr_info_def *pi = SSA_NAME_PTR_INFO (name);
+  if (pi == NULL)
+    return false;
+  /* TODO Now pt->null is conservatively set to null in PTA
+     analysis. vrp is the only pass (including ipa-vrp)
+     that clears pt.null via set_ptr_nonull when it knows
+     for sure. PTA will preserves the pt.null value set by VRP.
+
+     When PTA analysis is improved, pt.anything, pt.nonlocal
+     and pt.escaped may also has to be considered before
+     deciding that pointer cannot point to NULL.  */
+  return !pi->pt.null;
+}
+
 /* Change non-zero bits bitmask of NAME.  */
 
 void
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index 4496e1d..d39cc9d 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -88,6 +88,8 @@ extern void set_ptr_info_alignment (struct ptr_info_def *, unsigned int,
 extern void adjust_ptr_info_misalignment (struct ptr_info_def *,
 					  unsigned int);
 extern struct ptr_info_def *get_ptr_info (tree);
+extern void set_ptr_nonnull (tree);
+extern bool get_ptr_nonnull (const_tree);
 
 extern tree copy_ssa_name_fn (struct function *, tree, gimple *);
 extern void duplicate_ssa_name_ptr_info (tree, struct ptr_info_def *);
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 8a129c6..7e4f947 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -10603,18 +10603,24 @@ vrp_finalize (bool warn_array_bounds_p)
       {
 	tree name = ssa_name (i);
 
-      if (!name
-	  || POINTER_TYPE_P (TREE_TYPE (name))
-	  || (vr_value[i]->type == VR_VARYING)
-	  || (vr_value[i]->type == VR_UNDEFINED))
-	continue;
+	if (!name
+	    || (vr_value[i]->type == VR_VARYING)
+	    || (vr_value[i]->type == VR_UNDEFINED)
+	    || (TREE_CODE (vr_value[i]->min) != INTEGER_CST)
+	    || (TREE_CODE (vr_value[i]->max) != INTEGER_CST))
+	  continue;
 
-      if ((TREE_CODE (vr_value[i]->min) == INTEGER_CST)
-	  && (TREE_CODE (vr_value[i]->max) == INTEGER_CST)
-	  && (vr_value[i]->type == VR_RANGE
-	      || vr_value[i]->type == VR_ANTI_RANGE))
-	set_range_info (name, vr_value[i]->type, vr_value[i]->min,
-			vr_value[i]->max);
+	if (POINTER_TYPE_P (TREE_TYPE (name))
+	    && ((vr_value[i]->type == VR_RANGE
+		 && range_includes_zero_p (vr_value[i]->min,
+					   vr_value[i]->max) == 0)
+		|| (vr_value[i]->type == VR_ANTI_RANGE
+		    && range_includes_zero_p (vr_value[i]->min,
+					      vr_value[i]->max) == 1)))
+	  set_ptr_nonnull (name);
+	else if (!POINTER_TYPE_P (TREE_TYPE (name)))
+	  set_range_info (name, vr_value[i]->type, vr_value[i]->min,
+			  vr_value[i]->max);
       }
 
   substitute_and_fold (op_with_constant_singleton_value_range,
@@ -10817,17 +10823,25 @@ evrp_dom_walker::before_dom_children (basic_block bb)
 	  def_operand_p def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
 	  /* Set the SSA with the value range.  */
 	  if (def_p
-	      && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME
-	      && INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p))))
+	      && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME)
 	    {
 	      tree def = DEF_FROM_PTR (def_p);
 	      value_range *vr = get_value_range (def);
 
-	      if ((vr->type == VR_RANGE
-		   || vr->type == VR_ANTI_RANGE)
+	      if (INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p)))
+		  && (vr->type == VR_RANGE
+		      || vr->type == VR_ANTI_RANGE)
 		  && (TREE_CODE (vr->min) == INTEGER_CST)
 		  && (TREE_CODE (vr->max) == INTEGER_CST))
 		set_range_info (def, vr->type, vr->min, vr->max);
+	      else if (POINTER_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p)))
+		       && ((vr->type == VR_RANGE
+			    && range_includes_zero_p (vr->min,
+						      vr->max) == 0)
+			   || (vr->type == VR_ANTI_RANGE
+			       && range_includes_zero_p (vr->min,
+							 vr->max) == 1)))
+		set_ptr_nonnull (def);
 	    }
 	}
       else
-- 
2.7.4

Re: [ipa-vrp] Use get/set_ptr_nonnull in ipa-vrp

[kugan]

Hi Honza,

On 12/10/16 22:16, Jan Hubicka wrote:
>> Hi,
>>
>> This patch uses the get/set_ptr_nonnull so that ipa-vrp also
>> propagates nonnull ranges for pinter.
>>
>> Bootstrapped and regression tested this with other patched without
>> any new regressions on x86_64-linux-gnu.
>>
>> Is this OK for trunk?
>>
>> Thanks,
>> Kugan
>>
>>
>>
>>
>> gcc/ChangeLog:
>>
>> 2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
>>
>> 	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Set value range
>> 	  for pointer type too.
>> 	(ipcp_update_vr): set_ptr_nonnull for pointer.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-10-12  Kugan Vivekanandarajah  <kugan.vivekanandarajah@linaro.org>
>>
>> 	* gcc.dg/ipa/vrp4.c: New test.
> OK, thank you!
> We should be able to derive a lot of (useful) non-null information from the
> fact that the pointers are dereferenced either prior the function call or in a
> statement that postdominate the function entry.
I will try with spec2k/2006. Do you have any specific benchmark in mind 
that I can try first?

   I guess we could also give (semi)
> useful -Wmissing-attribute=nonnull hints in that case.

I will send a follow up patch for this.

Thanks,
Kugan
>
> Honza
>

Re: Set nonnull attribute to ptr_info_def based on VRP

[Richard Biener]

On Fri, Oct 14, 2016 at 1:12 AM, kugan
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
>
> On 13/10/16 20:44, Richard Biener wrote:
>>
>> On Thu, Oct 13, 2016 at 6:49 AM, kugan
>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>
>>> Hi Richard,
>>>
>>>>
>>>> what does this try to do?  Preserve info VRP computed across PTA?
>>>>
>>>> I think we didn't yet sort out the nonlocal/escaped vs. null handling
>>>> properly
>>>> (or how PTA should handle get_ptr_nonnull).  The way you are using it
>>>> asks for pt.null to be orthogonal to nonlocal/escaped and thus having
>>>> nonlocal or escaped would also require setting ptr.null in PTA.  It then
>>>> would be also more canonical to set it for pt.anything as well.  Which
>>>> means conservatively handling it would be equivalent to flipping its
>>>> semantic and changing its name to pt.nonnull.
>>>>
>>>> That said, you seem to be simply "reserving" the bit for VRP, keeping it
>>>> conservatively true when "not computed".  I guess I'm fine with this for
>>>> now
>>>> but it should be documented in the header file that way.
>>>>
>>>
>>> Thanks for the comments.
>>>
>>> To summarize, currently I am not relying on PTA analysis at all. Just
>>> saving
>>> null from VRP (or rather nonnull) and preserving it across PTA. Primary
>>> intention is to pass it for PARM_DECL SSA names (from ipa-vrp).
>>>
>>> In this case, using  pt.anything/nonlocal/escaped will only make the
>>> result
>>> more pessimistic.
>>>
>>> Ideally, we should improve pt.null within PTA but for now as you said, I
>>> will document it.
>>>
>>> When we start using pt.null from PTA analysis, we would also have to take
>>> into account pt.anything/nonlocal/escaped.
>>>
>>> Does that make sense?
>>
>>
>> Yes.
>
>
>
> Here is the revised patch based on the review. I also had to adjust two
> testcases since we set pt.null conservatively and dumps that too.

Hmm, sorry for another request, but seeing the testcase change I rather
want to expose the conservatism only at find_what_p_points_to time.

Thus,

@@ -6382,6 +6382,8 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)

   *slot = pt = XOBNEW (&final_solutions_obstack, struct pt_solution);
   memset (pt, 0, sizeof (struct pt_solution));
+  /* Conservatively set to NULL from PTA (to true). */
+  pt->null = 1;

   /* Translate artificial variables into SSA_NAME_PTR_INFO
      attributes.  */

remove this hunk

@@ -6466,6 +6469,10 @@ find_what_p_points_to (tree fndecl, tree p)

   pi = get_ptr_info (p);
   pi->pt = find_what_var_points_to (fndecl, vi);
+  /* Preserve pointer nonnull computed by VRP.  See get_ptr_nonnull
+     in gcc/tree-ssaname.c for more information.  */
+  if (nonnull)
+    set_ptr_nonnull (p);

and add pi->pt.null = true; here (with the comment from above).  This
preserves the (possibly incorrect) PTA solution in the dumps but does
not leak it to SSA_NAME_PTR_INFO.

+bool
+get_ptr_nonnull (const_tree name)
+{
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+  struct ptr_info_def *pi = SSA_NAME_PTR_INFO (name);
+  if (pi == NULL)
+    return false;
+  /* TODO Now pt->null is conservatively set to null in PTA

set to true

+     analysis. vrp is the only pass (including ipa-vrp)
+     that clears pt.null via set_ptr_nonull when it knows
+     for sure. PTA will preserves the pt.null value set by VRP.
+

Ok with those changes.

Thanks,
Richard.


> Thanks,
> Kugan
>
> gcc/ChangeLog:
>
> 2016-10-14  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>         * tree-ssa-alias.h (pt_solution_singleton_or_null_p): Renamed from
>         pt_solution_singleton_p.
>         * tree-ssa-ccp.c (fold_builtin_alloca_with_align): Use renamed
>         pt_solution_singleton_or_null_p from pt_solution_singleton_p.
>         * tree-ssa-structalias.c (find_what_var_points_to): Conservatively
> set
>         pt.null to 1.
>         (find_what_p_points_to): Preserve pointer nonnull computed by VRP.
>         (pt_solution_singleton_or_null_p): Renamed from
>         pt_solution_singleton_p.
>         * tree-ssanames.h (set_ptr_nonnull): Declare.
>         (get_ptr_nonnull): Likewise.
>         * tree-ssanames.c (set_ptr_nonnull): New.
>         (get_ptr_nonnull): Likewise.
>         * tree-vrp.c (vrp_finalize): Set ptr that are nonnull.
>         (evrp_dom_walker::before_dom_children): Likewise.
>
>
> gcc/testsuite/ChangeLog:
>
> 2016-10-14  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>         * gcc.dg/torture/pr39074-2.c: Adjust testcase.
>         * gcc.dg/torture/pr39074.c: Likewise.

Re: Set nonnull attribute to ptr_info_def based on VRP

[kugan]

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

Hi Richard,

On 14/10/16 23:53, Richard Biener wrote:
> On Fri, Oct 14, 2016 at 1:12 AM, kugan
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi Richard,
>>
>>
>> On 13/10/16 20:44, Richard Biener wrote:
>>>
>>> On Thu, Oct 13, 2016 at 6:49 AM, kugan
>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>
>>>> Hi Richard,
>>>>
>>>>>
>>>>> what does this try to do?  Preserve info VRP computed across PTA?
>>>>>
>>>>> I think we didn't yet sort out the nonlocal/escaped vs. null handling
>>>>> properly
>>>>> (or how PTA should handle get_ptr_nonnull).  The way you are using it
>>>>> asks for pt.null to be orthogonal to nonlocal/escaped and thus having
>>>>> nonlocal or escaped would also require setting ptr.null in PTA.  It then
>>>>> would be also more canonical to set it for pt.anything as well.  Which
>>>>> means conservatively handling it would be equivalent to flipping its
>>>>> semantic and changing its name to pt.nonnull.
>>>>>
>>>>> That said, you seem to be simply "reserving" the bit for VRP, keeping it
>>>>> conservatively true when "not computed".  I guess I'm fine with this for
>>>>> now
>>>>> but it should be documented in the header file that way.
>>>>>
>>>>
>>>> Thanks for the comments.
>>>>
>>>> To summarize, currently I am not relying on PTA analysis at all. Just
>>>> saving
>>>> null from VRP (or rather nonnull) and preserving it across PTA. Primary
>>>> intention is to pass it for PARM_DECL SSA names (from ipa-vrp).
>>>>
>>>> In this case, using  pt.anything/nonlocal/escaped will only make the
>>>> result
>>>> more pessimistic.
>>>>
>>>> Ideally, we should improve pt.null within PTA but for now as you said, I
>>>> will document it.
>>>>
>>>> When we start using pt.null from PTA analysis, we would also have to take
>>>> into account pt.anything/nonlocal/escaped.
>>>>
>>>> Does that make sense?
>>>
>>>
>>> Yes.
>>
>>
>>
>> Here is the revised patch based on the review. I also had to adjust two
>> testcases since we set pt.null conservatively and dumps that too.
>
> Hmm, sorry for another request, but seeing the testcase change I rather
> want to expose the conservatism only at find_what_p_points_to time.
>
> Thus,
>
> @@ -6382,6 +6382,8 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)
>
>    *slot = pt = XOBNEW (&final_solutions_obstack, struct pt_solution);
>    memset (pt, 0, sizeof (struct pt_solution));
> +  /* Conservatively set to NULL from PTA (to true). */
> +  pt->null = 1;
>
>    /* Translate artificial variables into SSA_NAME_PTR_INFO
>       attributes.  */
>
> remove this hunk
>
> @@ -6466,6 +6469,10 @@ find_what_p_points_to (tree fndecl, tree p)
>
>    pi = get_ptr_info (p);
>    pi->pt = find_what_var_points_to (fndecl, vi);
> +  /* Preserve pointer nonnull computed by VRP.  See get_ptr_nonnull
> +     in gcc/tree-ssaname.c for more information.  */
> +  if (nonnull)
> +    set_ptr_nonnull (p);
>
> and add pi->pt.null = true; here (with the comment from above).  This
> preserves the (possibly incorrect) PTA solution in the dumps but does
> not leak it to SSA_NAME_PTR_INFO.
>
> +bool
> +get_ptr_nonnull (const_tree name)
> +{
> +  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
> +  struct ptr_info_def *pi = SSA_NAME_PTR_INFO (name);
> +  if (pi == NULL)
> +    return false;
> +  /* TODO Now pt->null is conservatively set to null in PTA
>
> set to true
>
> +     analysis. vrp is the only pass (including ipa-vrp)
> +     that clears pt.null via set_ptr_nonull when it knows
> +     for sure. PTA will preserves the pt.null value set by VRP.
> +
>
> Ok with those changes.

Thanks for the review. Here is the updated patch. I also had to add 
pt.null to true in pt_solution_reset.

I will commit this version if there is no objection.

Thanks,
Kugan

> Thanks,
> Richard.
>
>
>> Thanks,
>> Kugan
>>
>> gcc/ChangeLog:
>>
>> 2016-10-14  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>>         * tree-ssa-alias.h (pt_solution_singleton_or_null_p): Renamed from
>>         pt_solution_singleton_p.
>>         * tree-ssa-ccp.c (fold_builtin_alloca_with_align): Use renamed
>>         pt_solution_singleton_or_null_p from pt_solution_singleton_p.
>>         * tree-ssa-structalias.c (find_what_var_points_to): Conservatively
>> set
>>         pt.null to 1.
>>         (find_what_p_points_to): Preserve pointer nonnull computed by VRP.
>>         (pt_solution_singleton_or_null_p): Renamed from
>>         pt_solution_singleton_p.
>>         * tree-ssanames.h (set_ptr_nonnull): Declare.
>>         (get_ptr_nonnull): Likewise.
>>         * tree-ssanames.c (set_ptr_nonnull): New.
>>         (get_ptr_nonnull): Likewise.
>>         * tree-vrp.c (vrp_finalize): Set ptr that are nonnull.
>>         (evrp_dom_walker::before_dom_children): Likewise.
>>
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-10-14  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>>         * gcc.dg/torture/pr39074-2.c: Adjust testcase.
>>         * gcc.dg/torture/pr39074.c: Likewise.

[-- Attachment #2: 0001-Add-nonnull-to-pointer-from-VRP.patch --]
[-- Type: text/x-patch, Size: 8776 bytes --]

From 8349bce90dcf2b17b6f129149b2073e7acc6fa8c Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Wed, 12 Oct 2016 13:48:07 +1100
Subject: [PATCH 1/3] Add-nonnull-to-pointer-from-VRP

---
 gcc/testsuite/gcc.dg/torture/pr39074-2.c |  2 +-
 gcc/testsuite/gcc.dg/torture/pr39074.c   |  2 +-
 gcc/tree-ssa-alias.h                     |  2 +-
 gcc/tree-ssa-ccp.c                       |  2 +-
 gcc/tree-ssa-structalias.c               | 12 +++++++--
 gcc/tree-ssanames.c                      | 29 +++++++++++++++++++++
 gcc/tree-ssanames.h                      |  2 ++
 gcc/tree-vrp.c                           | 44 +++++++++++++++++++++-----------
 8 files changed, 74 insertions(+), 21 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/torture/pr39074-2.c b/gcc/testsuite/gcc.dg/torture/pr39074-2.c
index 740b463..0693f2d 100644
--- a/gcc/testsuite/gcc.dg/torture/pr39074-2.c
+++ b/gcc/testsuite/gcc.dg/torture/pr39074-2.c
@@ -31,4 +31,4 @@ int main()
 }
 
 /* { dg-final { scan-tree-dump "y.._. = { i }" "alias" } } */
-/* { dg-final { scan-tree-dump "y.._., points-to vars: { D..... }" "alias" } } */
+/* { dg-final { scan-tree-dump "y.._., points-to NULL, points-to vars: { D..... }" "alias" } } */
diff --git a/gcc/testsuite/gcc.dg/torture/pr39074.c b/gcc/testsuite/gcc.dg/torture/pr39074.c
index 31ed499..54c444e 100644
--- a/gcc/testsuite/gcc.dg/torture/pr39074.c
+++ b/gcc/testsuite/gcc.dg/torture/pr39074.c
@@ -30,4 +30,4 @@ int main()
 }
 
 /* { dg-final { scan-tree-dump "y.._. = { i }" "alias" } } */
-/* { dg-final { scan-tree-dump "y.._., points-to vars: { D..... }" "alias" } } */
+/* { dg-final { scan-tree-dump "y.._., points-to NULL, points-to vars: { D..... }" "alias" } } */
diff --git a/gcc/tree-ssa-alias.h b/gcc/tree-ssa-alias.h
index 6680cc0..27a06fc 100644
--- a/gcc/tree-ssa-alias.h
+++ b/gcc/tree-ssa-alias.h
@@ -146,7 +146,7 @@ extern void dump_alias_stats (FILE *);
 /* In tree-ssa-structalias.c  */
 extern unsigned int compute_may_aliases (void);
 extern bool pt_solution_empty_p (struct pt_solution *);
-extern bool pt_solution_singleton_p (struct pt_solution *, unsigned *);
+extern bool pt_solution_singleton_or_null_p (struct pt_solution *, unsigned *);
 extern bool pt_solution_includes_global (struct pt_solution *);
 extern bool pt_solution_includes (struct pt_solution *, const_tree);
 extern bool pt_solutions_intersect (struct pt_solution *, struct pt_solution *);
diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index fe9a313..61754d8 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -2135,7 +2135,7 @@ fold_builtin_alloca_with_align (gimple *stmt)
       {
 	bool singleton_p;
 	unsigned uid;
-	singleton_p = pt_solution_singleton_p (&pi->pt, &uid);
+	singleton_p = pt_solution_singleton_or_null_p (&pi->pt, &uid);
 	gcc_assert (singleton_p);
 	SET_DECL_PT_UID (var, uid);
       }
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 2a4ab2f..78f4533 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -6451,6 +6451,7 @@ find_what_p_points_to (tree fndecl, tree p)
   struct ptr_info_def *pi;
   tree lookup_p = p;
   varinfo_t vi;
+  bool nonnull = get_ptr_nonnull (p);
 
   /* For parameters, get at the points-to set for the actual parm
      decl.  */
@@ -6466,6 +6467,12 @@ find_what_p_points_to (tree fndecl, tree p)
 
   pi = get_ptr_info (p);
   pi->pt = find_what_var_points_to (fndecl, vi);
+  /* Conservatively set to NULL from PTA (to true). */
+  pi->pt.null = 1;
+  /* Preserve pointer nonnull computed by VRP.  See get_ptr_nonnull
+     in gcc/tree-ssaname.c for more information.  */
+  if (nonnull)
+    set_ptr_nonnull (p);
 }
 
 
@@ -6505,6 +6512,7 @@ pt_solution_reset (struct pt_solution *pt)
 {
   memset (pt, 0, sizeof (struct pt_solution));
   pt->anything = true;
+  pt->null = true;
 }
 
 /* Set the points-to solution *PT to point only to the variables
@@ -6599,10 +6607,10 @@ pt_solution_empty_p (struct pt_solution *pt)
    return the var uid in *UID.  */
 
 bool
-pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid)
+pt_solution_singleton_or_null_p (struct pt_solution *pt, unsigned *uid)
 {
   if (pt->anything || pt->nonlocal || pt->escaped || pt->ipa_escaped
-      || pt->null || pt->vars == NULL
+      || pt->vars == NULL
       || !bitmap_single_bit_set_p (pt->vars))
     return false;
 
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 64ab13a..913d142 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -374,6 +374,35 @@ get_range_info (const_tree name, wide_int *min, wide_int *max)
   return SSA_NAME_RANGE_TYPE (name);
 }
 
+/* Set nonnull attribute to pointer NAME.  */
+
+void
+set_ptr_nonnull (tree name)
+{
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+  struct ptr_info_def *pi = get_ptr_info (name);
+  pi->pt.null = 0;
+}
+
+/* Return nonnull attribute of pointer NAME.  */
+bool
+get_ptr_nonnull (const_tree name)
+{
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+  struct ptr_info_def *pi = SSA_NAME_PTR_INFO (name);
+  if (pi == NULL)
+    return false;
+  /* TODO Now pt->null is conservatively set to true in PTA
+     analysis. vrp is the only pass (including ipa-vrp)
+     that clears pt.null via set_ptr_nonull when it knows
+     for sure. PTA will preserves the pt.null value set by VRP.
+
+     When PTA analysis is improved, pt.anything, pt.nonlocal
+     and pt.escaped may also has to be considered before
+     deciding that pointer cannot point to NULL.  */
+  return !pi->pt.null;
+}
+
 /* Change non-zero bits bitmask of NAME.  */
 
 void
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index 4496e1d..d39cc9d 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -88,6 +88,8 @@ extern void set_ptr_info_alignment (struct ptr_info_def *, unsigned int,
 extern void adjust_ptr_info_misalignment (struct ptr_info_def *,
 					  unsigned int);
 extern struct ptr_info_def *get_ptr_info (tree);
+extern void set_ptr_nonnull (tree);
+extern bool get_ptr_nonnull (const_tree);
 
 extern tree copy_ssa_name_fn (struct function *, tree, gimple *);
 extern void duplicate_ssa_name_ptr_info (tree, struct ptr_info_def *);
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 8a129c6..7e4f947 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -10603,18 +10603,24 @@ vrp_finalize (bool warn_array_bounds_p)
       {
 	tree name = ssa_name (i);
 
-      if (!name
-	  || POINTER_TYPE_P (TREE_TYPE (name))
-	  || (vr_value[i]->type == VR_VARYING)
-	  || (vr_value[i]->type == VR_UNDEFINED))
-	continue;
+	if (!name
+	    || (vr_value[i]->type == VR_VARYING)
+	    || (vr_value[i]->type == VR_UNDEFINED)
+	    || (TREE_CODE (vr_value[i]->min) != INTEGER_CST)
+	    || (TREE_CODE (vr_value[i]->max) != INTEGER_CST))
+	  continue;
 
-      if ((TREE_CODE (vr_value[i]->min) == INTEGER_CST)
-	  && (TREE_CODE (vr_value[i]->max) == INTEGER_CST)
-	  && (vr_value[i]->type == VR_RANGE
-	      || vr_value[i]->type == VR_ANTI_RANGE))
-	set_range_info (name, vr_value[i]->type, vr_value[i]->min,
-			vr_value[i]->max);
+	if (POINTER_TYPE_P (TREE_TYPE (name))
+	    && ((vr_value[i]->type == VR_RANGE
+		 && range_includes_zero_p (vr_value[i]->min,
+					   vr_value[i]->max) == 0)
+		|| (vr_value[i]->type == VR_ANTI_RANGE
+		    && range_includes_zero_p (vr_value[i]->min,
+					      vr_value[i]->max) == 1)))
+	  set_ptr_nonnull (name);
+	else if (!POINTER_TYPE_P (TREE_TYPE (name)))
+	  set_range_info (name, vr_value[i]->type, vr_value[i]->min,
+			  vr_value[i]->max);
       }
 
   substitute_and_fold (op_with_constant_singleton_value_range,
@@ -10817,17 +10823,25 @@ evrp_dom_walker::before_dom_children (basic_block bb)
 	  def_operand_p def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
 	  /* Set the SSA with the value range.  */
 	  if (def_p
-	      && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME
-	      && INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p))))
+	      && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME)
 	    {
 	      tree def = DEF_FROM_PTR (def_p);
 	      value_range *vr = get_value_range (def);
 
-	      if ((vr->type == VR_RANGE
-		   || vr->type == VR_ANTI_RANGE)
+	      if (INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p)))
+		  && (vr->type == VR_RANGE
+		      || vr->type == VR_ANTI_RANGE)
 		  && (TREE_CODE (vr->min) == INTEGER_CST)
 		  && (TREE_CODE (vr->max) == INTEGER_CST))
 		set_range_info (def, vr->type, vr->min, vr->max);
+	      else if (POINTER_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p)))
+		       && ((vr->type == VR_RANGE
+			    && range_includes_zero_p (vr->min,
+						      vr->max) == 0)
+			   || (vr->type == VR_ANTI_RANGE
+			       && range_includes_zero_p (vr->min,
+							 vr->max) == 1)))
+		set_ptr_nonnull (def);
 	    }
 	}
       else
-- 
2.7.4