Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.

[Ilya Enkovich <enkovich dot gnu at gmail dot com>]

2013/11/4 Richard Biener <richard.guenther@gmail.com>:
> Richard Biener <richard.guenther@gmail.com> wrote:
>>On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich
>><enkovich.gnu@gmail.com> wrote:
>>> Hi,
>>>
>>> Here is a patch which hadles the problem with optimization of
>>BUILT_IN_CHKP_ARG_BND calls.  Pointer Bounds Checker expects that
>>argument of this call is a default SSA_NAME of the PARM_DECL whose
>>bounds we want to get.  The problem is in optimizations which may
>>replace arg with it's copy or a known value.  This patch suppress such
>>modifications.
>>
>>This doesn't seem like a good fix.  I suppose you require the same on
>>RTL, that is, have the incoming arg reg coalesced with the use?
>>In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
>
> Btw, I would have chosen
>
> P_2 = __builtin_xyz (p_1, bound)
> Call (p_2)
>
> Thus make the builtins a transparent pass-through which effectively binds parameter to their bound, removing the need for the artificial arguments and making propagations a non.issue
>
> Also, how does the feature interact with other extensions such as nested functions or optimizations like inlining?

In RTL all incoming bounds are materialized into slot where bounds are
passed and arg_bnd call is expanded into this slot.  Thus in RTL bound
arg looks more like a regular arg.

If I just set abnormal phi flag for SSA_NAME, SSA verifier should fail
because it does not used in abnormal phi, shouldn't it?  Also it would
prevent all optimizations for these SSA_NAMEs right?  Instrumentation
is performed on the earlier stage, right after we build SSA. I think
using abnormal phi flag and binding pointers with bounds via calls
would prevent some useful optimizations.

Many interprocedural optimizations require some support when work with
instrumented calls.  Inlining support includes:
  - replacement of arg_bnd calls with actual bounds passed to the inlined call
  - replacement of retbnd call with bounds, returned by inlined function

Not all IPA passes are fully enabled right now.  E.g. I restrict
bounded value propagation in ipa-prop and bounded args in functions
generated by ipa-split.  Such features will be enabled later.

For nested functions I do not see much difference from checker point
of view.  It just has an additional static chain param. Probably I
miss here something.  I did just few tests with nested functions.

Ilya

>
> Richard.
>
>
>>Richard.
>>
>>> Thanks,
>>> Ilya
>>> --
>>>
>>> gcc/
>>>
>>> 2013-10-28  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>
>>>         * tree-into-ssa.c: Include "target.h"
>>>         (rewrite_update_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>>         * tree-ssa-dom.c: Include "target.h"
>>>         (cprop_into_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>>
>>>
>>> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
>>> index 981e9f4..8d48f6d 100644
>>> --- a/gcc/tree-into-ssa.c
>>> +++ b/gcc/tree-into-ssa.c
>>> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.  If not see
>>>  #include "params.h"
>>>  #include "diagnostic-core.h"
>>>  #include "tree-into-ssa.h"
>>> +#include "target.h"
>>>
>>>
>>>  /* This file builds the SSA form for a function as described in:
>>> @@ -1921,8 +1922,14 @@ rewrite_update_stmt (gimple stmt,
>>gimple_stmt_iterator gsi)
>>>      }
>>>
>>>    /* Rewrite USES included in OLD_SSA_NAMES and USES whose
>>underlying
>>> -     symbol is marked for renaming.  */
>>> -  if (rewrite_uses_p (stmt))
>>> +     symbol is marked for renaming.
>>> +     Skip calls to BUILT_IN_CHKP_ARG_BND whose arg should never be
>>> +     renamed.  */
>>> +  if (rewrite_uses_p (stmt)
>>> +      && !(flag_check_pointer_bounds
>>> +          && (gimple_code (stmt) == GIMPLE_CALL)
>>> +          && gimple_call_fndecl (stmt)
>>> +          == targetm.builtin_chkp_function (BUILT_IN_CHKP_ARG_BND)))
>>>      {
>>>        if (is_gimple_debug (stmt))
>>>         {
>>> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
>>> index 211bfcf..445278a 100644
>>> --- a/gcc/tree-ssa-dom.c
>>> +++ b/gcc/tree-ssa-dom.c
>>> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
>>>  #include "params.h"
>>>  #include "tree-ssa-threadedge.h"
>>>  #include "tree-ssa-dom.h"
>>> +#include "target.h"
>>>
>>>  /* This file implements optimizations on the dominator tree.  */
>>>
>>> @@ -2266,6 +2267,16 @@ cprop_into_stmt (gimple stmt)
>>>    use_operand_p op_p;
>>>    ssa_op_iter iter;
>>>
>>> +  /* Call used to obtain bounds of input arg by Pointer Bounds
>>Checker
>>> +     should not be optimized.  Argument of the call is a default
>>> +     SSA_NAME of PARM_DECL.  It should never be replaced by value.
>>*/
>>> +  if (flag_check_pointer_bounds && gimple_code (stmt) ==
>>GIMPLE_CALL)
>>> +    {
>>> +      tree fndecl = gimple_call_fndecl (stmt);
>>> +      if (fndecl == targetm.builtin_chkp_function
>>(BUILT_IN_CHKP_ARG_BND))
>>> +       return;
>>> +    }
>>> +
>>>    FOR_EACH_SSA_USE_OPERAND (op_p, stmt, iter, SSA_OP_USE)
>>>      cprop_operand (stmt, op_p);
>>>  }
>
>