RE: [PATCH] Do not run IPA transform phases multiple times

["Zamyatin, Igor" <igor dot zamyatin at intel dot com>]

Likely after this was checked in appeared following on x86

FAIL: gcc.dg/vect/vect-simd-clone-11.c -flto -ffat-lto-objects (internal compiler error)
FAIL: gcc.dg/vect/vect-simd-clone-11.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-simd-clone-12.c -flto -ffat-lto-objects (internal compiler error)
FAIL: gcc.dg/vect/vect-simd-clone-12.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-simd-clone-1.c -flto -ffat-lto-objects (internal compiler error)
FAIL: gcc.dg/vect/vect-simd-clone-1.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-simd-clone-2.c -flto -ffat-lto-objects (internal compiler error)
FAIL: gcc.dg/vect/vect-simd-clone-2.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-simd-clone-3.c -flto -ffat-lto-objects (internal compiler error)
FAIL: gcc.dg/vect/vect-simd-clone-3.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-simd-clone-4.c -flto -ffat-lto-objects (internal compiler error)
FAIL: gcc.dg/vect/vect-simd-clone-4.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-simd-clone-5.c -flto -ffat-lto-objects (internal compiler error)
FAIL: gcc.dg/vect/vect-simd-clone-5.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-simd-clone-6.c -flto -ffat-lto-objects (internal compiler error)
FAIL: gcc.dg/vect/vect-simd-clone-6.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-simd-clone-7.c -flto -ffat-lto-objects (internal compiler error)
FAIL: gcc.dg/vect/vect-simd-clone-7.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-simd-clone-8.c -flto -ffat-lto-objects (internal compiler error)
FAIL: gcc.dg/vect/vect-simd-clone-8.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-simd-clone-9.c -flto -ffat-lto-objects (internal compiler error)
FAIL: gcc.dg/vect/vect-simd-clone-9.c -flto -ffat-lto-objects (test for excess errors)



> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Martin Jambor
> Sent: Monday, April 14, 2014 9:37 PM
> To: GCC Patches
> Cc: Jan Hubicka
> Subject: [PATCH] Do not run IPA transform phases multiple times
> 
> Hi,
> 
> I have noticed that we often run IPA-transformation phases multiple times.
> The reason is that when a virtual clone is created, it starts with an empty
> ipa_transforms_to_apply vector and all subsequent IPA passes that see it are
> pushed onto it.  The original node gets these pushed on their
> ipa_transforms_to_apply too.  Later on, when a clone receives its body, in
> tree_function_versioning, the contents of original node's
> ipa_transforms_to_apply is added to clone's ipa_transforms_to_apply (in a
> slightly convoluted way).  I do not understand how this was supposed to
> work but obviously we can and do end up with multiple copies of a few
> passes in ipa_transforms_to_apply of the clone.
> 
> I believe the correct thing to do is to copy the vector when the virtual clone is
> created and remove the part in tree_function_versioning that does the
> copying.  I have verified that we no longer call transformation of IPA-CP
> multiple times when we previously did and added asserts to check that no
> other caller of tree_function_versioning expects to have the vector copied.
> 
> Bootstrapped and tested on x86_64-linux, OK for trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 2014-04-10  Martin Jambor  <mjambor@suse.cz>
> 
> 	* cgraphclones.c (cgraph_create_virtual_clone): Duplicate
> 	ipa_transforms_to_apply.
> 	(cgraph_function_versioning): Assert that old_node has empty
> 	ipa_transforms_to_apply.
> 	* trans-mem.c (ipa_tm_create_version): Likewise.
> 	* tree-inline.c (tree_function_versioning): Do not duplicate
> 	ipa_transforms_to_apply.
> 
> Index: src/gcc/cgraphclones.c
> ==========================================================
> =========
> --- src.orig/gcc/cgraphclones.c
> +++ src/gcc/cgraphclones.c
> @@ -600,6 +600,9 @@ cgraph_create_virtual_clone (struct cgra
>      }
>    else
>      new_node->clone.combined_args_to_skip = args_to_skip;
> +  if (old_node->ipa_transforms_to_apply.exists ())
> +    new_node->ipa_transforms_to_apply
> +      = old_node->ipa_transforms_to_apply.copy ();
> 
>    cgraph_call_node_duplication_hooks (old_node, new_node);
> 
> @@ -971,6 +974,7 @@ cgraph_function_versioning (struct cgrap
>      cgraph_copy_node_for_versioning (old_version_node, new_decl,
>  				     redirect_callers, bbs_to_copy);
> 
> +  gcc_assert (!old_version_node->ipa_transforms_to_apply.exists ());
>    /* Copy the OLD_VERSION_NODE function tree to the new version.  */
>    tree_function_versioning (old_decl, new_decl, tree_map, false,
> args_to_skip,
>  			    skip_return, bbs_to_copy, new_entry_block);
> Index: src/gcc/trans-mem.c
> ==========================================================
> =========
> --- src.orig/gcc/trans-mem.c
> +++ src/gcc/trans-mem.c
> @@ -4914,6 +4914,7 @@ ipa_tm_create_version (struct cgraph_nod
>    if (DECL_ONE_ONLY (new_decl))
>      DECL_COMDAT_GROUP (new_decl) = tm_mangle
> (DECL_COMDAT_GROUP (old_decl));
> 
> +  gcc_assert (!old_node->ipa_transforms_to_apply.exists ());
>    new_node = cgraph_copy_node_for_versioning (old_node, new_decl,
> vNULL, NULL);
>    new_node->local.local = false;
>    new_node->externally_visible = old_node->externally_visible;
> Index: src/gcc/tree-inline.c
> ==========================================================
> =========
> --- src.orig/gcc/tree-inline.c
> +++ src/gcc/tree-inline.c
> @@ -5323,18 +5323,6 @@ tree_function_versioning (tree old_decl,
>    id.dst_node = new_version_node;
>    id.src_cfun = DECL_STRUCT_FUNCTION (old_decl);
>    id.blocks_to_copy = blocks_to_copy;
> -  if (id.src_node->ipa_transforms_to_apply.exists ())
> -    {
> -      vec<ipa_opt_pass> old_transforms_to_apply
> -	    = id.dst_node->ipa_transforms_to_apply;
> -      unsigned int i;
> -
> -      id.dst_node->ipa_transforms_to_apply
> -	    = id.src_node->ipa_transforms_to_apply.copy ();
> -      for (i = 0; i < old_transforms_to_apply.length (); i++)
> -        id.dst_node->ipa_transforms_to_apply.safe_push
> (old_transforms_to_apply[i]);
> -      old_transforms_to_apply.release ();
> -    }
> 
>    id.copy_decl = copy_decl_no_change;
>    id.transform_call_graph_edges