[PATCH, Pointer Bounds Checker 14/x] Passes [14/n] Optimize constant checks

[PATCH, Pointer Bounds Checker 14/x] Passes [14/n] Optimize constant checks

[Ilya Enkovich]

Hi,

This patch adds a removal of checks known to always pass into checker optimization.

Thanks,
Ilya
--
2014-10-08  Ilya Enkovich  <ilya.enkovich@intel.com>

	* tree-chkp.c (chkp_remove_check_if_pass): New.
	(chkp_remove_constant_checks): New.
	(chkp_opt_execute): Run constant check removal
	algorithm.


diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index c9b616b..ade546b 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -4828,6 +4828,46 @@ 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.  */
+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");
+    }
+  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
@@ -4911,6 +4951,27 @@ chkp_remove_excess_intersections (void)
     }
 }
 
+/*  Try to remove all checks which are known to alwyas pass.  */
+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 throw 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.  */
 tree
 chkp_get_nobnd_fndecl (enum built_in_function fncode)
@@ -5194,6 +5255,8 @@ chkp_opt_execute (void)
 
   chkp_remove_excess_intersections ();
 
+  chkp_remove_constant_checks ();
+
   chkp_release_check_info ();
 
   chkp_opt_fini ();

Re: [PATCH, Pointer Bounds Checker 14/x] Passes [14/n] Optimize constant checks

[Jeff Law]

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.enkovich@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

Re: [PATCH, Pointer Bounds Checker 14/x] Passes [14/n] Optimize constant checks

[Ilya Enkovich]

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.enkovich@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.enkovich@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.enkovich@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 ();