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

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.

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

On Wed, Oct 8, 2014 at 9:08 PM, Ilya Enkovich <enkovich.gnu@gmail.com> 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.
>
>
> 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;

Please don't use ->next_bb but instead use FOR_EACH_BB_FN (cfun, bb)
instead.

> +      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);
> +             if (!update_call_from_tree (&gsi, res))
> +               gimplify_and_update_call_from_tree (&gsi, res);

update_call_from_tree should always succeed with res being a call argument.

Richard.

> +             stmt = gsi_stmt (gsi);
> +             update_stmt (stmt);
> +           }
> +        }
> +      bb = next;
> +    }
> +  while (bb);
> +}
> +
>  /* Initialize pass.  */
>  static void
>  chkp_init (void)
> @@ -3872,6 +3915,8 @@ chkp_execute (void)
>
>    chkp_instrument_function ();
>
> +  chkp_remove_useless_builtins ();
> +
>    chkp_function_mark_instrumented (cfun->decl);
>
>    chkp_fix_cfg ();