diff mbox

[CHKP,always_inline,1/2] Allow inlining for not instrumented calls to always_inline functions

Message ID 20150116110433.GB55666@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich Jan. 16, 2015, 11:04 a.m. UTC
Hi,

Currently compiler emits an error in case of not instrumented call to instrumented alwyas_inline function.  It happens because when we inline there is only instrumented version of function available and we don't inline thunks.  This patch solves the problem by split of thunk production pass into two passes.  The first one removes all functions we don't have to inline.  The other one does the rest after local optimizations.

Bootstrapped and checked on x86_64-unknown-linux-gnu.  This patch causes fault in chkp-strchr.c test and also breaks instrumented bootstrap.  Both are due to problem in SRA fixed by the next patch.  OK for trunk?

Thanks,
Ilya
--
gcc/

2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>

	* ipa-chkp.c (chkp_produce_thunks): Add early param.
	(pass_data_ipa_chkp_early_produce_thunks): New.
	(pass_ipa_chkp_early_produce_thunks): New.
	(pass_ipa_chkp_produce_thunks::execute): Adjust to new
	chkp_produce_thunks signature.
	(make_pass_ipa_chkp_early_produce_thunks): New.
	* passes.def (pass_ipa_chkp_early_produce_thunks): New.
	(pass_ipa_chkp_produce_thunks): Move after local optimizations.
	* tree-pass.h (make_pass_ipa_chkp_early_produce_thunks): New.

gcc/testsuite/

2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>

	* gcc.target/i386/chkp-always_inline.c: New.

Comments

Jeff Law Jan. 16, 2015, 4:58 p.m. UTC | #1
On 01/16/15 04:04, Ilya Enkovich wrote:
> Hi,
>
> Currently compiler emits an error in case of not instrumented call to instrumented alwyas_inline function.  It happens because when we inline there is only instrumented version of function available and we don't inline thunks.  This patch solves the problem by split of thunk production pass into two passes.  The first one removes all functions we don't have to inline.  The other one does the rest after local optimizations.
>
> Bootstrapped and checked on x86_64-unknown-linux-gnu.  This patch causes fault in chkp-strchr.c test and also breaks instrumented bootstrap.  Both are due to problem in SRA fixed by the next patch.  OK for trunk?
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>
>
> 	* ipa-chkp.c (chkp_produce_thunks): Add early param.
  to support splitting of thunk production.  Do not remove bodies of
"always_inline" functions.

Is that an accurate representation of what changed?


> 	(pass_data_ipa_chkp_early_produce_thunks): New.
> 	(pass_ipa_chkp_early_produce_thunks): New.
> 	(pass_ipa_chkp_produce_thunks::execute): Adjust to new
> 	chkp_produce_thunks signature.
> 	(make_pass_ipa_chkp_early_produce_thunks): New.
> 	* passes.def (pass_ipa_chkp_early_produce_thunks): New.
> 	(pass_ipa_chkp_produce_thunks): Move after local optimizations.
> 	* tree-pass.h (make_pass_ipa_chkp_early_produce_thunks): New.
>
> gcc/testsuite/
>
> 2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>
>
> 	* gcc.target/i386/chkp-always_inline.c: New.
With the updated ChangeLog, this is fine.

jeff
Ilya Enkovich Jan. 19, 2015, 10:21 a.m. UTC | #2
2015-01-16 19:58 GMT+03:00 Jeff Law <law@redhat.com>:
> On 01/16/15 04:04, Ilya Enkovich wrote:
>>
>> Hi,
>>
>> Currently compiler emits an error in case of not instrumented call to
>> instrumented alwyas_inline function.  It happens because when we inline
>> there is only instrumented version of function available and we don't inline
>> thunks.  This patch solves the problem by split of thunk production pass
>> into two passes.  The first one removes all functions we don't have to
>> inline.  The other one does the rest after local optimizations.
>>
>> Bootstrapped and checked on x86_64-unknown-linux-gnu.  This patch causes
>> fault in chkp-strchr.c test and also breaks instrumented bootstrap.  Both
>> are due to problem in SRA fixed by the next patch.  OK for trunk?
>>
>> Thanks,
>> Ilya
>> --
>> gcc/
>>
>> 2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>         * ipa-chkp.c (chkp_produce_thunks): Add early param.
>
>  to support splitting of thunk production.  Do not remove bodies of
> "always_inline" functions.
>
> Is that an accurate representation of what changed?
>
>
>>         (pass_data_ipa_chkp_early_produce_thunks): New.
>>         (pass_ipa_chkp_early_produce_thunks): New.
>>         (pass_ipa_chkp_produce_thunks::execute): Adjust to new
>>         chkp_produce_thunks signature.
>>         (make_pass_ipa_chkp_early_produce_thunks): New.
>>         * passes.def (pass_ipa_chkp_early_produce_thunks): New.
>>         (pass_ipa_chkp_produce_thunks): Move after local optimizations.
>>         * tree-pass.h (make_pass_ipa_chkp_early_produce_thunks): New.
>>
>> gcc/testsuite/
>>
>> 2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>         * gcc.target/i386/chkp-always_inline.c: New.
>
> With the updated ChangeLog, this is fine.
>
> jeff
>

