| Message ID | 0ef084d3-b9fa-4623-d353-c14a16c0b907@linaro.org |
|---|---|
| State | New |
| Headers | show |
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 > > > > > > > > > >
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" } } */