* [RFC][IPA-VRP] ADDR_EXPR and nonnull
@ 2016-10-19 2:12 kugan
2016-10-19 8:24 ` Richard Biener
0 siblings, 1 reply; 14+ messages in thread
From: kugan @ 2016-10-19 2:12 UTC (permalink / raw)
To: Richard Biener, Jan Hubicka, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 767 bytes --]
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.
[-- Attachment #2: p.txt --]
[-- Type: text/plain, Size: 1919 bytes --]
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" } } */
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-19 2:12 [RFC][IPA-VRP] ADDR_EXPR and nonnull kugan
@ 2016-10-19 8:24 ` Richard Biener
2016-10-19 8:48 ` kugan
0 siblings, 1 reply; 14+ messages in thread
From: Richard Biener @ 2016-10-19 8:24 UTC (permalink / raw)
To: kugan; +Cc: Jan Hubicka, gcc-patches
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.
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-19 8:24 ` Richard Biener
@ 2016-10-19 8:48 ` kugan
2016-10-19 9:26 ` Richard Biener
0 siblings, 1 reply; 14+ messages in thread
From: kugan @ 2016-10-19 8:48 UTC (permalink / raw)
To: Richard Biener; +Cc: Jan Hubicka, gcc-patches
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.
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-19 8:48 ` kugan
@ 2016-10-19 9:26 ` Richard Biener
2016-10-19 14:26 ` Jan Hubicka
0 siblings, 1 reply; 14+ messages in thread
From: Richard Biener @ 2016-10-19 9:26 UTC (permalink / raw)
To: kugan; +Cc: Jan Hubicka, gcc-patches
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.
> > >
> >
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-19 9:26 ` Richard Biener
@ 2016-10-19 14:26 ` Jan Hubicka
2016-10-20 3:13 ` kugan
0 siblings, 1 reply; 14+ messages in thread
From: Jan Hubicka @ 2016-10-19 14:26 UTC (permalink / raw)
To: Richard Biener; +Cc: kugan, Jan Hubicka, gcc-patches
> > 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)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-19 14:26 ` Jan Hubicka
@ 2016-10-20 3:13 ` kugan
2016-10-20 7:42 ` Richard Biener
0 siblings, 1 reply; 14+ messages in thread
From: kugan @ 2016-10-20 3:13 UTC (permalink / raw)
To: Jan Hubicka, Richard Biener; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]
On 20/10/16 01:26, Jan Hubicka wrote:
>>> 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()
Thanks for the pointer. Here is an attempt. Does this look OK?
+ if (TREE_CODE (arg) == ADDR_EXPR)
+ {
+ /* See if the AADR_EXPR is nonnull. */
+ varpool_node *node = NULL;
+ tree base = TREE_OPERAND (arg, 0);
+ base = get_base_address (base);
+
+ if (decl_address_ip_invariant_p (base)
+ || !is_global_var (base))
+ {
+ /* If the symbol address is local or
+ constant. */
+ addr_nonzero = true;
+ }
+ else
+ {
+ /* If symbol address is nonzero_address. */
+ node = varpool_node::get (base);
+ if (node && node->nonzero_address ())
+ addr_nonzero = true;
+ }
+ }
Attached patch passes bootstrap and regression testing on x86_64-linu-gnu.
Thanks,
Kugan
>
> 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)
[-- Attachment #2: p.txt --]
[-- Type: text/plain, Size: 2677 bytes --]
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 353b638..affd8b9 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -1670,9 +1670,35 @@ 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))
+ bool addr_nonzero = false;
+
+ if (TREE_CODE (arg) == ADDR_EXPR)
+ {
+ /* See if the AADR_EXPR is nonnull. */
+ varpool_node *node = NULL;
+ tree base = TREE_OPERAND (arg, 0);
+ base = get_base_address (base);
+
+ if (decl_address_ip_invariant_p (base)
+ || !is_global_var (base))
+ {
+ /* If the symbol address is local or
+ constant. */
+ addr_nonzero = true;
+ }
+ else
+ {
+ /* If symbol address is nonzero_address. */
+ node = varpool_node::get (base);
+ if (node && node->nonzero_address ())
+ addr_nonzero = true;
+ }
+ }
+
+ if ((TREE_CODE (arg) == SSA_NAME
+ && param_type
+ && get_ptr_nonnull (arg))
+ || addr_nonzero)
{
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..571798d 100644
--- a/gcc/testsuite/gcc.dg/ipa/vrp5.c
+++ b/gcc/testsuite/gcc.dg/ipa/vrp5.c
@@ -0,0 +1,34 @@
+/* { 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 arr1[10];
+int a;
+int bar (struct st *s)
+{
+ int arr2[10];
+ int b;
+ if (!s)
+ return 0;
+ foo (&s->a);
+ foo (&a);
+ foo (&b);
+ foo (&arr1[1]);
+ foo (&arr2[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..971db44 100644
--- a/gcc/testsuite/gcc.dg/ipa/vrp6.c
+++ b/gcc/testsuite/gcc.dg/ipa/vrp6.c
@@ -0,0 +1,34 @@
+/* { 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;
+};
+
+struct st s2;
+int a;
+int bar (struct st *s)
+{
+ struct st s3;
+ int b;
+ if (!s)
+ return 0;
+ foo (&s->a);
+ foo (&s2.a);
+ foo (&s3.a);
+ foo (&a);
+ foo (&b);
+}
+
+/* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */
+/* { dg-final { scan-tree-dump-times "if" 1 "vrp1" } } */
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-20 3:13 ` kugan
@ 2016-10-20 7:42 ` Richard Biener
2016-10-20 11:56 ` kugan
0 siblings, 1 reply; 14+ messages in thread
From: Richard Biener @ 2016-10-20 7:42 UTC (permalink / raw)
To: kugan; +Cc: Jan Hubicka, gcc-patches
On Thu, 20 Oct 2016, kugan wrote:
>
>
> On 20/10/16 01:26, Jan Hubicka wrote:
> > > > 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()
>
> Thanks for the pointer. Here is an attempt. Does this look OK?
>
> + if (TREE_CODE (arg) == ADDR_EXPR)
> + {
> + /* See if the AADR_EXPR is nonnull. */
> + varpool_node *node = NULL;
> + tree base = TREE_OPERAND (arg, 0);
> + base = get_base_address (base);
> +
> + if (decl_address_ip_invariant_p (base)
> + || !is_global_var (base))
> + {
> + /* If the symbol address is local or
> + constant. */
"constant" doesn't matter. You want
if (TREE_CODE (base) == CONST_DECL
|| TREE_CODE (base) == PARM_DECL
|| TREE_CODE (base) == RESULT_DECL)
addr_nonzero = true;
else if (VAR_P (base))
addr_nonzero = ! is_global_var (base) || (varpool_node::get
(base)->nonzero_address ());
Richard.
> + addr_nonzero = true;
> + }
> + else
> + {
> + /* If symbol address is nonzero_address. */
> + node = varpool_node::get (base);
> + if (node && node->nonzero_address ())
> + addr_nonzero = true;
> + }
> + }
>
> Attached patch passes bootstrap and regression testing on x86_64-linu-gnu.
>
> Thanks,
> Kugan
>
> >
> > 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)
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-20 7:42 ` Richard Biener
@ 2016-10-20 11:56 ` kugan
2016-10-20 12:09 ` Richard Biener
2016-10-20 12:15 ` Jan Hubicka
0 siblings, 2 replies; 14+ messages in thread
From: kugan @ 2016-10-20 11:56 UTC (permalink / raw)
To: Richard Biener; +Cc: Jan Hubicka, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2574 bytes --]
Hi Richard,
On 20/10/16 18:41, Richard Biener wrote:
> On Thu, 20 Oct 2016, kugan wrote:
>
>>
>>
>> On 20/10/16 01:26, Jan Hubicka wrote:
>>>>> 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()
>>
>> Thanks for the pointer. Here is an attempt. Does this look OK?
>>
>> + if (TREE_CODE (arg) == ADDR_EXPR)
>> + {
>> + /* See if the AADR_EXPR is nonnull. */
>> + varpool_node *node = NULL;
>> + tree base = TREE_OPERAND (arg, 0);
>> + base = get_base_address (base);
>> +
>> + if (decl_address_ip_invariant_p (base)
>> + || !is_global_var (base))
>> + {
>> + /* If the symbol address is local or
>> + constant. */
>
> "constant" doesn't matter. You want
>
> if (TREE_CODE (base) == CONST_DECL
> || TREE_CODE (base) == PARM_DECL
> || TREE_CODE (base) == RESULT_DECL)
> addr_nonzero = true;
> else if (VAR_P (base))
> addr_nonzero = ! is_global_var (base) || (varpool_node::get
> (base)->nonzero_address ());
Attached patch does this. Bootstrapped and regression tested on
x86_64-linux-gnu. Is this OK?
Thanks,
Kugan
>
> Richard.
>
>> + addr_nonzero = true;
>> + }
>> + else
>> + {
>> + /* If symbol address is nonzero_address. */
>> + node = varpool_node::get (base);
>> + if (node && node->nonzero_address ())
>> + addr_nonzero = true;
>> + }
>> + }
>>
>> Attached patch passes bootstrap and regression testing on x86_64-linu-gnu.
>>
>> Thanks,
>> Kugan
>>
>>>
>>> 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)
>>
>
[-- Attachment #2: p.txt --]
[-- Type: text/plain, Size: 2535 bytes --]
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 353b638..3fe60d6 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -1670,9 +1670,26 @@ 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))
+ bool addr_nonzero = false;
+
+ if (TREE_CODE (arg) == ADDR_EXPR)
+ {
+ /* See if the ADDR_EXPR is nonnull. */
+ tree base = get_base_address (TREE_OPERAND (arg, 0));
+
+ if (TREE_CODE (base) == CONST_DECL
+ || TREE_CODE (base) == PARM_DECL
+ || TREE_CODE (base) == RESULT_DECL)
+ addr_nonzero = true;
+ else if (VAR_P (base))
+ addr_nonzero = ! is_global_var (base)
+ || (varpool_node::get (base)->nonzero_address ());
+ }
+
+ if ((TREE_CODE (arg) == SSA_NAME
+ && param_type
+ && get_ptr_nonnull (arg))
+ || addr_nonzero)
{
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..571798d 100644
--- a/gcc/testsuite/gcc.dg/ipa/vrp5.c
+++ b/gcc/testsuite/gcc.dg/ipa/vrp5.c
@@ -0,0 +1,34 @@
+/* { 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 arr1[10];
+int a;
+int bar (struct st *s)
+{
+ int arr2[10];
+ int b;
+ if (!s)
+ return 0;
+ foo (&s->a);
+ foo (&a);
+ foo (&b);
+ foo (&arr1[1]);
+ foo (&arr2[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..971db44 100644
--- a/gcc/testsuite/gcc.dg/ipa/vrp6.c
+++ b/gcc/testsuite/gcc.dg/ipa/vrp6.c
@@ -0,0 +1,34 @@
+/* { 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;
+};
+
+struct st s2;
+int a;
+int bar (struct st *s)
+{
+ struct st s3;
+ int b;
+ if (!s)
+ return 0;
+ foo (&s->a);
+ foo (&s2.a);
+ foo (&s3.a);
+ foo (&a);
+ foo (&b);
+}
+
+/* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */
+/* { dg-final { scan-tree-dump-times "if" 1 "vrp1" } } */
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-20 11:56 ` kugan
@ 2016-10-20 12:09 ` Richard Biener
2016-10-20 12:15 ` Jan Hubicka
1 sibling, 0 replies; 14+ messages in thread
From: Richard Biener @ 2016-10-20 12:09 UTC (permalink / raw)
To: kugan; +Cc: Jan Hubicka, gcc-patches
On Thu, 20 Oct 2016, kugan wrote:
> Hi Richard,
>
>
> On 20/10/16 18:41, Richard Biener wrote:
> > On Thu, 20 Oct 2016, kugan wrote:
> >
> > >
> > >
> > > On 20/10/16 01:26, Jan Hubicka wrote:
> > > > > > 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()
> > >
> > > Thanks for the pointer. Here is an attempt. Does this look OK?
> > >
> > > + if (TREE_CODE (arg) == ADDR_EXPR)
> > > + {
> > > + /* See if the AADR_EXPR is nonnull. */
> > > + varpool_node *node = NULL;
> > > + tree base = TREE_OPERAND (arg, 0);
> > > + base = get_base_address (base);
> > > +
> > > + if (decl_address_ip_invariant_p (base)
> > > + || !is_global_var (base))
> > > + {
> > > + /* If the symbol address is local or
> > > + constant. */
> >
> > "constant" doesn't matter. You want
> >
> > if (TREE_CODE (base) == CONST_DECL
> > || TREE_CODE (base) == PARM_DECL
> > || TREE_CODE (base) == RESULT_DECL)
> > addr_nonzero = true;
> > else if (VAR_P (base))
> > addr_nonzero = ! is_global_var (base) || (varpool_node::get
> > (base)->nonzero_address ());
>
> Attached patch does this. Bootstrapped and regression tested on
> x86_64-linux-gnu. Is this OK?
Ok with
+ }
+
+ if ((TREE_CODE (arg) == SSA_NAME
+ && param_type
+ && get_ptr_nonnull (arg))
+ || addr_nonzero)
{
re-factored as
+ }
else if (TREE_CODE (arg) == SSA_NAME
+ && param_type
+ && get_ptr_nonnull (arg))
addr_nonzero = true;
if (addr_nonzero)
Richard.
> Thanks,
> Kugan
>
> >
> > Richard.
> >
> > > + addr_nonzero = true;
> > > + }
> > > + else
> > > + {
> > > + /* If symbol address is nonzero_address. */
> > > + node = varpool_node::get (base);
> > > + if (node && node->nonzero_address ())
> > > + addr_nonzero = true;
> > > + }
> > > + }
> > >
> > > Attached patch passes bootstrap and regression testing on x86_64-linu-gnu.
> > >
> > > Thanks,
> > > Kugan
> > >
> > > >
> > > > 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)
> > >
> >
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-20 11:56 ` kugan
2016-10-20 12:09 ` Richard Biener
@ 2016-10-20 12:15 ` Jan Hubicka
2016-10-21 3:39 ` kugan
1 sibling, 1 reply; 14+ messages in thread
From: Jan Hubicka @ 2016-10-20 12:15 UTC (permalink / raw)
To: kugan; +Cc: Richard Biener, Jan Hubicka, gcc-patches
> Hi Richard,
>
>
> On 20/10/16 18:41, Richard Biener wrote:
> >On Thu, 20 Oct 2016, kugan wrote:
> >
> >>
> >>
> >>On 20/10/16 01:26, Jan Hubicka wrote:
> >>>>>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()
> >>
> >>Thanks for the pointer. Here is an attempt. Does this look OK?
> >>
> >>+ if (TREE_CODE (arg) == ADDR_EXPR)
> >>+ {
> >>+ /* See if the AADR_EXPR is nonnull. */
> >>+ varpool_node *node = NULL;
> >>+ tree base = TREE_OPERAND (arg, 0);
> >>+ base = get_base_address (base);
> >>+
> >>+ if (decl_address_ip_invariant_p (base)
> >>+ || !is_global_var (base))
> >>+ {
> >>+ /* If the symbol address is local or
> >>+ constant. */
> >
> >"constant" doesn't matter. You want
> >
> > if (TREE_CODE (base) == CONST_DECL
> >|| TREE_CODE (base) == PARM_DECL
> >|| TREE_CODE (base) == RESULT_DECL)
> > addr_nonzero = true;
> > else if (VAR_P (base))
Better to check decl_in_symtab_p (decl)
> > addr_nonzero = ! is_global_var (base) || (varpool_node::get
> >(base)->nonzero_address ());
and symtab_node::get.
I wonder if we can't unify the logic with tree_expr_nonzero_warnv_p
and corresponding vrp code?
Otherwise the patch looks fine to me.
Honza
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-20 12:15 ` Jan Hubicka
@ 2016-10-21 3:39 ` kugan
2016-10-21 7:16 ` Richard Biener
0 siblings, 1 reply; 14+ messages in thread
From: kugan @ 2016-10-21 3:39 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Richard Biener, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1750 bytes --]
Hi,
On 20/10/16 23:15, Jan Hubicka wrote:
>> Hi Richard,
>>
>>
>> On 20/10/16 18:41, Richard Biener wrote:
>>> On Thu, 20 Oct 2016, kugan wrote:
>>>
>>>>
>>>>
>>>> On 20/10/16 01:26, Jan Hubicka wrote:
>>>>>>> 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()
>>>>
>>>> Thanks for the pointer. Here is an attempt. Does this look OK?
>>>>
>>>> + if (TREE_CODE (arg) == ADDR_EXPR)
>>>> + {
>>>> + /* See if the AADR_EXPR is nonnull. */
>>>> + varpool_node *node = NULL;
>>>> + tree base = TREE_OPERAND (arg, 0);
>>>> + base = get_base_address (base);
>>>> +
>>>> + if (decl_address_ip_invariant_p (base)
>>>> + || !is_global_var (base))
>>>> + {
>>>> + /* If the symbol address is local or
>>>> + constant. */
>>>
>>> "constant" doesn't matter. You want
>>>
>>> if (TREE_CODE (base) == CONST_DECL
>>> || TREE_CODE (base) == PARM_DECL
>>> || TREE_CODE (base) == RESULT_DECL)
>>> addr_nonzero = true;
>>> else if (VAR_P (base))
> Better to check decl_in_symtab_p (decl)
>>> addr_nonzero = ! is_global_var (base) || (varpool_node::get
>>> (base)->nonzero_address ());
> and symtab_node::get.
>
> I wonder if we can't unify the logic with tree_expr_nonzero_warnv_p
> and corresponding vrp code?
Are you saying that we should export tree_expr_nonzero_warnv_p and use
here with the logic from above added to tree_expr_nonzero_warnv_p as
shown in the attached patch?
Thanks,
Kugan
>
> Otherwise the patch looks fine to me.
> Honza
>
[-- Attachment #2: p2.txt --]
[-- Type: text/plain, Size: 3638 bytes --]
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 89ed89d..6fa42c9 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -8954,7 +8954,7 @@ mask_with_tz (tree type, const wide_int &x, const wide_int &y)
is undefined, set *STRICT_OVERFLOW_P to true; otherwise, don't
change *STRICT_OVERFLOW_P. */
-static bool
+bool
tree_expr_nonzero_warnv_p (tree t, bool *strict_overflow_p)
{
tree type = TREE_TYPE (t);
@@ -13402,9 +13402,15 @@ tree_single_nonzero_warnv_p (tree t, bool *strict_overflow_p)
return true;
/* Constants are never weak. */
- if (CONSTANT_CLASS_P (base))
+ if (CONSTANT_CLASS_P (base)
+ || TREE_CODE (base) == PARM_DECL
+ || TREE_CODE (base) == RESULT_DECL)
return true;
+ if (VAR_P (base)
+ && (!is_global_var (base)
+ || varpool_node::get (base)->nonzero_address ()))
+ return true;
return false;
}
diff --git a/gcc/fold-const.h b/gcc/fold-const.h
index bc22c88..a19a27a 100644
--- a/gcc/fold-const.h
+++ b/gcc/fold-const.h
@@ -198,6 +198,7 @@ extern tree fold_build_pointer_plus_loc (location_t loc, tree ptr, tree off);
/* Build and fold a POINTER_PLUS_EXPR at LOC offsetting PTR by OFF. */
extern tree fold_build_pointer_plus_hwi_loc (location_t loc, tree ptr, HOST_WIDE_INT off);
+extern bool tree_expr_nonzero_warnv_p (tree t, bool *strict_overflow_p);
#define fold_build_pointer_plus_hwi(p,o) \
fold_build_pointer_plus_hwi_loc (UNKNOWN_LOCATION, p, o)
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 353b638..55a2ea1 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -1670,9 +1670,20 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
if (POINTER_TYPE_P (TREE_TYPE (arg)))
{
+ bool addr_nonzero = false;
+ bool strict_overflow = false;
+
if (TREE_CODE (arg) == SSA_NAME
&& param_type
&& get_ptr_nonnull (arg))
+ addr_nonzero = true;
+ else if (tree_expr_nonzero_warnv_p (arg, &strict_overflow))
+ {
+ if (!strict_overflow)
+ addr_nonzero = true;
+ }
+
+ if (addr_nonzero)
{
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..571798d 100644
--- a/gcc/testsuite/gcc.dg/ipa/vrp5.c
+++ b/gcc/testsuite/gcc.dg/ipa/vrp5.c
@@ -0,0 +1,34 @@
+/* { 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 arr1[10];
+int a;
+int bar (struct st *s)
+{
+ int arr2[10];
+ int b;
+ if (!s)
+ return 0;
+ foo (&s->a);
+ foo (&a);
+ foo (&b);
+ foo (&arr1[1]);
+ foo (&arr2[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..971db44 100644
--- a/gcc/testsuite/gcc.dg/ipa/vrp6.c
+++ b/gcc/testsuite/gcc.dg/ipa/vrp6.c
@@ -0,0 +1,34 @@
+/* { 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;
+};
+
+struct st s2;
+int a;
+int bar (struct st *s)
+{
+ struct st s3;
+ int b;
+ if (!s)
+ return 0;
+ foo (&s->a);
+ foo (&s2.a);
+ foo (&s3.a);
+ foo (&a);
+ foo (&b);
+}
+
+/* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */
+/* { dg-final { scan-tree-dump-times "if" 1 "vrp1" } } */
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-21 3:39 ` kugan
@ 2016-10-21 7:16 ` Richard Biener
2016-10-21 7:38 ` kugan
0 siblings, 1 reply; 14+ messages in thread
From: Richard Biener @ 2016-10-21 7:16 UTC (permalink / raw)
To: kugan; +Cc: Jan Hubicka, gcc-patches
On Fri, 21 Oct 2016, kugan wrote:
> Hi,
>
> On 20/10/16 23:15, Jan Hubicka wrote:
> > > Hi Richard,
> > >
> > >
> > > On 20/10/16 18:41, Richard Biener wrote:
> > > > On Thu, 20 Oct 2016, kugan wrote:
> > > >
> > > > >
> > > > >
> > > > > On 20/10/16 01:26, Jan Hubicka wrote:
> > > > > > > > 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()
> > > > >
> > > > > Thanks for the pointer. Here is an attempt. Does this look OK?
> > > > >
> > > > > + if (TREE_CODE (arg) == ADDR_EXPR)
> > > > > + {
> > > > > + /* See if the AADR_EXPR is nonnull. */
> > > > > + varpool_node *node = NULL;
> > > > > + tree base = TREE_OPERAND (arg, 0);
> > > > > + base = get_base_address (base);
> > > > > +
> > > > > + if (decl_address_ip_invariant_p (base)
> > > > > + || !is_global_var (base))
> > > > > + {
> > > > > + /* If the symbol address is local or
> > > > > + constant. */
> > > >
> > > > "constant" doesn't matter. You want
> > > >
> > > > if (TREE_CODE (base) == CONST_DECL
> > > > || TREE_CODE (base) == PARM_DECL
> > > > || TREE_CODE (base) == RESULT_DECL)
> > > > addr_nonzero = true;
> > > > else if (VAR_P (base))
> > Better to check decl_in_symtab_p (decl)
> > > > addr_nonzero = ! is_global_var (base) || (varpool_node::get
> > > > (base)->nonzero_address ());
> > and symtab_node::get.
> >
> > I wonder if we can't unify the logic with tree_expr_nonzero_warnv_p
> > and corresponding vrp code?
> Are you saying that we should export tree_expr_nonzero_warnv_p and use here
> with the logic from above added to tree_expr_nonzero_warnv_p as shown in the
> attached patch?
tree_single_nonzero_warnv_p already handles this correctly via
maybe_nonzero_address. And tree_single_nonzero_warnv_p is already
exported.
Richard.
> Thanks,
> Kugan
>
> >
> > Otherwise the patch looks fine to me.
> > Honza
> >
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-21 7:16 ` Richard Biener
@ 2016-10-21 7:38 ` kugan
2016-10-21 8:08 ` Richard Biener
0 siblings, 1 reply; 14+ messages in thread
From: kugan @ 2016-10-21 7:38 UTC (permalink / raw)
To: Richard Biener; +Cc: Jan Hubicka, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2376 bytes --]
On 21/10/16 18:16, Richard Biener wrote:
> On Fri, 21 Oct 2016, kugan wrote:
>
>> Hi,
>>
>> On 20/10/16 23:15, Jan Hubicka wrote:
>>>> Hi Richard,
>>>>
>>>>
>>>> On 20/10/16 18:41, Richard Biener wrote:
>>>>> On Thu, 20 Oct 2016, kugan wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On 20/10/16 01:26, Jan Hubicka wrote:
>>>>>>>>> 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()
>>>>>>
>>>>>> Thanks for the pointer. Here is an attempt. Does this look OK?
>>>>>>
>>>>>> + if (TREE_CODE (arg) == ADDR_EXPR)
>>>>>> + {
>>>>>> + /* See if the AADR_EXPR is nonnull. */
>>>>>> + varpool_node *node = NULL;
>>>>>> + tree base = TREE_OPERAND (arg, 0);
>>>>>> + base = get_base_address (base);
>>>>>> +
>>>>>> + if (decl_address_ip_invariant_p (base)
>>>>>> + || !is_global_var (base))
>>>>>> + {
>>>>>> + /* If the symbol address is local or
>>>>>> + constant. */
>>>>>
>>>>> "constant" doesn't matter. You want
>>>>>
>>>>> if (TREE_CODE (base) == CONST_DECL
>>>>> || TREE_CODE (base) == PARM_DECL
>>>>> || TREE_CODE (base) == RESULT_DECL)
>>>>> addr_nonzero = true;
>>>>> else if (VAR_P (base))
>>> Better to check decl_in_symtab_p (decl)
>>>>> addr_nonzero = ! is_global_var (base) || (varpool_node::get
>>>>> (base)->nonzero_address ());
>>> and symtab_node::get.
>>>
>>> I wonder if we can't unify the logic with tree_expr_nonzero_warnv_p
>>> and corresponding vrp code?
>> Are you saying that we should export tree_expr_nonzero_warnv_p and use here
>> with the logic from above added to tree_expr_nonzero_warnv_p as shown in the
>> attached patch?
>
> tree_single_nonzero_warnv_p already handles this correctly via
> maybe_nonzero_address. And tree_single_nonzero_warnv_p is already
> exported.
Indeed. maybe_nonzero_address does look at node->nonzero_address () but
we still seems to miss TREE_CODE (base) == PARM_DECL and TREE_CODE
(base) == RESULT_DECL.
Does the attached patch looks OK if no regressions?
Thanks,
Kugan
>
> Richard.
>
>> Thanks,
>> Kugan
>>
>>>
>>> Otherwise the patch looks fine to me.
>>> Honza
>>>
>>
>
[-- Attachment #2: p3.txt --]
[-- Type: text/plain, Size: 2636 bytes --]
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 89ed89d..3c1803f 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -13402,7 +13402,9 @@ tree_single_nonzero_warnv_p (tree t, bool *strict_overflow_p)
return true;
/* Constants are never weak. */
- if (CONSTANT_CLASS_P (base))
+ if (CONSTANT_CLASS_P (base)
+ || TREE_CODE (base) == PARM_DECL
+ || TREE_CODE (base) == RESULT_DECL)
return true;
return false;
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 353b638..939fd1d 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -1670,9 +1670,20 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
if (POINTER_TYPE_P (TREE_TYPE (arg)))
{
+ bool addr_nonzero = false;
+ bool strict_overflow = false;
+
if (TREE_CODE (arg) == SSA_NAME
&& param_type
&& get_ptr_nonnull (arg))
+ addr_nonzero = true;
+ else if (tree_single_nonzero_warnv_p (arg, &strict_overflow))
+ {
+ if (!strict_overflow)
+ addr_nonzero = true;
+ }
+
+ if (addr_nonzero)
{
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..571798d 100644
--- a/gcc/testsuite/gcc.dg/ipa/vrp5.c
+++ b/gcc/testsuite/gcc.dg/ipa/vrp5.c
@@ -0,0 +1,34 @@
+/* { 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 arr1[10];
+int a;
+int bar (struct st *s)
+{
+ int arr2[10];
+ int b;
+ if (!s)
+ return 0;
+ foo (&s->a);
+ foo (&a);
+ foo (&b);
+ foo (&arr1[1]);
+ foo (&arr2[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..971db44 100644
--- a/gcc/testsuite/gcc.dg/ipa/vrp6.c
+++ b/gcc/testsuite/gcc.dg/ipa/vrp6.c
@@ -0,0 +1,34 @@
+/* { 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;
+};
+
+struct st s2;
+int a;
+int bar (struct st *s)
+{
+ struct st s3;
+ int b;
+ if (!s)
+ return 0;
+ foo (&s->a);
+ foo (&s2.a);
+ foo (&s3.a);
+ foo (&a);
+ foo (&b);
+}
+
+/* { dg-final { scan-ipa-dump "Setting nonnull for 0" "cp" } } */
+/* { dg-final { scan-tree-dump-times "if" 1 "vrp1" } } */
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC][IPA-VRP] ADDR_EXPR and nonnull
2016-10-21 7:38 ` kugan
@ 2016-10-21 8:08 ` Richard Biener
0 siblings, 0 replies; 14+ messages in thread
From: Richard Biener @ 2016-10-21 8:08 UTC (permalink / raw)
To: kugan; +Cc: Jan Hubicka, gcc-patches
On Fri, 21 Oct 2016, kugan wrote:
>
>
> On 21/10/16 18:16, Richard Biener wrote:
> > On Fri, 21 Oct 2016, kugan wrote:
> >
> > > Hi,
> > >
> > > On 20/10/16 23:15, Jan Hubicka wrote:
> > > > > Hi Richard,
> > > > >
> > > > >
> > > > > On 20/10/16 18:41, Richard Biener wrote:
> > > > > > On Thu, 20 Oct 2016, kugan wrote:
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On 20/10/16 01:26, Jan Hubicka wrote:
> > > > > > > > > > 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()
> > > > > > >
> > > > > > > Thanks for the pointer. Here is an attempt. Does this look OK?
> > > > > > >
> > > > > > > + if (TREE_CODE (arg) == ADDR_EXPR)
> > > > > > > + {
> > > > > > > + /* See if the AADR_EXPR is nonnull. */
> > > > > > > + varpool_node *node = NULL;
> > > > > > > + tree base = TREE_OPERAND (arg, 0);
> > > > > > > + base = get_base_address (base);
> > > > > > > +
> > > > > > > + if (decl_address_ip_invariant_p (base)
> > > > > > > + || !is_global_var (base))
> > > > > > > + {
> > > > > > > + /* If the symbol address is local or
> > > > > > > + constant. */
> > > > > >
> > > > > > "constant" doesn't matter. You want
> > > > > >
> > > > > > if (TREE_CODE (base) == CONST_DECL
> > > > > > || TREE_CODE (base) == PARM_DECL
> > > > > > || TREE_CODE (base) == RESULT_DECL)
> > > > > > addr_nonzero = true;
> > > > > > else if (VAR_P (base))
> > > > Better to check decl_in_symtab_p (decl)
> > > > > > addr_nonzero = ! is_global_var (base) || (varpool_node::get
> > > > > > (base)->nonzero_address ());
> > > > and symtab_node::get.
> > > >
> > > > I wonder if we can't unify the logic with tree_expr_nonzero_warnv_p
> > > > and corresponding vrp code?
> > > Are you saying that we should export tree_expr_nonzero_warnv_p and use
> > > here
> > > with the logic from above added to tree_expr_nonzero_warnv_p as shown in
> > > the
> > > attached patch?
> >
> > tree_single_nonzero_warnv_p already handles this correctly via
> > maybe_nonzero_address. And tree_single_nonzero_warnv_p is already
> > exported.
> Indeed. maybe_nonzero_address does look at node->nonzero_address () but we
> still seems to miss TREE_CODE (base) == PARM_DECL and TREE_CODE (base) ==
> RESULT_DECL.
Should be handled by
/* Function local objects are never NULL. */
if (DECL_P (base)
&& (DECL_CONTEXT (base)
&& TREE_CODE (DECL_CONTEXT (base)) == FUNCTION_DECL
&& auto_var_in_fn_p (base, DECL_CONTEXT (base))))
return true;
> Does the attached patch looks OK if no regressions?
+ else if (tree_single_nonzero_warnv_p (arg, &strict_overflow))
+ {
+ if (!strict_overflow)
+ addr_nonzero = true;
you don't need to care for strict_overflow here.
The patch is ok with that removed and the fold-const.c changes dropped.
Richard.
> Thanks,
> Kugan
>
>
> >
> > Richard.
> >
> > > Thanks,
> > > Kugan
> > >
> > > >
> > > > Otherwise the patch looks fine to me.
> > > > Honza
> > > >
> > >
> >
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-10-21 8:08 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-19 2:12 [RFC][IPA-VRP] ADDR_EXPR and nonnull kugan
2016-10-19 8:24 ` Richard Biener
2016-10-19 8:48 ` kugan
2016-10-19 9:26 ` Richard Biener
2016-10-19 14:26 ` Jan Hubicka
2016-10-20 3:13 ` kugan
2016-10-20 7:42 ` Richard Biener
2016-10-20 11:56 ` kugan
2016-10-20 12:09 ` Richard Biener
2016-10-20 12:15 ` Jan Hubicka
2016-10-21 3:39 ` kugan
2016-10-21 7:16 ` Richard Biener
2016-10-21 7:38 ` kugan
2016-10-21 8:08 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).