Thanks for review!  Here is a committed ChangeLog.

Ilya
--
gcc/

2015-01-19  Ilya Enkovich  <ilya.enkovich@intel.com>

        * ipa-chkp.c (chkp_produce_thunks): Add early param
        to split thunks production into two passes.  Keep
        'always_inline' function bodies after the first pass.
        (pass_data_ipa_chkp_early_produce_thunks): New.
        (pass_ipa_chkp_early_produce_thunks): New.
        (pass_ipa_chkp_produce_thunks::execute): Adjust to new
        chkp_produce_thunks signature.
        (make_pass_ipa_chkp_early_produce_thunks): New.
        * passes.def (pass_ipa_chkp_early_produce_thunks): New.
        (pass_ipa_chkp_produce_thunks): Move after local optimizations.
        * tree-pass.h (make_pass_ipa_chkp_early_produce_thunks): New.

gcc/testsuite/

2015-01-19  Ilya Enkovich  <ilya.enkovich@intel.com>

        * gcc.target/i386/chkp-always_inline.c: New.
diff mbox

Patch

diff --git a/gcc/ipa-chkp.c b/gcc/ipa-chkp.c
index 30d511d..b637987 100644
--- a/gcc/ipa-chkp.c
+++ b/gcc/ipa-chkp.c
@@ -622,7 +622,7 @@  chkp_versioning (void)
    function.  */
 
 static unsigned int
-chkp_produce_thunks (void)
+chkp_produce_thunks (bool early)
 {
   struct cgraph_node *node;
 
@@ -631,7 +631,9 @@  chkp_produce_thunks (void)
       if (!node->instrumentation_clone
 	  && node->instrumented_version
 	  && gimple_has_body_p (node->decl)
-	  && gimple_has_body_p (node->instrumented_version->decl))
+	  && gimple_has_body_p (node->instrumented_version->decl)
+	  && (!lookup_attribute ("always_inline", DECL_ATTRIBUTES (node->decl))
+	      || !early))
 	{
 	  node->release_body ();
 	  node->remove_callees ();
@@ -649,12 +651,15 @@  chkp_produce_thunks (void)
   /* Mark instrumentation clones created for aliases and thunks
      as insttrumented so they could be removed as unreachable
      now.  */
-  FOR_EACH_DEFINED_FUNCTION (node)
+  if (!early)
     {
-      if (node->instrumentation_clone
-	  && (node->alias || node->thunk.thunk_p)
-	  && !chkp_function_instrumented_p (node->decl))
-	chkp_function_mark_instrumented (node->decl);
+      FOR_EACH_DEFINED_FUNCTION (node)
+      {
+	if (node->instrumentation_clone
+	    && (node->alias || node->thunk.thunk_p)
+	    && !chkp_function_instrumented_p (node->decl))
+	  chkp_function_mark_instrumented (node->decl);
+      }
     }
 
   return TODO_remove_functions;
@@ -673,6 +678,19 @@  const pass_data pass_data_ipa_chkp_versioning =
   0 /* todo_flags_finish */
 };
 
+const pass_data pass_data_ipa_chkp_early_produce_thunks =
+{
+  SIMPLE_IPA_PASS, /* type */
+  "chkp_ecleanup", /* name */
+  OPTGROUP_NONE, /* optinfo_flags */
+  TV_NONE, /* tv_id */
+  0, /* properties_required */
+  0, /* properties_provided */
+  0, /* properties_destroyed */
+  0, /* todo_flags_start */
+  0 /* todo_flags_finish */
+};
+
 const pass_data pass_data_ipa_chkp_produce_thunks =
 {
   SIMPLE_IPA_PASS, /* type */
@@ -711,6 +729,31 @@  public:
 
 }; // class pass_ipa_chkp_versioning
 
