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, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.


On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>> On Tue, Nov 5, 2013 at 1:02 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 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.
>>
>> I don't care so much for RTL, but this description hints at that the
>> suggestion above would work (and it would eliminate all my concerns about
>> the representation on the GIMPLE level - you'd not even need this
>> strange POINTER_BOUNDS_TYPE as far as I can see.
>>
>>> 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.
>>
>> Well, what are the constraints that you need to avoid propagation in
>> the first place?
>
> For input parameter P I need to have
>   BOUNDS = __builtin_arg_bnd (P)
> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
> __builtin_arg_bnd (P) replacing P with its copy or some value. It
> makes call useless because removes information about parameter whose
> bounds we refer to. I want such optimization to ignore
> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
> there as arg.

How does a compilable testcase look like that shows how the default def
is used?  And what transforms break that use?  I really cannot see
how this would happen (and you didn't add a testcase).

Richard.

>>
>>> 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.
>>
>> I was thinking of
>>
>> int foo (char *s)
>> {
>>    int bar (void)
>>    {
>>       ... use bound of 's' of the containing function ...
>>       foo (q, and pass it along here for q)
>>    }
>> }
>>
>> that is references to the containing functions parameters and their
>
> All foo's locals referenced by nested bar here are placed into special
> structure and all bounds should be stored in appropriated entries of
> Bound Table.  Nested function may refer to them via this Table.
>
> Ilya
>
>>
>> Richard.
>>
>>> 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);
>>>>>>  }
>>>>
>>>>


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