[MPX,2/X] Pointers Checker [15/25] IPA Propagation

Hi,

Here is a patch to disable propagation of bounded values.

Thanks,
Ilya
--
2013-11-13  Ilya Enkovich  <ilya.enkovich@intel.com>

	* ipa-prop.c: Include tree-chkp.h.
	(ipa_compute_jump_functions_for_edge): Do not propagate bounded args.

On Mon, Nov 18, 2013 at 02:28:58PM +0400, Ilya Enkovich wrote:
> Hi,
> 
> Here is a patch to disable propagation of bounded values.
> 

Why do ypu need to do this?  If the problem is that IPA-CP can remove
parameter it knows is a constant, which somehow confuses how you pass
bounds, then it is much better to clear
node->local.can_change_signature flag for such nodes and it will not
happen while still propagating stuff.

Or is there some other reason?

Thanks,

Martin


> Thanks,
> Ilya
> --
> 2013-11-13  Ilya Enkovich  <ilya.enkovich@intel.com>
> 
> 	* ipa-prop.c: Include tree-chkp.h.
> 	(ipa_compute_jump_functions_for_edge): Do not propagate bounded args.
> 
> 
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index eb464e4..81e1237 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "tree-streamer.h"
>  #include "params.h"
>  #include "ipa-utils.h"
> +#include "tree-chkp.h"
>  
>  /* Intermediate information about a parameter that is only useful during the
>     run of ipa_analyze_node and is not kept afterwards.  */
> @@ -1558,6 +1559,7 @@ ipa_compute_jump_functions_for_edge (struct param_analysis_info *parms_ainfo,
>    struct ipa_node_params *info = IPA_NODE_REF (cs->caller);
>    struct ipa_edge_args *args = IPA_EDGE_REF (cs);
>    gimple call = cs->call_stmt;
> +  tree fndecl = gimple_call_fndecl (call);
>    int n, arg_num = gimple_call_num_args (call);
>  
>    if (arg_num == 0 || args->jump_functions)
> @@ -1575,7 +1577,13 @@ ipa_compute_jump_functions_for_edge (struct param_analysis_info *parms_ainfo,
>        tree arg = gimple_call_arg (call, n);
>        tree param_type = ipa_get_callee_param_type (cs, n);
>  
> -      if (is_gimple_ip_invariant (arg))
> +      /* No optimization for bounded types yet implemented.  */
> +      if ((gimple_call_with_bounds_p (call)
> +	   || (fndecl && chkp_function_instrumented_p (fndecl)))
> +	  && ((param_type && chkp_type_has_pointer (param_type))
> +	      || (!param_type && chkp_type_has_pointer (TREE_TYPE (arg)))))
> +	continue;
> +      else if (is_gimple_ip_invariant (arg))
>  	ipa_set_jf_constant (jfunc, arg, cs);
>        else if (!is_gimple_reg_type (TREE_TYPE (arg))
>  	       && TREE_CODE (arg) == PARM_DECL)

2013/11/18 Martin Jambor <mjambor@suse.cz>:
> On Mon, Nov 18, 2013 at 02:28:58PM +0400, Ilya Enkovich wrote:
>> Hi,
>>
>> Here is a patch to disable propagation of bounded values.
>>
>
> Why do ypu need to do this?  If the problem is that IPA-CP can remove
> parameter it knows is a constant, which somehow confuses how you pass
> bounds, then it is much better to clear
> node->local.can_change_signature flag for such nodes and it will not
> happen while still propagating stuff.
>
> Or is there some other reason?

Thanks for pointing to this flag. I'll look into it.

There is another problem in propagation - value shoud never be
propagated into BUILT_IN_CHKP_ARG_BND calls.  Ideally bounds of
propagated value should also be analyzed and propagated (in the most
cases bounds for constant value are also constant).  I suppose
can_change_signature flag can be used for lightweight support, when
param is propagated but bounds are not (it would still require special
handling of BUILT_IN_CHKP_ARG_BND calls).

This patch is to avoid ICEs before some scheme is implemented.

Thanks,
Ilya
>
> Thanks,
>
> Martin
>
>
>> Thanks,
>> Ilya
>> --
>> 2013-11-13  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>       * ipa-prop.c: Include tree-chkp.h.
>>       (ipa_compute_jump_functions_for_edge): Do not propagate bounded args.
>>
>>
>> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
>> index eb464e4..81e1237 100644
>> --- a/gcc/ipa-prop.c
>> +++ b/gcc/ipa-prop.c
>> @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "tree-streamer.h"
>>  #include "params.h"
>>  #include "ipa-utils.h"
>> +#include "tree-chkp.h"
>>
>>  /* Intermediate information about a parameter that is only useful during the
>>     run of ipa_analyze_node and is not kept afterwards.  */
>> @@ -1558,6 +1559,7 @@ ipa_compute_jump_functions_for_edge (struct param_analysis_info *parms_ainfo,
>>    struct ipa_node_params *info = IPA_NODE_REF (cs->caller);
>>    struct ipa_edge_args *args = IPA_EDGE_REF (cs);
>>    gimple call = cs->call_stmt;
>> +  tree fndecl = gimple_call_fndecl (call);
>>    int n, arg_num = gimple_call_num_args (call);
>>
>>    if (arg_num == 0 || args->jump_functions)
>> @@ -1575,7 +1577,13 @@ ipa_compute_jump_functions_for_edge (struct param_analysis_info *parms_ainfo,
>>        tree arg = gimple_call_arg (call, n);
>>        tree param_type = ipa_get_callee_param_type (cs, n);
>>
>> -      if (is_gimple_ip_invariant (arg))
>> +      /* No optimization for bounded types yet implemented.  */
>> +      if ((gimple_call_with_bounds_p (call)
>> +        || (fndecl && chkp_function_instrumented_p (fndecl)))
>> +       && ((param_type && chkp_type_has_pointer (param_type))
>> +           || (!param_type && chkp_type_has_pointer (TREE_TYPE (arg)))))
>> +     continue;
>> +      else if (is_gimple_ip_invariant (arg))
>>       ipa_set_jf_constant (jfunc, arg, cs);
>>        else if (!is_gimple_reg_type (TREE_TYPE (arg))
>>              && TREE_CODE (arg) == PARM_DECL)

Hi,

On Mon, Nov 18, 2013 at 10:38:49PM +0400, Ilya Enkovich wrote:
> 2013/11/18 Martin Jambor <mjambor@suse.cz>:
> > On Mon, Nov 18, 2013 at 02:28:58PM +0400, Ilya Enkovich wrote:
> >> Hi,
> >>
> >> Here is a patch to disable propagation of bounded values.
> >>
> >
> > Why do ypu need to do this?  If the problem is that IPA-CP can remove
> > parameter it knows is a constant, which somehow confuses how you pass
> > bounds, then it is much better to clear
> > node->local.can_change_signature flag for such nodes and it will not
> > happen while still propagating stuff.
> >
> > Or is there some other reason?
> 
> Thanks for pointing to this flag. I'll look into it.
> 
> There is another problem in propagation - value shoud never be
> propagated into BUILT_IN_CHKP_ARG_BND calls.

If a particular function should be excluded from propagation then
initialize_node_lattices should set all its lattices to bottom and
that is enough, no need to complicate creation of individual jump
functions.

> Ideally bounds of
> propagated value should also be analyzed and propagated (in the most
> cases bounds for constant value are also constant).  I suppose
> can_change_signature flag can be used for lightweight support, when
> param is propagated but bounds are not (it would still require special
> handling of BUILT_IN_CHKP_ARG_BND calls).

If you want to remove bounds when also removing a parameter, adding
those to parms_to_skip in create_specialized_node should do the trick.

> 
> This patch is to avoid ICEs before some scheme is implemented.

Well, while I understand that one has things like this on a
development branch, I would be unhappy to see them merged into trunk
without a better explanation of what the problem is.

Thanks,

Martin


> 
> Thanks,
> Ilya
> >
> > Thanks,
> >
> > Martin
> >
> >
> >> Thanks,
> >> Ilya
> >> --
> >> 2013-11-13  Ilya Enkovich  <ilya.enkovich@intel.com>
> >>
> >>       * ipa-prop.c: Include tree-chkp.h.
> >>       (ipa_compute_jump_functions_for_edge): Do not propagate bounded args.
> >>
> >>
> >> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> >> index eb464e4..81e1237 100644
> >> --- a/gcc/ipa-prop.c
> >> +++ b/gcc/ipa-prop.c
> >> @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3.  If not see
> >>  #include "tree-streamer.h"
> >>  #include "params.h"
> >>  #include "ipa-utils.h"
> >> +#include "tree-chkp.h"
> >>
> >>  /* Intermediate information about a parameter that is only useful during the
> >>     run of ipa_analyze_node and is not kept afterwards.  */
> >> @@ -1558,6 +1559,7 @@ ipa_compute_jump_functions_for_edge (struct param_analysis_info *parms_ainfo,
> >>    struct ipa_node_params *info = IPA_NODE_REF (cs->caller);
> >>    struct ipa_edge_args *args = IPA_EDGE_REF (cs);
> >>    gimple call = cs->call_stmt;
> >> +  tree fndecl = gimple_call_fndecl (call);
> >>    int n, arg_num = gimple_call_num_args (call);
> >>
> >>    if (arg_num == 0 || args->jump_functions)
> >> @@ -1575,7 +1577,13 @@ ipa_compute_jump_functions_for_edge (struct param_analysis_info *parms_ainfo,
> >>        tree arg = gimple_call_arg (call, n);
> >>        tree param_type = ipa_get_callee_param_type (cs, n);
> >>
> >> -      if (is_gimple_ip_invariant (arg))
> >> +      /* No optimization for bounded types yet implemented.  */
> >> +      if ((gimple_call_with_bounds_p (call)
> >> +        || (fndecl && chkp_function_instrumented_p (fndecl)))
> >> +       && ((param_type && chkp_type_has_pointer (param_type))
> >> +           || (!param_type && chkp_type_has_pointer (TREE_TYPE (arg)))))
> >> +     continue;
> >> +      else if (is_gimple_ip_invariant (arg))
> >>       ipa_set_jf_constant (jfunc, arg, cs);
> >>        else if (!is_gimple_reg_type (TREE_TYPE (arg))
> >>              && TREE_CODE (arg) == PARM_DECL)