* [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).