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.
>
>> 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);
>>>>> }
>>>
>>>
