| Message ID | CA+=Sn1merkL4qBDb-WO_OAgfSNyXYsq42t1yUBSRd7qY2oP_=w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Andrew Pinski schrieb: > Hi, > While fixing some code not to have aliasing violations in it, I can > across that some builtins were not causing their arguments or their > return values being marked as non-null. This patch implements just > that in VPR while allowing to remove some null pointer checks later > on. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > Thanks, > Andrew Pinski > > ChangeLog: > * tree-vrp.c (vrp_stmt_computes_nonzero): Return true for some > builtins (memcpy and memmove). > (infer_value_range): Infer nonzero for some arguments to > some builtins (memcpy, memmove, strcmp and memcmp). > > testsuite/ChangeLog: > * gcc.dg/tree-ssa/vrp-builtins1.c: New testcase. > > > > Index: tree-vrp.c > =================================================================== > --- tree-vrp.c (revision 190868) > +++ tree-vrp.c (working copy) > @@ -1057,6 +1057,20 @@ vrp_stmt_computes_nonzero (gimple stmt, > } > } > > + /* With some builtins, we can infer if the pointer return value > + will be non null. */ > + if (flag_delete_null_pointer_checks > + && is_gimple_call (stmt) && gimple_call_fndecl (stmt) > + && DECL_BUILT_IN_CLASS (gimple_call_fndecl (stmt)) == BUILT_IN_NORMAL) > + { > + switch (DECL_FUNCTION_CODE (gimple_call_fndecl (stmt))) > + { > + case BUILT_IN_MEMCPY: > + case BUILT_IN_MEMMOVE: > + return true; > + } > + } > + > return false; > } > > @@ -4231,6 +4245,32 @@ infer_value_range (gimple stmt, tree op, > } > } > > + /* With some builtins, we can infer if the pointer argument > + will be non null. */ > + if (flag_delete_null_pointer_checks > + && is_gimple_call (stmt) && gimple_call_fndecl (stmt)) > + { > + tree callee = gimple_call_fndecl (stmt); > + if (DECL_BUILT_IN_CLASS (callee) == BUILT_IN_NORMAL) > + { > + switch (DECL_FUNCTION_CODE (callee)) > + { > + case BUILT_IN_MEMCPY: > + case BUILT_IN_MEMMOVE: > + case BUILT_IN_STRCMP: > + case BUILT_IN_MEMCMP: > + /* The first and second arguments of memcpy and memmove will be non null after the call. */ > + if (gimple_call_arg (stmt, 0) == op > + || gimple_call_arg (stmt, 1) == op) > + { > + *val_p = build_int_cst (TREE_TYPE (op), 0); > + *comp_code_p = NE_EXPR; > + return true; > + } > + } > + } > + } > + > return false; > } > > Index: testsuite/gcc.dg/tree-ssa/vrp-builtins1.c > =================================================================== > --- testsuite/gcc.dg/tree-ssa/vrp-builtins1.c (revision 0) > +++ testsuite/gcc.dg/tree-ssa/vrp-builtins1.c (revision 0) > @@ -0,0 +1,30 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-vrp1" } */ Shouldn't this xfail for avr and other targets that set flag_delete_null_pointer_checks = 0, cf. avr.c:avr_option_override() for example. Johann > + > +struct f1 > +{ > +char a[4]; > +}; > + > +int f(int *a, struct f1 b) > +{ > + int *c = __builtin_memcpy(a, b.a, 4); > + if (c == 0) > + return 0; > + return *a; > +} > + > + > +int f1(int *a, struct f1 b) > +{ > + int *c = __builtin_memcpy(a, b.a, 4); > + if (a == 0) > + return 0; > + return *a; > +} > + > +/* Both the if statements should be folded when the target does not keep around null pointer checks. */ > +/* { dg-final { scan-tree-dump-times "Folding predicate" 0 "vrp1" { target { keeps_null_pointer_checks } } } } */ > +/* { dg-final { scan-tree-dump-times "Folding predicate" 2 "vrp1" { target { ! keeps_null_pointer_checks } } } } */ > +/* { dg-final { cleanup-tree-dump "vrp1" } } */ > +
On Sun, Sep 02, 2012 at 10:18:15PM -0700, Andrew Pinski wrote: > While fixing some code not to have aliasing violations in it, I can > across that some builtins were not causing their arguments or their > return values being marked as non-null. This patch implements just > that in VPR while allowing to remove some null pointer checks later > on. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. > @@ -1057,6 +1057,20 @@ vrp_stmt_computes_nonzero (gimple stmt, > } > } > > + /* With some builtins, we can infer if the pointer return value > + will be non null. */ > + if (flag_delete_null_pointer_checks > + && is_gimple_call (stmt) && gimple_call_fndecl (stmt) > + && DECL_BUILT_IN_CLASS (gimple_call_fndecl (stmt)) == BUILT_IN_NORMAL) > + { > + switch (DECL_FUNCTION_CODE (gimple_call_fndecl (stmt))) > + { > + case BUILT_IN_MEMCPY: > + case BUILT_IN_MEMMOVE: > + return true; > + } > + } > + > return false; > } > That is too hackish and lists way too few builtins. If you rely on nonnull attribute marked builtins, I'd say you want flags = gimple_call_return_flags (stmt); if ((flags & ERF_RETURNS_ARG) && (flags & ERF_RETURN_ARG_MASK) < gimple_call_num_args (stmt)) { /* Test nonnull attribute on the decl, either argument-less or on the (flags & ERF_RETURN_ARG_MASK)th argument. */ } Or at least handle builtins e.g. CCP handles as pass-thru arg1: BUILT_IN_MEMCPY, BUILT_IN_MEMMOVE, BUILT_IN_MEMSET, BUILT_IN_STRCPY, BUILT_IN_STRNCPY, BUILT_IN_MEMCPY_CHK, BUILT_IN_MEMMOVE_CHK, BUILT_IN_MEMSET_CHK, BUILT_IN_STRCPY_CHK, BUILT_IN_STRNCPY_CHK (which reminds me that some of these apparently aren't marked with ATTR_RET1_NOTHROW_NONNULL_LEAF, why?). > @@ -4231,6 +4245,32 @@ infer_value_range (gimple stmt, tree op, > } > } > > + /* With some builtins, we can infer if the pointer argument > + will be non null. */ > + if (flag_delete_null_pointer_checks > + && is_gimple_call (stmt) && gimple_call_fndecl (stmt)) > + { > + tree callee = gimple_call_fndecl (stmt); > + if (DECL_BUILT_IN_CLASS (callee) == BUILT_IN_NORMAL) > + { > + switch (DECL_FUNCTION_CODE (callee)) > + { > + case BUILT_IN_MEMCPY: > + case BUILT_IN_MEMMOVE: > + case BUILT_IN_STRCMP: > + case BUILT_IN_MEMCMP: > + /* The first and second arguments of memcpy and memmove will be non null after the call. */ > + if (gimple_call_arg (stmt, 0) == op > + || gimple_call_arg (stmt, 1) == op) > + { > + *val_p = build_int_cst (TREE_TYPE (op), 0); > + *comp_code_p = NE_EXPR; > + return true; > + } > + } > + } > + } Again, what you are looking for here? Passing pointers to nonnull attributes, or something more specific? What exactly? There are tons of builtins that behave similarly to memcpy/memmove/strcmp/memcmp. Jakub
On Sun, Sep 2, 2012 at 11:36 PM, Georg-Johann Lay <gjl@gcc.gnu.org> wrote: > Andrew Pinski schrieb: >> >> Hi, >> While fixing some code not to have aliasing violations in it, I can >> across that some builtins were not causing their arguments or their >> return values being marked as non-null. This patch implements just >> that in VPR while allowing to remove some null pointer checks later >> on. >> >> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. >> >> Thanks, >> Andrew Pinski >> >> ChangeLog: >> * tree-vrp.c (vrp_stmt_computes_nonzero): Return true for some >> builtins (memcpy and memmove). >> (infer_value_range): Infer nonzero for some arguments to >> some builtins (memcpy, memmove, strcmp and memcmp). >> >> testsuite/ChangeLog: >> * gcc.dg/tree-ssa/vrp-builtins1.c: New testcase. >> >> >> >> Index: tree-vrp.c >> =================================================================== >> --- tree-vrp.c (revision 190868) >> +++ tree-vrp.c (working copy) >> @@ -1057,6 +1057,20 @@ vrp_stmt_computes_nonzero (gimple stmt, >> } >> } >> + /* With some builtins, we can infer if the pointer return value >> + will be non null. */ >> + if (flag_delete_null_pointer_checks >> + && is_gimple_call (stmt) && gimple_call_fndecl (stmt) >> + && DECL_BUILT_IN_CLASS (gimple_call_fndecl (stmt)) == >> BUILT_IN_NORMAL) >> + { >> + switch (DECL_FUNCTION_CODE (gimple_call_fndecl (stmt))) >> + { >> + case BUILT_IN_MEMCPY: >> + case BUILT_IN_MEMMOVE: >> + return true; >> + } >> + } >> + >> return false; >> } >> @@ -4231,6 +4245,32 @@ infer_value_range (gimple stmt, tree op, >> } >> } >> + /* With some builtins, we can infer if the pointer argument >> + will be non null. */ >> + if (flag_delete_null_pointer_checks >> + && is_gimple_call (stmt) && gimple_call_fndecl (stmt)) >> + { >> + tree callee = gimple_call_fndecl (stmt); >> + if (DECL_BUILT_IN_CLASS (callee) == BUILT_IN_NORMAL) >> + { >> + switch (DECL_FUNCTION_CODE (callee)) >> + { >> + case BUILT_IN_MEMCPY: >> + case BUILT_IN_MEMMOVE: >> + case BUILT_IN_STRCMP: >> + case BUILT_IN_MEMCMP: >> + /* The first and second arguments of memcpy and memmove >> will be non null after the call. */ >> + if (gimple_call_arg (stmt, 0) == op >> + || gimple_call_arg (stmt, 1) == op) >> + { >> + *val_p = build_int_cst (TREE_TYPE (op), 0); >> + *comp_code_p = NE_EXPR; >> + return true; >> + } >> + } >> + } >> + } >> + >> return false; >> } >> Index: testsuite/gcc.dg/tree-ssa/vrp-builtins1.c >> =================================================================== >> --- testsuite/gcc.dg/tree-ssa/vrp-builtins1.c (revision 0) >> +++ testsuite/gcc.dg/tree-ssa/vrp-builtins1.c (revision 0) >> @@ -0,0 +1,30 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2 -fdump-tree-vrp1" } */ > > > Shouldn't this xfail for avr and other targets that set > flag_delete_null_pointer_checks = 0, cf. > avr.c:avr_option_override() for example. This is taken care of by the check keeps_null_pointer_checks which returns true for avr-*-*. Thanks, Andrew Pinski > > Johann > >> + >> +struct f1 >> +{ >> +char a[4]; >> +}; >> + >> +int f(int *a, struct f1 b) >> +{ >> + int *c = __builtin_memcpy(a, b.a, 4); >> + if (c == 0) >> + return 0; >> + return *a; >> +} >> + >> + >> +int f1(int *a, struct f1 b) >> +{ >> + int *c = __builtin_memcpy(a, b.a, 4); >> + if (a == 0) >> + return 0; >> + return *a; >> +} >> + >> +/* Both the if statements should be folded when the target does not keep >> around null pointer checks. */ >> +/* { dg-final { scan-tree-dump-times "Folding predicate" 0 "vrp1" { >> target { keeps_null_pointer_checks } } } } */ >> +/* { dg-final { scan-tree-dump-times "Folding predicate" 2 "vrp1" { >> target { ! keeps_null_pointer_checks } } } } */ >> +/* { dg-final { cleanup-tree-dump "vrp1" } } */ >> + > >
On Mon, Sep 3, 2012 at 12:03 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Sun, Sep 02, 2012 at 10:18:15PM -0700, Andrew Pinski wrote: >> While fixing some code not to have aliasing violations in it, I can >> across that some builtins were not causing their arguments or their >> return values being marked as non-null. This patch implements just >> that in VPR while allowing to remove some null pointer checks later >> on. >> >> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. >> @@ -1057,6 +1057,20 @@ vrp_stmt_computes_nonzero (gimple stmt, >> } >> } >> >> + /* With some builtins, we can infer if the pointer return value >> + will be non null. */ >> + if (flag_delete_null_pointer_checks >> + && is_gimple_call (stmt) && gimple_call_fndecl (stmt) >> + && DECL_BUILT_IN_CLASS (gimple_call_fndecl (stmt)) == BUILT_IN_NORMAL) >> + { >> + switch (DECL_FUNCTION_CODE (gimple_call_fndecl (stmt))) >> + { >> + case BUILT_IN_MEMCPY: >> + case BUILT_IN_MEMMOVE: >> + return true; >> + } >> + } >> + >> return false; >> } >> > > That is too hackish and lists way too few builtins. > If you rely on nonnull attribute marked builtins, I'd say you want > flags = gimple_call_return_flags (stmt); > if ((flags & ERF_RETURNS_ARG) > && (flags & ERF_RETURN_ARG_MASK) < gimple_call_num_args (stmt)) > { > /* Test nonnull attribute on the decl, either argument-less or > on the (flags & ERF_RETURN_ARG_MASK)th argument. */ > } > Or at least handle builtins e.g. CCP handles as pass-thru arg1: > BUILT_IN_MEMCPY, BUILT_IN_MEMMOVE, BUILT_IN_MEMSET, BUILT_IN_STRCPY, > BUILT_IN_STRNCPY, BUILT_IN_MEMCPY_CHK, BUILT_IN_MEMMOVE_CHK, > BUILT_IN_MEMSET_CHK, BUILT_IN_STRCPY_CHK, BUILT_IN_STRNCPY_CHK > (which reminds me that some of these apparently aren't marked > with ATTR_RET1_NOTHROW_NONNULL_LEAF, why?). I did not know of these attributes and flags on the decl. Yes I know it only lists only a few of the builtins. I will look into those flags and see if I can improve it. > >> @@ -4231,6 +4245,32 @@ infer_value_range (gimple stmt, tree op, >> } >> } >> >> + /* With some builtins, we can infer if the pointer argument >> + will be non null. */ >> + if (flag_delete_null_pointer_checks >> + && is_gimple_call (stmt) && gimple_call_fndecl (stmt)) >> + { >> + tree callee = gimple_call_fndecl (stmt); >> + if (DECL_BUILT_IN_CLASS (callee) == BUILT_IN_NORMAL) >> + { >> + switch (DECL_FUNCTION_CODE (callee)) >> + { >> + case BUILT_IN_MEMCPY: >> + case BUILT_IN_MEMMOVE: >> + case BUILT_IN_STRCMP: >> + case BUILT_IN_MEMCMP: >> + /* The first and second arguments of memcpy and memmove will be non null after the call. */ >> + if (gimple_call_arg (stmt, 0) == op >> + || gimple_call_arg (stmt, 1) == op) >> + { >> + *val_p = build_int_cst (TREE_TYPE (op), 0); >> + *comp_code_p = NE_EXPR; >> + return true; >> + } >> + } >> + } >> + } > > Again, what you are looking for here? Passing pointers to nonnull > attributes, or something more specific? What exactly? There are tons of > builtins that behave similarly to memcpy/memmove/strcmp/memcmp. Passing pointers arguments to nonnull attributed functions. I will see if I can use the nonnull attribute on those functions so that this can be done without checking the builtin functions. Thanks, Andrew Pinski > > Jakub
Index: tree-vrp.c =================================================================== --- tree-vrp.c (revision 190868) +++ tree-vrp.c (working copy) @@ -1057,6 +1057,20 @@ vrp_stmt_computes_nonzero (gimple stmt, } } + /* With some builtins, we can infer if the pointer return value + will be non null. */ + if (flag_delete_null_pointer_checks + && is_gimple_call (stmt) && gimple_call_fndecl (stmt) + && DECL_BUILT_IN_CLASS (gimple_call_fndecl (stmt)) == BUILT_IN_NORMAL) + { + switch (DECL_FUNCTION_CODE (gimple_call_fndecl (stmt))) + { + case BUILT_IN_MEMCPY: + case BUILT_IN_MEMMOVE: + return true; + } + } + return false; } @@ -4231,6 +4245,32 @@ infer_value_range (gimple stmt, tree op, } } + /* With some builtins, we can infer if the pointer argument + will be non null. */ + if (flag_delete_null_pointer_checks + && is_gimple_call (stmt) && gimple_call_fndecl (stmt)) + { + tree callee = gimple_call_fndecl (stmt); + if (DECL_BUILT_IN_CLASS (callee) == BUILT_IN_NORMAL) + { + switch (DECL_FUNCTION_CODE (callee)) + { + case BUILT_IN_MEMCPY: + case BUILT_IN_MEMMOVE: + case BUILT_IN_STRCMP: + case BUILT_IN_MEMCMP: + /* The first and second arguments of memcpy and memmove will be non null after the call. */ + if (gimple_call_arg (stmt, 0) == op + || gimple_call_arg (stmt, 1) == op) + { + *val_p = build_int_cst (TREE_TYPE (op), 0); + *comp_code_p = NE_EXPR; + return true; + } + } + } + } + return false; } Index: testsuite/gcc.dg/tree-ssa/vrp-builtins1.c =================================================================== --- testsuite/gcc.dg/tree-ssa/vrp-builtins1.c (revision 0) +++ testsuite/gcc.dg/tree-ssa/vrp-builtins1.c (revision 0) @@ -0,0 +1,30 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-vrp1" } */ + +struct f1 +{ +char a[4]; +}; + +int f(int *a, struct f1 b) +{ + int *c = __builtin_memcpy(a, b.a, 4); + if (c == 0) + return 0; + return *a; +} + + +int f1(int *a, struct f1 b) +{ + int *c = __builtin_memcpy(a, b.a, 4); + if (a == 0) + return 0; + return *a; +} + +/* Both the if statements should be folded when the target does not keep around null pointer checks. */ +/* { dg-final { scan-tree-dump-times "Folding predicate" 0 "vrp1" { target { keeps_null_pointer_checks } } } } */ +/* { dg-final { scan-tree-dump-times "Folding predicate" 2 "vrp1" { target { ! keeps_null_pointer_checks } } } } */ +/* { dg-final { cleanup-tree-dump "vrp1" } } */ +
Hi, While fixing some code not to have aliasing violations in it, I can across that some builtins were not causing their arguments or their return values being marked as non-null. This patch implements just that in VPR while allowing to remove some null pointer checks later on. OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. Thanks, Andrew Pinski ChangeLog: * tree-vrp.c (vrp_stmt_computes_nonzero): Return true for some builtins (memcpy and memmove). (infer_value_range): Infer nonzero for some arguments to some builtins (memcpy, memmove, strcmp and memcmp). testsuite/ChangeLog: * gcc.dg/tree-ssa/vrp-builtins1.c: New testcase.