On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich....@gmail.com> wrote:
> 2013/11/5 Richard Biener <richard.guent...@gmail.com>:
>> On Tue, Nov 5, 2013 at 1:02 PM, Ilya Enkovich <enkovich....@gmail.com> wrote:
>>> 2013/11/4 Richard Biener <richard.guent...@gmail.com>:
>>>> Richard Biener <richard.guent...@gmail.com> wrote:
>>>>>On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich
>>>>><enkovich....@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.enkov...@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);
>>>>>> }
>>>>
>>>>
