On 09 Oct 11:13, Jeff Law wrote: > On 10/08/14 13:21, Ilya Enkovich wrote: > >Hi, > > > >This patch adds a removal of checks known to always pass into checker > >optimization. > > > >Thanks, > >Ilya > >-- > >2014-10-08 Ilya Enkovich <ilya.enkov...@intel.com> > > > > * tree-chkp.c (chkp_remove_check_if_pass): New. > > (chkp_remove_constant_checks): New. > > (chkp_opt_execute): Run constant check removal > > algorithm. > So again, I'd like to see all the optimization stuff pulled into its > own file and and basic tests that we can use for smoke testing now > and in the future. > > > > > >+ else if (result == -1) > >+ { > >+ if (dump_file && (dump_flags & TDF_DETAILS)) > >+ fprintf (dump_file, " action: keep check (always fail)\n"); > >+ } > ISTM this case should generate a compile-time warning. We've just > determined statically that this test is always going to fail, right? > > >+ /* Iterate throw all found checks in BB. */ > s/throw/through/ > > With the changes above, this will be OK for the trunk. > > > Jeff
Thanks for review! Here is a version with a warning and a couple of tests added. Ilya -- gcc/ 2014-10-13 Ilya Enkovich <ilya.enkov...@intel.com> * tree-chkp-opt.c: Include diagnostic.h. (chkp_remove_check_if_pass): New. (chkp_remove_constant_checks): New. (chkp_opt_execute): Run constant check removal algorithm. * c-family/c.opt (Wchkp): New. gcc/testsuite/ 2014-10-13 Ilya Enkovich <ilya.enkov...@intel.com> * gcc.target/i386/chkp-const-check-1.c: New. * gcc.target/i386/chkp-const-check-2.c: New. diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 1ca5a95..5202e3c 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -323,6 +323,10 @@ Wchar-subscripts C ObjC C++ ObjC++ Var(warn_char_subscripts) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) Warn about subscripts whose type is \"char\" +Wchkp +C ObjC C++ ObjC++ Var(warn_chkp) Warning EnabledBy(Wall) +Warn about memory access errors found by Pointer Bounds Checker + Wclobbered C ObjC C++ ObjC++ Var(warn_clobbered) Warning EnabledBy(Wextra) Warn about variables that might be changed by \"longjmp\" or \"vfork\" diff --git a/gcc/testsuite/gcc.target/i386/chkp-const-check-1.c b/gcc/testsuite/gcc.target/i386/chkp-const-check-1.c new file mode 100644 index 0000000..8c90239 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/chkp-const-check-1.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-fcheck-pointer-bounds -mmpx -fdump-tree-chkp -O2 -fdump-tree-chkpopt" } */ +/* { dg-final { scan-tree-dump-not "bndcl" "chkpopt" } } */ +/* { dg-final { scan-tree-dump-not "bndcu" "chkpopt" } } */ + + +int test (int *p) +{ + p = (int *)__builtin___bnd_set_ptr_bounds (p, sizeof (int)); + return *p; +} diff --git a/gcc/testsuite/gcc.target/i386/chkp-const-check-2.c b/gcc/testsuite/gcc.target/i386/chkp-const-check-2.c new file mode 100644 index 0000000..ab573eb --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/chkp-const-check-2.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-fcheck-pointer-bounds -mmpx -fdump-tree-chkp -O2 -Wchkp" } */ + +int test (int *p) +{ + p = (int *)__builtin___bnd_set_ptr_bounds (p, sizeof (int)); + return *(p + 1); /* { dg-warning "memory access check always fail" "" } */ +} diff --git a/gcc/tree-chkp-opt.c b/gcc/tree-chkp-opt.c index 620df47..5112769 100644 --- a/gcc/tree-chkp-opt.c +++ b/gcc/tree-chkp-opt.c @@ -47,6 +47,7 @@ along with GCC; see the file COPYING3. If not see #include "gimplify-me.h" #include "expr.h" #include "tree-chkp.h" +#include "diagnostic.h" enum check_type { @@ -693,6 +694,48 @@ chkp_get_check_result (struct check_info *ci, tree bounds) return res; } +/* Try to compare bounds value and address value + used in the check CI. If we can prove that check + always pass then remove it. */ +static void +chkp_remove_check_if_pass (struct check_info *ci) +{ + int result = 0; + + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fprintf (dump_file, "Trying to remove check: "); + print_gimple_stmt (dump_file, ci->stmt, 0, 0); + } + + result = chkp_get_check_result (ci, ci->bounds); + + if (result == 1) + { + gimple_stmt_iterator i = gsi_for_stmt (ci->stmt); + + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, " action: delete check (always pass)\n"); + + gsi_remove (&i, true); + unlink_stmt_vdef (ci->stmt); + release_defs (ci->stmt); + ci->stmt = NULL; + } + else if (result == -1) + { + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, " action: keep check (always fail)\n"); + warning_at (gimple_location (ci->stmt), OPT_Wchkp, + "memory access check always fail"); + } + else if (result == 0) + { + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, " action: keep check (cannot compute result)\n"); + } +} + /* For bounds used in CI check if bounds are produced by intersection and we may use outer bounds instead. If transformation is possible then fix check statement and @@ -776,6 +819,27 @@ chkp_remove_excess_intersections (void) } } +/* Try to remove all checks which are known to alwyas pass. */ +static void +chkp_remove_constant_checks (void) +{ + basic_block bb; + + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, "Searching for redundant checks...\n"); + + FOR_EACH_BB_FN (bb, cfun) + { + struct bb_checks *bbc = &check_infos[bb->index]; + unsigned int no; + + /* Iterate through all found checks in BB. */ + for (no = 0; no < bbc->checks.length (); no++) + if (bbc->checks[no].stmt) + chkp_remove_check_if_pass (&bbc->checks[no]); + } +} + /* Return fast version of string function FNCODE. */ static tree chkp_get_nobnd_fndecl (enum built_in_function fncode) @@ -1043,6 +1107,8 @@ chkp_opt_execute (void) chkp_remove_excess_intersections (); + chkp_remove_constant_checks (); + chkp_release_check_info (); chkp_opt_fini ();