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