This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, Pointer Bounds Checker 14/x] Passes [8/n] Remove useless builtin calls


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 ();


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]