Message ID | edc65ca7-a71f-2fc0-1455-e49f357c7cfb@linaro.org |
---|---|
State | Superseded |
Headers | show |
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)
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. >> >
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)
> > 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 --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" } } */