This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [middle-end, patch 2/8] Overhaul of modification analysis
Hi,
On Fri, Jul 18, 2008 at 12:13:41AM +0200, Jan Hubicka wrote:
> > 2008-07-15 Martin Jambor <mjambor@suse.cz>
> > * ipa-prop.h (struct ipa_param_flags): New structure.
> > (struct ipa_node_params): New field modification_analysis_done,
> > modified_flags changed into param_flags.
> > (ipa_is_ith_param_modified): Changed to use new flags.
>
> I am not completely happy about DECL_UNINLINABLE checks. THose are
> there since the inlinable functions are supposed to have already
> computed data, right?
> Can't we check the presense of summary rather than relying that the
> other part of code will check complementar conditional? I guess sooner
> or later someone will start modifying DECL_UNINLINABLE or modify the
> test and we get surprise.
>
> If DECL_UNINLINABLE is tested here to avoid problems with pending sizes
> as per ??? comment you are removing, I think it can go away.
> We no longer prevent inlining with pending sizes, so this test is wrong
> in every case. I believe in gimplifier form those are lesser evil now,
> so things should just work ;)
I don't know why they were there but I also did not have any
compelling reason to try removing them. Anyway, I just did exactly
that and am testing the new patch. We will see if anything breakes.
> >
> > - if (ipa_get_param_count (info) == 0 || info->modified_flags)
> > + if (ipa_get_param_count (info) == 0 || info->modification_analysis_done
> > + || !DECL_SAVED_TREE (decl))
>
> THe DECL_SAVED_TREE should be set for all analyzed nodes and for other
> nodes we should not even try to compute summary.
> Either check !node->analyzed here or remove the check if it is checked
> later.
OK, I have removed the check altogether, the analyzed flag is checked
alsewhere.
> > + for (bsi = bsi_start (bb); !bsi_end_p (bsi); bsi_next (&bsi))
> > + {
> > + stmt = bsi_stmt (bsi);
> > + ipa_check_stmt_modifications (info, stmt);
>
> This is looking just for non-SSA parameters, right? (otherwise you
> probably should track PHI nodes)
Yes, there is no point in doinf this for SSA.
> It might make more sense to use bitmap here, but since I guess all this
> absurdity is going away with followup cleanup sooner or later, it is OK
> as it is.
I'll start working on such a cleanup straight away.
>
> Patch is OK if the DECL_SAVED_TREE and DECL_UNINLINABLE is resolved.
>
Here is the updated version:
2008-07-21 Martin Jambor <mjambor@suse.cz>
* ipa-cp.c (ipcp_print_edge_profiles): Test for node->analyzed
rather than for DECL_SAVED_TREE.
* ipa-prop.c: Include diagnostic.h.
(ipa_check_stmt_modifications): Check LHS of GIMPLE_MODIFY_EXPRs
thoroughly.
(ipa_detect_param_modifications): Function rewritten from scratch.
(ipa_compute_jump_functions): Changed accesses to modification flags.
(ipa_free_node_params_substructures): Update flags destruction.
(ipa_node_duplication_hook): Update flags duplication.
(ipa_print_all_params_modified): Updated flag access.
* ipa-prop.h (struct ipa_param_flags): New structure.
(struct ipa_node_params): New field modification_analysis_done,
modified_flags changed into param_flags.
(ipa_is_ith_param_modified): Changed to use new flags.
* Makefile.in (ipa-prop.o): Add $(DIAGNOSTIC_H) to dependencies.
Index: iinln/gcc/ipa-cp.c
===================================================================
--- iinln.orig/gcc/ipa-cp.c
+++ iinln/gcc/ipa-cp.c
@@ -664,7 +664,7 @@ ipcp_print_edge_profiles (FILE * f)
for (node = cgraph_nodes; node; node = node->next)
{
fprintf (f, "function %s: \n", cgraph_node_name (node));
- if (DECL_SAVED_TREE (node->decl))
+ if (node->analyzed)
{
bb =
ENTRY_BLOCK_PTR_FOR_FUNCTION (DECL_STRUCT_FUNCTION (node->decl));
Index: iinln/gcc/ipa-prop.c
===================================================================
--- iinln.orig/gcc/ipa-prop.c
+++ iinln/gcc/ipa-prop.c
@@ -32,6 +32,7 @@ along with GCC; see the file COPYING3.
#include "flags.h"
#include "timevar.h"
#include "flags.h"
+#include "diagnostic.h"
/* Vector where the parameter infos are actually stored. */
VEC (ipa_node_params_t, heap) *ipa_node_params_vector;
@@ -146,95 +147,84 @@ ipa_count_formal_params (struct cgraph_n
ipa_set_param_count (IPA_NODE_REF (mt), param_num);
}
-/* Check STMT to detect whether a formal is modified within MT, the appropriate
- entry is updated in the modified_flags array of ipa_node_params (associated
- with MT). */
+/* Check STMT to detect whether a formal parameter is directly modified within
+ STMT, the appropriate entry is updated in the modified flags of INFO.
+ Directly means that this function does not check for modifications through
+ pointers or escaping addresses because all TREE_ADDRESSABLE parameters are
+ considered modified anyway. */
static void
-ipa_check_stmt_modifications (struct cgraph_node *mt, tree stmt)
+ipa_check_stmt_modifications (struct ipa_node_params *info, tree stmt)
{
- int index, j;
- tree parm_decl;
- struct ipa_node_params *info;
+ int j;
+ int index;
+ tree lhs;
switch (TREE_CODE (stmt))
{
case GIMPLE_MODIFY_STMT:
- if (TREE_CODE (GIMPLE_STMT_OPERAND (stmt, 0)) == PARM_DECL)
- {
- info = IPA_NODE_REF (mt);
- parm_decl = GIMPLE_STMT_OPERAND (stmt, 0);
- index = ipa_get_param_decl_index (info, parm_decl);
- if (index >= 0)
- info->modified_flags[index] = true;
- }
+ lhs = GIMPLE_STMT_OPERAND (stmt, 0);
+
+ while (handled_component_p (lhs))
+ lhs = TREE_OPERAND (lhs, 0);
+ if (TREE_CODE (lhs) == SSA_NAME)
+ lhs = SSA_NAME_VAR (lhs);
+ index = ipa_get_param_decl_index (info, lhs);
+ if (index >= 0)
+ info->param_flags[index].modified = true;
break;
+
case ASM_EXPR:
/* Asm code could modify any of the parameters. */
- info = IPA_NODE_REF (mt);
- for (j = 0; j < ipa_get_param_count (IPA_NODE_REF (mt)); j++)
- info->modified_flags[j] = true;
+ for (j = 0; j < ipa_get_param_count (info); j++)
+ info->param_flags[j].modified = true;
break;
+
default:
break;
}
}
-/* The modify computation driver for MT. Compute which formal arguments
- of function MT are locally modified. Formals may be modified in MT
- if their address is taken, or if
- they appear on the left hand side of an assignment. */
-void
-ipa_detect_param_modifications (struct cgraph_node *mt)
-{
- tree decl;
- tree body;
- int j, count;
+/* Compute which formal parameters of function associated with NODE are locally
+ modified. Parameters may be modified in NODE if they are TREE_ADDRESSABLE,
+ if they appear on the left hand side of an assignment or if there is an
+ ASM_EXPR in the function. */
+void
+ipa_detect_param_modifications (struct cgraph_node *node)
+{
+ tree decl = node->decl;
basic_block bb;
struct function *func;
block_stmt_iterator bsi;
- tree stmt, parm_tree;
- struct ipa_node_params *info = IPA_NODE_REF (mt);
+ tree stmt;
+ struct ipa_node_params *info = IPA_NODE_REF (node);
+ int i, count;
- if (ipa_get_param_count (info) == 0 || info->modified_flags)
+ if (ipa_get_param_count (info) == 0 || info->modification_analysis_done)
return;
- count = ipa_get_param_count (info);
- info->modified_flags = XCNEWVEC (bool, count);
- decl = mt->decl;
- /* ??? Handle pending sizes case. Set all parameters
- of the function to be modified. */
+ if (!info->param_flags)
+ info->param_flags = XCNEWVEC (struct ipa_param_flags,
+ ipa_get_param_count (info));
- if (DECL_UNINLINABLE (decl))
+ func = DECL_STRUCT_FUNCTION (decl);
+ FOR_EACH_BB_FN (bb, func)
{
- for (j = 0; j < count; j++)
- info->modified_flags[j] = true;
-
- return;
- }
- /* Formals whose address is taken are considered modified. */
- for (j = 0; j < count; j++)
- {
- parm_tree = ipa_get_ith_param (info, j);
- if (!is_gimple_reg (parm_tree)
- && TREE_ADDRESSABLE (parm_tree))
- info->modified_flags[j] = true;
- }
- body = DECL_SAVED_TREE (decl);
- if (body != NULL)
- {
- func = DECL_STRUCT_FUNCTION (decl);
- FOR_EACH_BB_FN (bb, func)
- {
- for (bsi = bsi_start (bb); !bsi_end_p (bsi); bsi_next (&bsi))
- {
- stmt = bsi_stmt (bsi);
- ipa_check_stmt_modifications (mt, stmt);
- }
- }
+ for (bsi = bsi_start (bb); !bsi_end_p (bsi); bsi_next (&bsi))
+ {
+ stmt = bsi_stmt (bsi);
+ ipa_check_stmt_modifications (info, stmt);
+ }
}
+
+ count = ipa_get_param_count (info);
+ for (i = 0; i < count; i++)
+ if (TREE_ADDRESSABLE (ipa_get_ith_param (info, i)))
+ info->param_flags[i].modified = true;
+
+ info->modification_analysis_done = 1;
}
-/* Count number of arguments callsite CS has and store it in
+/* Count number of arguments callsite CS has and store it in
ipa_edge_args structure corresponding to this callsite. */
void
ipa_count_arguments (struct cgraph_edge *cs)
@@ -291,11 +281,12 @@ ipa_compute_jump_functions (struct cgrap
if (TREE_CODE (arg) == SSA_NAME && IS_VALID_JUMP_FUNC_INDEX (index))
{
curr_cfun = DECL_STRUCT_FUNCTION (mt->decl);
- if (!gimple_default_def (curr_cfun, parm_decl)
+ if (!gimple_default_def (curr_cfun, parm_decl)
|| gimple_default_def (curr_cfun, parm_decl) != arg)
- info->modified_flags[index] = true;
+ info->param_flags[index].modified = true;
}
- if (!IS_VALID_JUMP_FUNC_INDEX (index) || info->modified_flags[index])
+ if (!IS_VALID_JUMP_FUNC_INDEX (index)
+ || ipa_is_ith_param_modified (info, index))
args->jump_functions[arg_num].type = IPA_UNKNOWN;
else
{
@@ -371,8 +362,8 @@ ipa_free_node_params_substructures (stru
free (info->ipcp_lattices);
if (info->param_decls)
free (info->param_decls);
- if (info->modified_flags)
- free (info->modified_flags);
+ if (info->param_flags)
+ free (info->param_flags);
memset (info, 0, sizeof (*info));
}
@@ -464,8 +455,9 @@ ipa_node_duplication_hook (struct cgraph
sizeof (struct ipcp_lattice) * param_count);
new_info->param_decls = (tree *)
duplicate_array (old_info->param_decls, sizeof (tree) * param_count);
- new_info->modified_flags = (bool *)
- duplicate_array (old_info->modified_flags, sizeof (bool) * param_count);
+ new_info->param_flags = (struct ipa_param_flags *)
+ duplicate_array (old_info->param_flags,
+ sizeof (struct ipa_param_flags) * param_count);
new_info->ipcp_orig_node = old_info->ipcp_orig_node;
new_info->count_scale = old_info->count_scale;
@@ -566,7 +558,7 @@ ipa_print_all_params_modified (FILE * f)
count = ipa_get_param_count (info);
for (i = 0; i < count; i++)
{
- temp = info->modified_flags[i];
+ temp = info->param_flags[i].modified;
if (temp)
fprintf (f, " param [%d] true \n", i);
else
Index: iinln/gcc/ipa-prop.h
===================================================================
--- iinln.orig/gcc/ipa-prop.h
+++ iinln/gcc/ipa-prop.h
@@ -101,6 +101,15 @@ struct ipa_replace_map
bool ref_p;
};
+/* ipa_param_flags contains various flags that describe how the associated
+ parameter is treated within a function. */
+struct ipa_param_flags
+{
+ /* Whether the value parameter has been modified within the function. */
+ unsigned modified : 1;
+
+};
+
/* ipa_node_params stores information related to formal parameters of functions
and some other information for interprocedural passes that operate on
parameters (such as ipa-cp). */
@@ -115,8 +124,8 @@ struct ipa_node_params
struct ipcp_lattice *ipcp_lattices;
/* Mapping each parameter to its PARM_DECL tree. */
tree *param_decls;
- /* Indicating which parameter is modified in its function. */
- bool *modified_flags;
+ /* Various flags describing individual parameters. */
+ struct ipa_param_flags *param_flags;
/* Only for versioned nodes this field would not be NULL,
it points to the node that IPA cp cloned from. */
struct cgraph_node *ipcp_orig_node;
@@ -130,6 +139,8 @@ struct ipa_node_params
/* Whether this function is called with variable number of actual
arguments. */
unsigned called_with_var_arguments : 1;
+ /* Whether the modification analysis has already been performed. */
+ unsigned modification_analysis_done : 1;
};
/* ipa_node_params access functions. Please use these to access fields that
@@ -164,7 +175,7 @@ ipa_get_ith_param (struct ipa_node_param
static inline bool
ipa_is_ith_param_modified (struct ipa_node_params *info, int i)
{
- return info->modified_flags[i];
+ return info->param_flags[i].modified;
}
/* Flag this node as having callers with variable number of arguments. */
Index: iinln/gcc/Makefile.in
===================================================================
--- iinln.orig/gcc/Makefile.in
+++ iinln/gcc/Makefile.in
@@ -2563,7 +2563,7 @@ varpool.o : varpool.c $(CONFIG_H) $(SYST
ipa.o : ipa.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(CGRAPH_H) \
tree-pass.h $(TIMEVAR_H)
ipa-prop.o : ipa-prop.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
- langhooks.h $(GGC_H) $(TARGET_H) $(CGRAPH_H) $(IPA_PROP_H) \
+ langhooks.h $(GGC_H) $(TARGET_H) $(CGRAPH_H) $(IPA_PROP_H) $(DIAGNOSTIC_H) \
$(TREE_FLOW_H) $(TM_H) tree-pass.h $(FLAGS_H) $(TREE_H) $(TREE_INLINE_H) \
$(TIMEVAR_H)
ipa-cp.o : ipa-cp.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \