On 10 Oct 10:11, Jeff Law wrote: > On 10/10/14 08:52, Ilya Enkovich wrote: > >> > >>THanks, Jeff > > > >With this code we remove user builtins calls coming from source code. > >E.g.: > > > >p2 = (int *)__bnd_init_ptr_bounds (p1); *p2 = 0; > > > >which means p2 has value of p1 but has default bounds and following > >store is unchecked. These calls are important for instrumentation > >but useless after instrumentation. I don't think it is a part of > >checker optimizer because it doesn't optimize instrumentation code. > >Also this transformation is trivial enough for O0 and checker > >optimizer works starting from O2. > > > >Below is a version fixed according to Richard's comments. > > > >Thanks, Ilya -- 2014-10-10 Ilya Enkovich <ilya.enkov...@intel.com> > > > >* tree-chkp.c (chkp_remove_useless_builtins): New. (chkp_execute): > >Remove useless calls to Pointer Bounds Checker builtins. > Tests instrumentation are still needed. With some basic tests and > instrumentation this will be OK. > > I hate to be harping tests, but few developers are going to be > familiar with the MPX and related infrastructure and those tests are > critical to helping them know when they break something. > > Similarly if the plan is to iterate on improving things, then those > basic functionality tests will ultimately save time as you can smoke > test before running larger benchmarks. > > > jeff
Here is a version with tests added. Thanks, Ilya -- gcc/ 2014-10-13 Ilya Enkovich <ilya.enkov...@intel.com> * tree-chkp.c (chkp_remove_useless_builtins): New. (chkp_execute): Remove useless calls to Pointer Bounds Checker builtins. gcc/testsuite 2014-10-13 Ilya Enkovich <ilya.enkov...@intel.com> * gcc.target/i386/chkp-builtins-1.c: New. * gcc.target/i386/chkp-builtins-2.c: New. * gcc.target/i386/chkp-builtins-3.c: New. * gcc.target/i386/chkp-builtins-4.c: New. diff --git a/gcc/testsuite/gcc.target/i386/chkp-builtins-1.c b/gcc/testsuite/gcc.target/i386/chkp-builtins-1.c new file mode 100644 index 0000000..bcc1198 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/chkp-builtins-1.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-fcheck-pointer-bounds -mmpx -fdump-tree-chkp" } */ +/* { dg-final { scan-tree-dump-not "bnd_init_ptr_bounds" "chkp" } } */ + +void * +chkp_test (void *p) +{ + return __builtin___bnd_init_ptr_bounds (p); +} diff --git a/gcc/testsuite/gcc.target/i386/chkp-builtins-2.c b/gcc/testsuite/gcc.target/i386/chkp-builtins-2.c new file mode 100644 index 0000000..1f4a244 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/chkp-builtins-2.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-fcheck-pointer-bounds -mmpx -fdump-tree-chkp" } */ +/* { dg-final { scan-tree-dump-not "bnd_copy_ptr_bounds" "chkp" } } */ + +void * +chkp_test (void *p, void *q) +{ + return __builtin___bnd_copy_ptr_bounds (p, q); +} diff --git a/gcc/testsuite/gcc.target/i386/chkp-builtins-3.c b/gcc/testsuite/gcc.target/i386/chkp-builtins-3.c new file mode 100644 index 0000000..ea54ede --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/chkp-builtins-3.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-fcheck-pointer-bounds -mmpx -fdump-tree-chkp" } */ +/* { dg-final { scan-tree-dump-not "bnd_set_ptr_bounds" "chkp" } } */ + +void * +chkp_test (void *p) +{ + return __builtin___bnd_set_ptr_bounds (p, 10); +} diff --git a/gcc/testsuite/gcc.target/i386/chkp-builtins-4.c b/gcc/testsuite/gcc.target/i386/chkp-builtins-4.c new file mode 100644 index 0000000..cee780b --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/chkp-builtins-4.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-fcheck-pointer-bounds -mmpx -fdump-tree-chkp" } */ +/* { dg-final { scan-tree-dump-not "bnd_null_ptr_bounds" "chkp" } } */ + +void * +chkp_test (void *p) +{ + return __builtin___bnd_null_ptr_bounds (p); +} diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c index 9be153a..5957e45 100644 --- a/gcc/tree-chkp.c +++ b/gcc/tree-chkp.c @@ -3800,6 +3800,44 @@ chkp_instrument_function (void) } } +/* Find init/null/copy_ptr_bounds calls and replace them + with assignments. It should allow better code + optimization. */ + +static void +chkp_remove_useless_builtins () +{ + basic_block bb; + gimple_stmt_iterator gsi; + + FOR_EACH_BB_FN (bb, cfun) + { + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) + { + gimple stmt = gsi_stmt (gsi); + tree fndecl; + enum built_in_function fcode; + + /* Find builtins returning first arg and replace + them with assignments. */ + if (gimple_code (stmt) == GIMPLE_CALL + && (fndecl = gimple_call_fndecl (stmt)) + && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL + && (fcode = DECL_FUNCTION_CODE (fndecl)) + && (fcode == BUILT_IN_CHKP_INIT_PTR_BOUNDS + || fcode == BUILT_IN_CHKP_NULL_PTR_BOUNDS + || fcode == BUILT_IN_CHKP_COPY_PTR_BOUNDS + || fcode == BUILT_IN_CHKP_SET_PTR_BOUNDS)) + { + tree res = gimple_call_arg (stmt, 0); + update_call_from_tree (&gsi, res); + stmt = gsi_stmt (gsi); + update_stmt (stmt); + } + } + } +} + /* Initialize pass. */ static void chkp_init (void) @@ -3867,6 +3905,8 @@ chkp_execute (void) chkp_instrument_function (); + chkp_remove_useless_builtins (); + chkp_function_mark_instrumented (cfun->decl); chkp_fix_cfg ();