This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Pass through jump functions for addressable (scalar) parameters


On Wed, Oct 26, 2011 at 8:25 PM, Martin Jambor <mjambor@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 ?<mjambor@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);
>
>
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]