diff mbox

[RFC,IPA-VRP] ADDR_EXPR and nonnull

Message ID edc65ca7-a71f-2fc0-1455-e49f357c7cfb@linaro.org
State New
Headers show

Commit Message

Kugan Vivekanandarajah Oct. 19, 2016, 2:12 a.m. UTC
Hi,

While computing jump function value range for pointer, I am wondering if 
we can assume that any tree with ADDR_EXPR will be nonnull.

That is, in cases like:

int arr[10];
foo (&arr[1]);

OR

struct st
{
   int a;
   int b;
};
struct st s2;
foo (&s2.a);

Attached patch tries to do this. I am not sure if this can be wrong. Any 
thoughts?

Attached patch bootstraps and regression testing didn't introduce any 
new regressions.

Thanks,
Kugan


gcc/ChangeLog:

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

	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Set
	value range to nonull for ADDR_EXPR.

gcc/testsuite/ChangeLog:

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

	* gcc.dg/ipa/vrp5.c: New test.
	* gcc.dg/ipa/vrp6.c: New test.

Comments

Richard Biener Oct. 19, 2016, 8:23 a.m. UTC | #1
On Wed, 19 Oct 2016, kugan wrote:

> Hi,
> 
> While computing jump function value range for pointer, I am wondering if we
> can assume that any tree with ADDR_EXPR will be nonnull.
> 
> That is, in cases like:
> 
> int arr[10];
> foo (&arr[1]);
> 
> OR
> 
> struct st
> {
>   int a;
>   int b;
> };
> struct st s2;
> foo (&s2.a);
> 
> Attached patch tries to do this. I am not sure if this can be wrong. Any
> thoughts?

It can be wrong for weak symbols for example.

Richard.

> Attached patch bootstraps and regression testing didn't introduce any new
> regressions.
> 
> Thanks,
> Kugan
> 
> 
> gcc/ChangeLog:
> 
> 2016-10-19  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Set
> 	value range to nonull for ADDR_EXPR.
> 
> gcc/testsuite/ChangeLog:
> 
> 2016-10-19  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* gcc.dg/ipa/vrp5.c: New test.
> 	* gcc.dg/ipa/vrp6.c: New test.
>
Kugan Vivekanandarajah Oct. 19, 2016, 8:48 a.m. UTC | #2
Hi Richard,


On 19/10/16 19:23, Richard Biener wrote:
> On Wed, 19 Oct 2016, kugan wrote:
>
>> Hi,
>>
>> While computing jump function value range for pointer, I am wondering if we
>> can assume that any tree with ADDR_EXPR will be nonnull.
>>
>> That is, in cases like:
>>
>> int arr[10];
>> foo (&arr[1]);
>>
>> OR
>>
>> struct st
>> {
>>   int a;
>>   int b;
>> };
>> struct st s2;
>> foo (&s2.a);
>>
>> Attached patch tries to do this. I am not sure if this can be wrong. Any
>> thoughts?
>
> It can be wrong for weak symbols for example.
Would excluding weak symbols (I believe I can check DECL_WEAK for this) 
good enough. Or looking for acceptable subset would work?

Thanks,
Kugan

>
> Richard.
>
>> Attached patch bootstraps and regression testing didn't introduce any new
>> regressions.
>>
>> Thanks,
>> Kugan
>>
>>
>> gcc/ChangeLog:
>>
>> 2016-10-19  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>> 	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Set
>> 	value range to nonull for ADDR_EXPR.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-10-19  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>> 	* gcc.dg/ipa/vrp5.c: New test.
>> 	* gcc.dg/ipa/vrp6.c: New test.
>>
>
Richard Biener Oct. 19, 2016, 9:26 a.m. UTC | #3
On Wed, 19 Oct 2016, kugan wrote:

> Hi Richard,
> 
> 
> On 19/10/16 19:23, Richard Biener wrote:
> > On Wed, 19 Oct 2016, kugan wrote:
> > 
> > > Hi,
> > > 
> > > While computing jump function value range for pointer, I am wondering if
> > > we
> > > can assume that any tree with ADDR_EXPR will be nonnull.
> > > 
> > > That is, in cases like:
> > > 
> > > int arr[10];
> > > foo (&arr[1]);
> > > 
> > > OR
> > > 
> > > struct st
> > > {
> > >   int a;
> > >   int b;
> > > };
> > > struct st s2;
> > > foo (&s2.a);
> > > 
> > > Attached patch tries to do this. I am not sure if this can be wrong. Any
> > > thoughts?
> > 
> > It can be wrong for weak symbols for example.
> Would excluding weak symbols (I believe I can check DECL_WEAK for this) good
> enough. Or looking for acceptable subset would work?

