This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, Pointer Bounds Checker 14/x] Passes [8/n] Remove useless builtin calls
- From: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- To: Jeff Law <law at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 10 Oct 2014 18:52:19 +0400
- Subject: Re: [PATCH, Pointer Bounds Checker 14/x] Passes [8/n] Remove useless builtin calls
- Authentication-results: sourceware.org; auth=none
- References: <20141008190857 dot GH13454 at msticlxl57 dot ims dot intel dot com> <5436F80D dot 9010706 at redhat dot com>
On 09 Oct 15:03, Jeff Law wrote:
> On 10/08/14 13:08, Ilya Enkovich wrote:
> >Hi,
> >
> >This patch adds removal of user calls to chkp builtins which become useless after instrumentation.
> >
> >Thanks,
> >Ilya
> >--
> >2014-10-08 Ilya Enkovich <ilya.enkovich@intel.com>
> >
> > * tree-chkp.c (chkp_remove_useless_builtins): New.
> > (chkp_execute): Remove useless calls to Pointer Bounds
> > Checker builtins.
> Please put this in the file with the optimizations. Tests too,
> which may require you to add some dumping bits into this code since
> you may not be able to directly see the behaviour you want in the
> gimple dumps later.
>
> What I'm a bit confused about is it looks like every one of these
> builtin calls you end up optimizing -- why not generate the simple
> copy in the first place and avoid the need for
> chkp_remove_useless_builtins completely? Clearly I'm missing
> something here.
>
>
> >
> >
> >diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
> >index 5443950..b424af8 100644
> >--- a/gcc/tree-chkp.c
> >+++ b/gcc/tree-chkp.c
> >@@ -3805,6 +3805,49 @@ 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, next;
> >+ gimple_stmt_iterator gsi;
> >+
> >+ bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
> >+ do
> >+ {
> >+ next = bb->next_bb;
> >+ 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;
> There's some kind of formatting goof on one or more of the
> preceeding lines. They should be at the same indention level. Most
> likely they aren't consistent with their use of tabs vs spaces. A
> nit, but please take care of it.
>
> If we end up keeping this code, then I think it'll be fine with the
> whitespace nit fixed. I just want to make sure there's a good
> reason to generate these builtins in the first place rather than the
> more direct assignment.
>
> 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.enkovich@intel.com>
* tree-chkp.c (chkp_remove_useless_builtins): New.
(chkp_execute): Remove useless calls to Pointer Bounds
Checker builtins.
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index 170b33b..fde0228 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -3805,6 +3805,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, next;
+ 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)
@@ -3872,6 +3910,8 @@ chkp_execute (void)
chkp_instrument_function ();
+ chkp_remove_useless_builtins ();
+
chkp_function_mark_instrumented (cfun->decl);
chkp_fix_cfg ();