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: [middle-end, patch 2/8] Overhaul of modification analysis


> 2008-07-15  Martin Jambor  <mjambor@suse.cz>
> 
>         * ipa-cp.c (ipcp_initialize_node_lattices): Initialize lattices to
>         bottom if the function is not inlinable.
> 
> 	* 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.

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


> +/* 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
> +      || !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.
> +      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)


> @@ -464,8 +456,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;

>  
> +/* 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;

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.

Patch is OK if the DECL_SAVED_TREE and DECL_UNINLINABLE is resolved.

Honza


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