| Message ID | 76ab5854-4c4e-ae4a-8d33-e2b1bdd02522@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
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)
> 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
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" } } */