| Message ID | alpine.DEB.2.10.1310071517260.21427@stedding.saclay.inria.fr |
|---|---|
| State | New |
| Headers | show |
On 10/07/13 07:52, Marc Glisse wrote: > On Mon, 7 Oct 2013, Richard Biener wrote: > >> On Mon, Oct 7, 2013 at 12:33 AM, Marc Glisse <marc.glisse@inria.fr> >> wrote: >>> Hello, >>> >>> this patch asserts that when we call a function with the nonnull >>> attribute, >>> the corresponding argument is not zero, just like when we dereference a >>> pointer. Everything is under a check for >>> flag_delete_null_pointer_checks. >>> >>> Note that this function currently gives up if the statement may throw >>> (because in some languages indirections may throw?), but this could >>> probably >>> be relaxed a bit so my example would still work when compiled with g++, >>> without having to mark f1 and f2 as throw(). >>> >>> Bootstrap (default languages) + testsuite on x86_64-unknown-linux-gnu. >> >> Can you please restructure it in a way to not require a goto? That is, >> move searching for a non-null opportunity into a helper function? > > Thanks. I wasn't sure what to put in the helper and what to keep in the > original function, so I'll wait a bit for comments before the commit. > > Bootstrap (default languages) + testsuite on x86_64-unknown-linux-gnu. I like it. Tweak per Richi's suggestion to avoid the goto and get it committed! jeff
On Mon, Oct 7, 2013 at 3:52 PM, Marc Glisse <marc.glisse@inria.fr> wrote: > On Mon, 7 Oct 2013, Richard Biener wrote: > >> On Mon, Oct 7, 2013 at 12:33 AM, Marc Glisse <marc.glisse@inria.fr> wrote: >>> >>> Hello, >>> >>> this patch asserts that when we call a function with the nonnull >>> attribute, >>> the corresponding argument is not zero, just like when we dereference a >>> pointer. Everything is under a check for flag_delete_null_pointer_checks. >>> >>> Note that this function currently gives up if the statement may throw >>> (because in some languages indirections may throw?), but this could >>> probably >>> be relaxed a bit so my example would still work when compiled with g++, >>> without having to mark f1 and f2 as throw(). >>> >>> Bootstrap (default languages) + testsuite on x86_64-unknown-linux-gnu. >> >> >> Can you please restructure it in a way to not require a goto? That is, >> move searching for a non-null opportunity into a helper function? > > > Thanks. I wasn't sure what to put in the helper and what to keep in the > original function, so I'll wait a bit for comments before the commit. Works for me. Richard. > > Bootstrap (default languages) + testsuite on x86_64-unknown-linux-gnu. > > 2013-10-08 Marc Glisse <marc.glisse@inria.fr> > > PR tree-optimization/58480 > gcc/ > * tree-vrp.c (infer_nonnull_range): New function. > (infer_value_range): Call infer_nonnull_range. > > > gcc/testsuite/ > * gcc.dg/tree-ssa/pr58480.c: New file. > > -- > Marc Glisse > > Index: testsuite/gcc.dg/tree-ssa/pr58480.c > =================================================================== > --- testsuite/gcc.dg/tree-ssa/pr58480.c (revision 0) > +++ testsuite/gcc.dg/tree-ssa/pr58480.c (working copy) > @@ -0,0 +1,19 @@ > +/* { dg-do compile { target { ! keeps_null_pointer_checks } } } */ > +/* { dg-options "-O2 -fdump-tree-vrp1" } */ > + > +extern void eliminate (void); > +extern void* f1 (void *a, void *b) __attribute__((nonnull)); > +extern void* f2 (void *a, void *b) __attribute__((nonnull(2))); > +void g1 (void*p, void*q){ > + f1 (q, p); > + if (p == 0) > + eliminate (); > +} > +void g2 (void*p, void*q){ > + f2 (q, p); > + if (p == 0) > + eliminate (); > +} > + > +/* { dg-final { scan-tree-dump-times "Folding predicate\[^\\n\]*to 0" 2 > "vrp1" } } */ > +/* { dg-final { cleanup-tree-dump "vrp1" } } */ > > Property changes on: testsuite/gcc.dg/tree-ssa/pr58480.c > ___________________________________________________________________ > Added: svn:eol-style > ## -0,0 +1 ## > +native > \ No newline at end of property > Added: svn:keywords > ## -0,0 +1 ## > +Author Date Id Revision URL > \ No newline at end of property > Index: tree-vrp.c > =================================================================== > --- tree-vrp.c (revision 203241) > +++ tree-vrp.c (working copy) > @@ -4455,63 +4455,103 @@ build_assert_expr_for (tree cond, tree v > > static inline bool > fp_predicate (gimple stmt) > { > GIMPLE_CHECK (stmt, GIMPLE_COND); > > return FLOAT_TYPE_P (TREE_TYPE (gimple_cond_lhs (stmt))); > } > > > +/* If OP can be inferred to be non-zero after STMT executes, return true. > */ > + > +static bool > +infer_nonnull_range (gimple stmt, tree op) > +{ > + /* We can only assume that a pointer dereference will yield > + non-NULL if -fdelete-null-pointer-checks is enabled. */ > + if (!flag_delete_null_pointer_checks > + || !POINTER_TYPE_P (TREE_TYPE (op)) > + || gimple_code (stmt) == GIMPLE_ASM) > + return false; > + > + unsigned num_uses, num_loads, num_stores; > + > + count_uses_and_derefs (op, stmt, &num_uses, &num_loads, &num_stores); > + if (num_loads + num_stores > 0) > + return true; > + > + if (gimple_code (stmt) == GIMPLE_CALL) > + { > + tree fntype = gimple_call_fntype (stmt); > + tree attrs = TYPE_ATTRIBUTES (fntype); > + for (; attrs; attrs = TREE_CHAIN (attrs)) > + { > + attrs = lookup_attribute ("nonnull", attrs); > + > + /* If "nonnull" wasn't specified, we know nothing about > + the argument. */ > + if (attrs == NULL_TREE) > + return false; > + > + /* If "nonnull" applies to all the arguments, then ARG > + is non-null. */ > + if (TREE_VALUE (attrs) == NULL_TREE) > + return true; > + > + /* Now see if op appears in the nonnull list. */ > + for (tree t = TREE_VALUE (attrs); t; t = TREE_CHAIN (t)) > + { > + int idx = TREE_INT_CST_LOW (TREE_VALUE (t)) - 1; > + tree arg = gimple_call_arg (stmt, idx); > + if (op == arg) > + return true; > + } > + } > + } > + > + return false; > +} > + > /* If the range of values taken by OP can be inferred after STMT executes, > return the comparison code (COMP_CODE_P) and value (VAL_P) that > describes the inferred range. Return true if a range could be > inferred. */ > > static bool > infer_value_range (gimple stmt, tree op, enum tree_code *comp_code_p, tree > *val_p) > { > *val_p = NULL_TREE; > *comp_code_p = ERROR_MARK; > > /* Do not attempt to infer anything in names that flow through > abnormal edges. */ > if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (op)) > return false; > > /* Similarly, don't infer anything from statements that may throw > - exceptions. */ > + exceptions. ??? Relax this requirement? */ > if (stmt_could_throw_p (stmt)) > return false; > > /* If STMT is the last statement of a basic block with no > successors, there is no point inferring anything about any of its > operands. We would not be able to find a proper insertion point > for the assertion, anyway. */ > if (stmt_ends_bb_p (stmt) && EDGE_COUNT (gimple_bb (stmt)->succs) == 0) > return false; > > - /* We can only assume that a pointer dereference will yield > - non-NULL if -fdelete-null-pointer-checks is enabled. */ > - if (flag_delete_null_pointer_checks > - && POINTER_TYPE_P (TREE_TYPE (op)) > - && gimple_code (stmt) != GIMPLE_ASM) > + if (infer_nonnull_range (stmt, op)) > { > - unsigned num_uses, num_loads, num_stores; > - > - count_uses_and_derefs (op, stmt, &num_uses, &num_loads, &num_stores); > - if (num_loads + num_stores > 0) > - { > - *val_p = build_int_cst (TREE_TYPE (op), 0); > - *comp_code_p = NE_EXPR; > - return true; > - } > + *val_p = build_int_cst (TREE_TYPE (op), 0); > + *comp_code_p = NE_EXPR; > + return true; > } > > return false; > } > > > void dump_asserts_for (FILE *, tree); > void debug_asserts_for (tree); > void dump_all_asserts (FILE *); > void debug_all_asserts (void); >
Index: testsuite/gcc.dg/tree-ssa/pr58480.c =================================================================== --- testsuite/gcc.dg/tree-ssa/pr58480.c (revision 0) +++ testsuite/gcc.dg/tree-ssa/pr58480.c (working copy) @@ -0,0 +1,19 @@ +/* { dg-do compile { target { ! keeps_null_pointer_checks } } } */ +/* { dg-options "-O2 -fdump-tree-vrp1" } */ + +extern void eliminate (void); +extern void* f1 (void *a, void *b) __attribute__((nonnull)); +extern void* f2 (void *a, void *b) __attribute__((nonnull(2))); +void g1 (void*p, void*q){ + f1 (q, p); + if (p == 0) + eliminate (); +} +void g2 (void*p, void*q){ + f2 (q, p); + if (p == 0) + eliminate (); +} + +/* { dg-final { scan-tree-dump-times "Folding predicate\[^\\n\]*to 0" 2 "vrp1" } } */ +/* { dg-final { cleanup-tree-dump "vrp1" } } */ Property changes on: testsuite/gcc.dg/tree-ssa/pr58480.c ___________________________________________________________________ Added: svn:eol-style ## -0,0 +1 ## +native \ No newline at end of property Added: svn:keywords ## -0,0 +1 ## +Author Date Id Revision URL \ No newline at end of property Index: tree-vrp.c =================================================================== --- tree-vrp.c (revision 203241) +++ tree-vrp.c (working copy) @@ -4455,63 +4455,103 @@ build_assert_expr_for (tree cond, tree v static inline bool fp_predicate (gimple stmt) { GIMPLE_CHECK (stmt, GIMPLE_COND); return FLOAT_TYPE_P (TREE_TYPE (gimple_cond_lhs (stmt))); } +/* If OP can be inferred to be non-zero after STMT executes, return true. */ + +static bool +infer_nonnull_range (gimple stmt, tree op) +{ + /* We can only assume that a pointer dereference will yield + non-NULL if -fdelete-null-pointer-checks is enabled. */ + if (!flag_delete_null_pointer_checks + || !POINTER_TYPE_P (TREE_TYPE (op)) + || gimple_code (stmt) == GIMPLE_ASM) + return false; + + unsigned num_uses, num_loads, num_stores; + + count_uses_and_derefs (op, stmt, &num_uses, &num_loads, &num_stores); + if (num_loads + num_stores > 0) + return true; + + if (gimple_code (stmt) == GIMPLE_CALL) + { + tree fntype = gimple_call_fntype (stmt); + tree attrs = TYPE_ATTRIBUTES (fntype); + for (; attrs; attrs = TREE_CHAIN (attrs)) + { + attrs = lookup_attribute ("nonnull", attrs); + + /* If "nonnull" wasn't specified, we know nothing about + the argument. */ + if (attrs == NULL_TREE) + return false; + + /* If "nonnull" applies to all the arguments, then ARG + is non-null. */ + if (TREE_VALUE (attrs) == NULL_TREE) + return true; + + /* Now see if op appears in the nonnull list. */ + for (tree t = TREE_VALUE (attrs); t; t = TREE_CHAIN (t)) + { + int idx = TREE_INT_CST_LOW (TREE_VALUE (t)) - 1; + tree arg = gimple_call_arg (stmt, idx); + if (op == arg) + return true; + } + } + } + + return false; +} + /* If the range of values taken by OP can be inferred after STMT executes, return the comparison code (COMP_CODE_P) and value (VAL_P) that describes the inferred range. Return true if a range could be inferred. */ static bool infer_value_range (gimple stmt, tree op, enum tree_code *comp_code_p, tree *val_p) { *val_p = NULL_TREE; *comp_code_p = ERROR_MARK; /* Do not attempt to infer anything in names that flow through abnormal edges. */ if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (op)) return false; /* Similarly, don't infer anything from statements that may throw - exceptions. */ + exceptions. ??? Relax this requirement? */ if (stmt_could_throw_p (stmt)) return false; /* If STMT is the last statement of a basic block with no successors, there is no point inferring anything about any of its operands. We would not be able to find a proper insertion point for the assertion, anyway. */ if (stmt_ends_bb_p (stmt) && EDGE_COUNT (gimple_bb (stmt)->succs) == 0) return false; - /* We can only assume that a pointer dereference will yield - non-NULL if -fdelete-null-pointer-checks is enabled. */ - if (flag_delete_null_pointer_checks - && POINTER_TYPE_P (TREE_TYPE (op)) - && gimple_code (stmt) != GIMPLE_ASM) + if (infer_nonnull_range (stmt, op)) { - unsigned num_uses, num_loads, num_stores; - - count_uses_and_derefs (op, stmt, &num_uses, &num_loads, &num_stores); - if (num_loads + num_stores > 0) - { - *val_p = build_int_cst (TREE_TYPE (op), 0); - *comp_code_p = NE_EXPR; - return true; - } + *val_p = build_int_cst (TREE_TYPE (op), 0); + *comp_code_p = NE_EXPR; + return true; } return false; } void dump_asserts_for (FILE *, tree); void debug_asserts_for (tree); void dump_all_asserts (FILE *); void debug_all_asserts (void);