On Thu, Oct 27, 2011 at 9:11 PM, Martin Jambor <mjam...@suse.cz> wrote:
> Hi,
>
> On Thu, Oct 27, 2011 at 03:07:10PM +0200, Richard Guenther wrote:
>> On Wed, Oct 26, 2011 at 8:25 PM, Martin Jambor <mjam...@suse.cz> wrote:
>> > Hi,
>> >
>> >
>> >
>> > 2011-10-26 Martin Jambor <mjam...@suse.cz>
>> >
>> > * ipa-prop.c (mark_modified): Moved up in the file.
>> > (is_parm_modified_before_call): Renamed to
>> > is_parm_modified_before_stmt, moved up in the file.
>> > (load_from_unmodified_param): New function.
>> > (compute_complex_assign_jump_func): Also attempt to create pass
>> > through jump functions for values loaded from (addressable)
>> > parameters.
>> >
>> > * testsuite/gcc.dg/ipa/ipcp-4.c: New test.
>> >
>
> ...
>
>> > Index: src/gcc/ipa-prop.c
>> > ===================================================================
>> > --- src.orig/gcc/ipa-prop.c
>> > +++ src/gcc/ipa-prop.c
>> > @@ -419,31 +419,105 @@ detect_type_change_ssa (tree arg, gimple
>> > return detect_type_change (arg, arg, call, jfunc, 0);
>> > }
>> >
>> > +/* Callback of walk_aliased_vdefs. Flags that it has been invoked to the
>> > + boolean variable pointed to by DATA. */
>> > +
>> > +static bool
>> > +mark_modified (ao_ref *ao ATTRIBUTE_UNUSED, tree vdef ATTRIBUTE_UNUSED,
>> > + void *data)
>> > +{
>> > + bool *b = (bool *) data;
>> > + *b = true;
>> > + return true;
>> > +}
>> > +
>> > +/* Return true if the formal parameter PARM might have been modified in
>> > this
>> > + function before reaching the statement STMT. PARM_AINFO is a pointer
>> > to a
>> > + structure containing temporary information about PARM. */
>> > +
>> > +static bool
>> > +is_parm_modified_before_stmt (struct param_analysis_info *parm_ainfo,
>> > + gimple stmt, tree parm)
>> > +{
>> > + bool modified = false;
>> > + ao_ref refd;
>> > +
>> > + if (parm_ainfo->modified)
>> > + return true;
>> > +
>> > + ao_ref_init (&refd, parm);
>> > + walk_aliased_vdefs (&refd, gimple_vuse (stmt), mark_modified,
>> > + &modified, &parm_ainfo->visited_statements);
>> > + if (modified)
>> > + {
>> > + parm_ainfo->modified = true;
>> > + return true;
>> > + }
>> > + return false;
>> > +}
>> > +
>> > +/* If STMT is an assignment that loads a value from an parameter
>> > declaration,
>> > + return the index of the parameter in ipa_node_params which has not been
>> > + modified. Otherwise return -1. */
>> > +
>> > +static int
>> > +load_from_unmodified_param (struct ipa_node_params *info,
>> > + struct param_analysis_info *parms_ainfo,
>> > + gimple stmt)
>> > +{
>> > + int index;
>> > + tree op1;
>> > +
>> > + if (!gimple_assign_single_p (stmt)
>> > + || gimple_assign_cast_p (stmt))
>>
>> The || gimple_assign_cast_p (stmt) check is redundant. Hopefully
>> you don't want to test for VIEW_CONVERT_EXPR here?
>
> Yeah, right, I managed to confuse myself with all the gimple
> accessors.
>
>>
>> > + return -1;
>> > +
>> > + op1 = gimple_assign_rhs1 (stmt);
>> > + index = ipa_get_param_decl_index (info, op1);
>>
>> That only succeeds for decls? Where do you check this is actually
>> a (full) load? I suppose it's a side-effect of ipa_get_parm_decl_index
>> in some way, but it's not clear. Beause ...
>
> Yes, it is a side-effect of the function. I've added the check for
> clarity and to save us the search but it is not strictly necessary.
>
>>
>> > + if (index < 0
>> > + || is_parm_modified_before_stmt (&parms_ainfo[index], stmt, op1))
>>
>> ... entering this without a VUSE (in case op1 is an SSA name, for example)
>> will do interesting things (likely ICE, at least compute garbage).
>
> I know. This has to be a load, though. I added a checking assert for
> non-NULL VUSE too, just for convenience.
>
>>
>> So, do you want to have TREE_CODE (op1) == PARM_DECL here?
>>
>> > + return -1;
>> > +
>> > + return index;
>> > +}
>> >
>> > /* Given that an actual argument is an SSA_NAME (given in NAME) and is a
>> > result
>> > of an assignment statement STMT, try to find out whether NAME can be
>> > described by a (possibly polynomial) pass-through jump-function or an
>> > - ancestor jump function and if so, write the appropriate function into
>> > - JFUNC */
>> > + ancestor jump function and if so, write the appropriate function into
>> > JFUNC.
>> > + PARMS_AINFO describes state of analysis with respect to individual
>> > formal
>> > + parameters. */
>> >
>> > static void
>> > compute_complex_assign_jump_func (struct ipa_node_params *info,
>> > + struct param_analysis_info *parms_ainfo,
>> > struct ipa_jump_func *jfunc,
>> > gimple call, gimple stmt, tree name)
>> > {
>> > HOST_WIDE_INT offset, size, max_size;
>> > - tree op1, op2, base, ssa;
>> > + tree op1, tc_ssa, base, ssa;
>> > int index;
>> >
>> > op1 = gimple_assign_rhs1 (stmt);
>> > - op2 = gimple_assign_rhs2 (stmt);
>> >
>> > - if (TREE_CODE (op1) == SSA_NAME
>> > - && SSA_NAME_IS_DEFAULT_DEF (op1))
>> > + if (TREE_CODE (op1) == SSA_NAME)
>> > {
>> > - index = ipa_get_param_decl_index (info, SSA_NAME_VAR (op1));
>> > - if (index < 0)
>> > - return;
>> > + if (SSA_NAME_IS_DEFAULT_DEF (op1))
>> > + index = ipa_get_param_decl_index (info, SSA_NAME_VAR (op1));
>> > + else
>> > + index = load_from_unmodified_param (info, parms_ainfo,
>> > + SSA_NAME_DEF_STMT (op1));
>>
>> Can you explain what you are doing for non-default-def names here?
>> I suppose it's to handle name = *this?
>
> No, not *this, rather to handle "a.0_2 = a;" where a is an addressable
> scalar PARM_DECL.
>
> Basically if I don't have a default-def SSA name (which guarantees I'm
> passing an unchanged value of a formal parameter) I look at the
> definition of the name and see if I can guarantee it is a load from a
> PARM_DECL (no dereferences involved). It is one step more complicated
> because I also want to be able to handle:
>
> a.0_3 = a;
> D.2064_4 = a.0_3 + 4;
> foo (D.2064_4);
>
> and construct an arithmetic jump function for that. So either the
> STMT itself is the load or it is a simple operation with the first
> operand being a result of a load. That is why
> load_from_unmodified_param is attempted at two places.
>
> But yes, apparently I need to improve the comment of the whole
> function and describe the cases it is supposed to handle.
>
>>
>> > + tc_ssa = op1;
>> > + }
>> > + else
>> > + {
>> > + index = load_from_unmodified_param (info, parms_ainfo, stmt);
>> > + tc_ssa = gimple_assign_lhs (stmt);
>> > + }
>> > +
>> > + if (index >= 0)
>> > + {
>> > + tree op2 = gimple_assign_rhs2 (stmt);
>>
>> So you also want to handle
>>
>> name = *this;
>> x = name + 5;
>>
>> ? How do you represent those jump-functions?
>>
>> This is lacking comments :/
>>
>> > if (op2)
>> > {
>> > @@ -458,8 +532,9 @@ compute_complex_assign_jump_func (struct
>> > jfunc->value.pass_through.operation = gimple_assign_rhs_code
>> > (stmt);
>> > jfunc->value.pass_through.operand = op2;
>> > }
>> > - else if (gimple_assign_unary_nop_p (stmt)
>> > - && !detect_type_change_ssa (op1, call, jfunc))
>> > + else if (gimple_assign_single_p (stmt)
>> > + && !gimple_assign_cast_p (stmt)
>>
>> Don't use gimple_assign_cast_p, it's weird and should go away. All
>> current users want to check sth more specific and consistent.
>
> Removed.
>
>
> I'm currently bootstrapping and testing this version of the patch. OK
> id it passes?
Ok.
Thanks,
Richard.
> Thanks,
>
> Martin
>
> 2011-10-27 Martin Jambor <mjam...@suse.cz>
>
> * ipa-prop.c (mark_modified): Moved up in the file.
> (is_parm_modified_before_call): Renamed to
> is_parm_modified_before_stmt, moved up in the file.
> (load_from_unmodified_param): New function.
> (compute_complex_assign_jump_func): Also attempt to create pass
> through jump functions for values loaded from (addressable)
> parameters.
>
> * testsuite/gcc.dg/ipa/ipcp-4.c: New test.
>
> Index: src/gcc/testsuite/gcc.dg/ipa/ipcp-4.c
> ===================================================================
> --- /dev/null
> +++ src/gcc/testsuite/gcc.dg/ipa/ipcp-4.c
> @@ -0,0 +1,68 @@
> +/* Test that IPA-CP is able to produce a pass-through jump function for the
> + call of g1 and g2 even though a is addressable. Also test that h is not
> + cloned. */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fipa-cp -fipa-cp-clone -fdump-ipa-cp
> -fno-early-inlining" } */
> +/* { dg-add-options bind_pic_locally } */
> +
> +extern void use_stuff (int);
> +extern void use_pointer (int *);
> +
> +static int
> +h (int a, int b)
> +{
> + int i;
> +
> + for (i = 8; i <= b; i++)
> + use_stuff (a+8);
> +}
> +
> +static int
> +g1 (int a, int b)
> +{
> + int i;
> +
> + for (i = 0; i <= b; i++)
> + use_pointer (&a);
> + h (a, b);
> +}
> +
> +static int
> +g2 (int a, int b)
> +{
> + int i;
> +
> + for (i = 4; i <= b; i += 2)
> + use_stuff (a);
> +}
> +
> +
> +static void
> +f (int a, int z)
> +{
> + if (z > 1)
> + g1 (a, z);
> + else
> + g2 (a + 4, z);
> + use_pointer (&a);
> +}
> +
> +int
> +main (int argc, char *argv[])
> +{
> + int i;
> + for (i = 0; i < 100; i++)
> + f (7, argc);
> + return 0;
> +}
> +
> +
> +/* { dg-final { scan-ipa-dump "Creating a specialized node of g1.*for all
> known contexts" "cp" } } */
> +/* { dg-final { scan-ipa-dump "Creating a specialized node of g2.*for all
> known contexts" "cp" } } */
> +/* { dg-final { scan-ipa-dump-not "Creating a specialized node of h.*for all
> known contexts" "cp" } } */
> +/* { dg-final { scan-ipa-dump-times "replacing param a with const 7" 2 "cp"
> } } */
> +/* { dg-final { scan-ipa-dump "replacing param a with const 11" "cp" } } */
> +/* { dg-final { cleanup-ipa-dump "cp" } } */
> +
> +
> Index: src/gcc/ipa-prop.c
> ===================================================================
> --- src.orig/gcc/ipa-prop.c
> +++ src/gcc/ipa-prop.c
> @@ -419,31 +419,154 @@ detect_type_change_ssa (tree arg, gimple
> return detect_type_change (arg, arg, call, jfunc, 0);
> }
>
> +/* Callback of walk_aliased_vdefs. Flags that it has been invoked to the
> + boolean variable pointed to by DATA. */
> +
> +static bool
> +mark_modified (ao_ref *ao ATTRIBUTE_UNUSED, tree vdef ATTRIBUTE_UNUSED,
> + void *data)
> +{
> + bool *b = (bool *) data;
> + *b = true;
> + return true;
> +}
> +
> +/* Return true if the formal parameter PARM might have been modified in this
> + function before reaching the statement STMT. PARM_AINFO is a pointer to a
> + structure containing temporary information about PARM. */
> +
> +static bool
> +is_parm_modified_before_stmt (struct param_analysis_info *parm_ainfo,
> + gimple stmt, tree parm)
> +{
> + bool modified = false;
> + ao_ref refd;
> +
> + if (parm_ainfo->modified)
> + return true;
> +
> + gcc_checking_assert (gimple_vuse (stmt) != NULL_TREE);
> + ao_ref_init (&refd, parm);
> + walk_aliased_vdefs (&refd, gimple_vuse (stmt), mark_modified,
> + &modified, &parm_ainfo->visited_statements);
> + if (modified)
> + {
> + parm_ainfo->modified = true;
> + return true;
> + }
> + return false;
> +}
> +
> +/* If STMT is an assignment that loads a value from an parameter declaration,
> + return the index of the parameter in ipa_node_params which has not been
> + modified. Otherwise return -1. */
> +
> +static int
> +load_from_unmodified_param (struct ipa_node_params *info,
> + struct param_analysis_info *parms_ainfo,
> + gimple stmt)
> +{
> + int index;
> + tree op1;
> +
> + if (!gimple_assign_single_p (stmt))
> + return -1;
> +
> + op1 = gimple_assign_rhs1 (stmt);
> + if (TREE_CODE (op1) != PARM_DECL)
> + return -1;
> +
> + index = ipa_get_param_decl_index (info, op1);
> + if (index < 0
> + || is_parm_modified_before_stmt (&parms_ainfo[index], stmt, op1))
> + return -1;
> +
> + return index;
> +}
>
> /* Given that an actual argument is an SSA_NAME (given in NAME) and is a
> result
> - of an assignment statement STMT, try to find out whether NAME can be
> - described by a (possibly polynomial) pass-through jump-function or an
> - ancestor jump function and if so, write the appropriate function into
> - JFUNC */
> + of an assignment statement STMT, try to determine whether we are actually
> + handling any of the following cases and construct an appropriate jump
> + function into JFUNC if so:
> +
> + 1) The passed value is loaded from a formal parameter which is not a
> gimple
> + register (most probably because it is addressable, the value has to be
> + scalar) and we can guarantee the value has not changed. This case can
> + therefore be described by a simple pass-through jump function. For
> example:
> +
> + foo (int a)
> + {
> + int a.0;
> +
> + a.0_2 = a;
> + bar (a.0_2);
> +
> + 2) The passed value can be described by a simple arithmetic pass-through
> + jump function. E.g.
> +
> + foo (int a)
> + {
> + int D.2064;
> +
> + D.2064_4 = a.1(D) + 4;
> + bar (D.2064_4);
> +
> + This case can also occur in combination of the previous one, e.g.:
> +
> + foo (int a, int z)
> + {
> + int a.0;
> + int D.2064;
> +
> + a.0_3 = a;
> + D.2064_4 = a.0_3 + 4;
> + foo (D.2064_4);
> +
> + 3) The passed value is an address of an object within another one (which
> + also passed by reference). Such situations are described by an ancestor
> + jump function and describe situations such as:
> +
> + B::foo() (struct B * const this)
> + {
> + struct A * D.1845;
> +
> + D.1845_2 = &this_1(D)->D.1748;
> + A::bar (D.1845_2);
> +
> + INFO is the structure describing individual parameters access different
> + stages of IPA optimizations. PARMS_AINFO contains the information that is
> + only needed for intraprocedural analysis. */
>
> static void
> compute_complex_assign_jump_func (struct ipa_node_params *info,
> + struct param_analysis_info *parms_ainfo,
> struct ipa_jump_func *jfunc,
> gimple call, gimple stmt, tree name)
> {
> HOST_WIDE_INT offset, size, max_size;
> - tree op1, op2, base, ssa;
> + tree op1, tc_ssa, base, ssa;
> int index;
>
> op1 = gimple_assign_rhs1 (stmt);
> - op2 = gimple_assign_rhs2 (stmt);
>
> - if (TREE_CODE (op1) == SSA_NAME
> - && SSA_NAME_IS_DEFAULT_DEF (op1))
> + if (TREE_CODE (op1) == SSA_NAME)
> {
> - index = ipa_get_param_decl_index (info, SSA_NAME_VAR (op1));
> - if (index < 0)
> - return;
> + if (SSA_NAME_IS_DEFAULT_DEF (op1))
> + index = ipa_get_param_decl_index (info, SSA_NAME_VAR (op1));
> + else
> + index = load_from_unmodified_param (info, parms_ainfo,
> + SSA_NAME_DEF_STMT (op1));
> + tc_ssa = op1;
> + }
> + else
> + {
> + index = load_from_unmodified_param (info, parms_ainfo, stmt);
> + tc_ssa = gimple_assign_lhs (stmt);
> + }
> +
> + if (index >= 0)
> + {
> + tree op2 = gimple_assign_rhs2 (stmt);
>
> if (op2)
> {
> @@ -458,8 +581,8 @@ compute_complex_assign_jump_func (struct
> jfunc->value.pass_through.operation = gimple_assign_rhs_code (stmt);
> jfunc->value.pass_through.operand = op2;
> }
> - else if (gimple_assign_unary_nop_p (stmt)
> - && !detect_type_change_ssa (op1, call, jfunc))
> + else if (gimple_assign_single_p (stmt)
> + && !detect_type_change_ssa (tc_ssa, call, jfunc))
> {
> jfunc->type = IPA_JF_PASS_THROUGH;
> jfunc->value.pass_through.formal_id = index;
> @@ -665,12 +788,14 @@ compute_known_type_jump_func (tree op, s
>
> /* Determine the jump functions of scalar arguments. Scalar means SSA names
> and constants of a number of selected types. INFO is the ipa_node_params
> - structure associated with the caller, FUNCTIONS is a pointer to an array
> of
> - jump function structures associated with CALL which is the call statement
> - being examined.*/
> + structure associated with the caller, PARMS_AINFO describes state of
> + analysis with respect to individual formal parameters. ARGS is the
> + ipa_edge_args structure describing the callsite CALL which is the call
> + statement being examined.*/
>
> static void
> compute_scalar_jump_functions (struct ipa_node_params *info,
> + struct param_analysis_info *parms_ainfo,
> struct ipa_edge_args *args,
> gimple call)
> {
> @@ -705,7 +830,8 @@ compute_scalar_jump_functions (struct ip
> {
> gimple stmt = SSA_NAME_DEF_STMT (arg);
> if (is_gimple_assign (stmt))
> - compute_complex_assign_jump_func (info, jfunc, call, stmt,
> arg);
> + compute_complex_assign_jump_func (info, parms_ainfo, jfunc,
> + call, stmt, arg);
> else if (gimple_code (stmt) == GIMPLE_PHI)
> compute_complex_ancestor_jump_func (info, jfunc, call, stmt);
> }
> @@ -748,43 +874,6 @@ type_like_member_ptr_p (tree type, tree
> return true;
> }
>
> -/* Callback of walk_aliased_vdefs. Flags that it has been invoked to the
> - boolean variable pointed to by DATA. */
> -
> -static bool
> -mark_modified (ao_ref *ao ATTRIBUTE_UNUSED, tree vdef ATTRIBUTE_UNUSED,
> - void *data)
> -{
> - bool *b = (bool *) data;
> - *b = true;
> - return true;
> -}
> -
> -/* Return true if the formal parameter PARM might have been modified in this
> - function before reaching the statement CALL. PARM_INFO is a pointer to a
> - structure containing intermediate information about PARM. */
> -
> -static bool
> -is_parm_modified_before_call (struct param_analysis_info *parm_info,
> - gimple call, tree parm)
> -{
> - bool modified = false;
> - ao_ref refd;
> -
> - if (parm_info->modified)
> - return true;
> -
> - ao_ref_init (&refd, parm);
> - walk_aliased_vdefs (&refd, gimple_vuse (call), mark_modified,
> - &modified, &parm_info->visited_statements);
> - if (modified)
> - {
> - parm_info->modified = true;
> - return true;
> - }
> - return false;
> -}
> -
> /* Go through arguments of the CALL and for every one that looks like a
> member
> pointer, check whether it can be safely declared pass-through and if so,
> mark that to the corresponding item of jump FUNCTIONS. Return true iff
> @@ -813,7 +902,7 @@ compute_pass_through_member_ptrs (struct
> int index = ipa_get_param_decl_index (info, arg);
>
> gcc_assert (index >=0);
> - if (!is_parm_modified_before_call (&parms_ainfo[index], call,
> + if (!is_parm_modified_before_stmt (&parms_ainfo[index], call,
> arg))
> {
> struct ipa_jump_func *jfunc = ipa_get_ith_jump_func (args,
> @@ -982,7 +1071,7 @@ ipa_compute_jump_functions_for_edge (str
> VEC_safe_grow_cleared (ipa_jump_func_t, gc, args->jump_functions, arg_num);
>
> /* We will deal with constants and SSA scalars first: */
> - compute_scalar_jump_functions (info, args, call);
> + compute_scalar_jump_functions (info, parms_ainfo, args, call);
>
> /* Let's check whether there are any potential member pointers and if so,
> whether we can determine their functions as pass_through. */
> @@ -1284,7 +1373,7 @@ ipa_analyze_indirect_call_uses (struct c
> return;
>
> index = ipa_get_param_decl_index (info, rec);
> - if (index >= 0 && !is_parm_modified_before_call (&parms_ainfo[index],
> + if (index >= 0 && !is_parm_modified_before_stmt (&parms_ainfo[index],
> call, rec))
> ipa_note_param_call (node, index, call);
>
>