I think we should add a symtab helper to tell if address_nonzero_p (if
that doesn't aleady exist).

Richard.

> Thanks,
> Kugan
> 
> > 
> > Richard.
> > 
> > > Attached patch bootstraps and regression testing didn't introduce any new
> > > regressions.
> > > 
> > > Thanks,
> > > Kugan
> > > 
> > > 
> > > gcc/ChangeLog:
> > > 
> > > 2016-10-19  Kugan Vivekanandarajah  <kuganv@linaro.org>
> > > 
> > > 	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Set
> > > 	value range to nonull for ADDR_EXPR.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 2016-10-19  Kugan Vivekanandarajah  <kuganv@linaro.org>
> > > 
> > > 	* gcc.dg/ipa/vrp5.c: New test.
> > > 	* gcc.dg/ipa/vrp6.c: New test.
> > > 
> > 
> 
>
Jan Hubicka Oct. 19, 2016, 2:26 p.m. UTC | #4
> > Would excluding weak symbols (I believe I can check DECL_WEAK for this) good
> > enough. Or looking for acceptable subset would work?
> 
> I think we should add a symtab helper to tell if address_nonzero_p (if
> that doesn't aleady exist).

We have node->nonzero_address()

Honza
> 
> Richard.
> 
> > Thanks,
> > Kugan
> > 
> > > 
> > > Richard.
> > > 
> > > > Attached patch bootstraps and regression testing didn't introduce any new
> > > > regressions.
> > > > 
> > > > Thanks,
> > > > Kugan
> > > > 
> > > > 
> > > > gcc/ChangeLog:
> > > > 
> > > > 2016-10-19  Kugan Vivekanandarajah  <kuganv@linaro.org>
> > > > 
> > > > 	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Set
> > > > 	value range to nonull for ADDR_EXPR.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > 2016-10-19  Kugan Vivekanandarajah  <kuganv@linaro.org>
> > > > 
> > > > 	* gcc.dg/ipa/vrp5.c: New test.
> > > > 	* gcc.dg/ipa/vrp6.c: New test.
> > > > 
> > > 
> > 
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
diff mbox

Patch

diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 353b638..b795d30 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -1670,9 +1670,10 @@  ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
 
       if (POINTER_TYPE_P (TREE_TYPE (arg)))
 	{
-	  if (TREE_CODE (arg) == SSA_NAME
-	      && param_type
-	      && get_ptr_nonnull (arg))
+	  if ((TREE_CODE (arg) == SSA_NAME
+	       && param_type
+	       && get_ptr_nonnull (arg))
+	      || (TREE_CODE (arg) == ADDR_EXPR))
 	    {
 	      jfunc->vr_known = true;
 	      jfunc->m_vr.type = VR_ANTI_RANGE;
diff --git a/gcc/testsuite/gcc.dg/ipa/vrp5.c b/gcc/testsuite/gcc.dg/ipa/vrp5.c
index e69de29..8e43a65 100644
--- a/gcc/testsuite/gcc.dg/ipa/vrp5.c
+++ b/gcc/testsuite/gcc.dg/ipa/vrp5.c
@@ -0,0 +1,28 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-cp-details -fdump-tree-vrp1" } */
+
+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)
+{
+int arr[10];
+  if (!s)
+    return 0;
+  foo (&s->a);
+  foo (&arr[1]);
+}
+
+/* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */
+/* { dg-final { scan-tree-dump-times "if" 1 "vrp1" } } */
diff --git a/gcc/testsuite/gcc.dg/ipa/vrp6.c b/gcc/testsuite/gcc.dg/ipa/vrp6.c
index e69de29..f837758 100644
--- a/gcc/testsuite/gcc.dg/ipa/vrp6.c
+++ b/gcc/testsuite/gcc.dg/ipa/vrp6.c
@@ -0,0 +1,28 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-cp-details -fdump-tree-vrp1" } */
+
+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)
+{
+struct st s2;
+  if (!s)
+    return 0;
+  foo (&s->a);
+  foo (&s2.a);
+}
+
+/* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */
+/* { dg-final { scan-tree-dump-times "if" 1 "vrp1" } } */