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?
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);
