On Wed, Oct 26, 2011 at 8:25 PM, Martin Jambor <mjam...@suse.cz> wrote:
> Hi,
>
> Honza requested that we do attempt to produce pass-through jump
> functions even when the actual formal parameter that is being passed
> on is addressable - provided that we can prove it has not changed
> value, of course. The following patch does this, hopefully the
> limitations of our parameter-not-modified mechanism are not too strict
> for this purpose.
>
> The patch is supposed to be applied on top of the one I have just sent
> that renames all parm_infos to parm_ainfos. I have successfully
> bootstrapped and tested it on x86_64-linux.
>
> OK for trunk?
>
> Thanks,
>
> Martin
>
>
>
> 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/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,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?
> + 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 ...
> + 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).
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?
> + 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.
> + && !detect_type_change_ssa (tc_ssa, call, jfunc))
> {
> jfunc->type = IPA_JF_PASS_THROUGH;
> jfunc->value.pass_through.formal_id = index;
> @@ -665,12 +740,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 +782,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 +826,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 +854,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 +1023,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 +1325,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);
>
>
>