+class pass_ipa_chkp_early_produce_thunks : public simple_ipa_opt_pass
+{
+public:
+  pass_ipa_chkp_early_produce_thunks (gcc::context *ctxt)
+    : simple_ipa_opt_pass (pass_data_ipa_chkp_early_produce_thunks, ctxt)
+  {}
+
+  /* opt_pass methods: */
+  virtual opt_pass * clone ()
+    {
+      return new pass_ipa_chkp_early_produce_thunks (m_ctxt);
+    }
+
+  virtual bool gate (function *)
+    {
+      return flag_check_pointer_bounds;
+    }
+
+  virtual unsigned int execute (function *)
+    {
+      return chkp_produce_thunks (true);
+    }
+
+}; // class pass_chkp_produce_thunks
+
 class pass_ipa_chkp_produce_thunks : public simple_ipa_opt_pass
 {
 public:
@@ -731,7 +774,7 @@  public:
 
   virtual unsigned int execute (function *)
     {
-      return chkp_produce_thunks ();
+      return chkp_produce_thunks (false);
     }
 
 }; // class pass_chkp_produce_thunks
@@ -743,6 +786,12 @@  make_pass_ipa_chkp_versioning (gcc::context *ctxt)
 }
 
 simple_ipa_opt_pass *
+make_pass_ipa_chkp_early_produce_thunks (gcc::context *ctxt)
+{
+  return new pass_ipa_chkp_early_produce_thunks (ctxt);
+}
+
+simple_ipa_opt_pass *
 make_pass_ipa_chkp_produce_thunks (gcc::context *ctxt)
 {
   return new pass_ipa_chkp_produce_thunks (ctxt);
diff --git a/gcc/passes.def b/gcc/passes.def
index c0ddee4..2bc5dcd 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -50,6 +50,7 @@  along with GCC; see the file COPYING3.  If not see
   NEXT_PASS (pass_ipa_free_lang_data);
   NEXT_PASS (pass_ipa_function_and_variable_visibility);
   NEXT_PASS (pass_ipa_chkp_versioning);
+  NEXT_PASS (pass_ipa_chkp_early_produce_thunks);
   NEXT_PASS (pass_build_ssa_passes);
   PUSH_INSERT_PASSES_WITHIN (pass_build_ssa_passes)
       NEXT_PASS (pass_fixup_cfg);
@@ -65,7 +66,6 @@  along with GCC; see the file COPYING3.  If not see
       NEXT_PASS (pass_chkp);
       NEXT_PASS (pass_rebuild_cgraph_edges);
   POP_INSERT_PASSES ()
-  NEXT_PASS (pass_ipa_chkp_produce_thunks);
 
   NEXT_PASS (pass_local_optimization_passes);
   PUSH_INSERT_PASSES_WITHIN (pass_local_optimization_passes)
@@ -103,6 +103,7 @@  along with GCC; see the file COPYING3.  If not see
       NEXT_PASS (pass_rebuild_cgraph_edges);
       NEXT_PASS (pass_inline_parameters);
   POP_INSERT_PASSES ()
+  NEXT_PASS (pass_ipa_chkp_produce_thunks);
   NEXT_PASS (pass_ipa_auto_profile);
   NEXT_PASS (pass_ipa_free_inline_summary);
   NEXT_PASS (pass_ipa_tree_profile);
diff --git a/gcc/testsuite/gcc.target/i386/chkp-always_inline.c b/gcc/testsuite/gcc.target/i386/chkp-always_inline.c
new file mode 100644
index 0000000..16d2358
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/chkp-always_inline.c
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-require-effective-target mpx } */
+/* { dg-options "-fcheck-pointer-bounds -mmpx -O2 -Wno-attributes" } */
+
+__attribute__((always_inline)) int f1 (int *p)
+{
+  return *p;
+}
+
+__attribute__((bnd_legacy)) int f2 (int *p)
+{
+  return f1 (p);
+}
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index 398ab83..c6d4a2d 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -334,6 +334,7 @@  extern void register_pass (opt_pass* pass, pass_positioning_ops pos,
 			   const char* ref_pass_name, int ref_pass_inst_number);
 
 extern simple_ipa_opt_pass *make_pass_ipa_chkp_versioning (gcc::context *ctxt);
+extern simple_ipa_opt_pass *make_pass_ipa_chkp_early_produce_thunks (gcc::context *ctxt);
 extern simple_ipa_opt_pass *make_pass_ipa_chkp_produce_thunks (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_chkp (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_chkp_opt (gcc::context *ctxt);