Re: [PATCH] Copy over section name during cloning (PR ipa/64963)

[Jan Hubicka <hubicka at ucw dot cz>]

Hi,
> Hi!
> 
> As discussed in the PR, in 4.9 we used to clone DECL_SECTION_NAME
> through using copy_node on the FUNCTION_DECL, and only in selected places
> (e.g. when creating artificial_thunk.*, or when creating virtual clones
> of DECL_ONE_ONLY functions) we used to explicitly clear DECL_SECTION_NAME.
> In 5 the section name is stored in cgraph node instead, and thus not
> copied by default, so we instead need to copy it over to restore previous
> behavior, otherwise we break the Linux kernel and various other packages.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
> 
> 2015-02-16  Jakub Jelinek  <jakub@redhat.com>
> 	    James Greenhalgh  <james.greenhalgh@arm.com>
> 
> 	PR ipa/64963
> 	* cgraphclones.c (cgraph_node::create_virtual_clone): Copy
> 	section if not linkonce.  Fix up formatting.
> 	(cgraph_node::create_version_clone_with_body): Copy section.
> 	* trans-mem.c (ipa_tm_create_version): Likewise.
> 
> 	* gcc.dg/ipa/ipa-clone-1.c: New test.

Sorry, for taking so long on this. I made similar patch yesterday just did not
get around testing it. Imissed the trans-mem case though :)
> 
> --- gcc/cgraphclones.c.jj	2015-01-09 21:59:44.000000000 +0100
> +++ gcc/cgraphclones.c	2015-02-16 14:02:16.564725881 +0100
> @@ -577,7 +577,7 @@ cgraph_node::create_virtual_clone (vec<c
>    char *name;
>  
>    if (!in_lto_p)
> -    gcc_checking_assert  (tree_versionable_function_p (old_decl));
> +    gcc_checking_assert (tree_versionable_function_p (old_decl));
>  
>    gcc_assert (local.can_change_signature || !args_to_skip);
>  
> @@ -617,6 +617,8 @@ cgraph_node::create_virtual_clone (vec<c
>       ABI support for this.  */
>    set_new_clone_decl_and_node_flags (new_node);
>    new_node->clone.tree_map = tree_map;
> +  if (!DECL_ONE_ONLY (old_decl))

Instead of DECL_ONE_ONLY you want to test implicit_section flag.  I think
resolving unique section with -ffunction-section is also needed.

> +    new_node->set_section (this->get_section ());

No need for this->...

OK with these changes.
>  
>    /* Clones of global symbols or symbols with unique names are unique.  */
>    if ((TREE_PUBLIC (old_decl)
> @@ -1009,6 +1011,7 @@ cgraph_node::create_version_clone_with_b
>    new_version_node->externally_visible = 0;
>    new_version_node->local.local = 1;
>    new_version_node->lowered = true;
> +  new_version_node->set_section (this->get_section ());
>    /* Clones of global symbols or symbols with unique names are unique.  */
>    if ((TREE_PUBLIC (old_decl)
>         && !DECL_EXTERNAL (old_decl)
> --- gcc/trans-mem.c.jj	2015-01-14 09:55:19.000000000 +0100
> +++ gcc/trans-mem.c	2015-02-16 12:58:01.399808815 +0100
> @@ -4967,6 +4967,7 @@ ipa_tm_create_version (struct cgraph_nod
>    new_node->externally_visible = old_node->externally_visible;
>    new_node->lowered = true;
>    new_node->tm_clone = 1;
> +  new_node->set_section (old_node->get_section ());
>    get_cg_data (&old_node, true)->clone = new_node;
>  
>    if (old_node->get_availability () >= AVAIL_INTERPOSABLE)
> --- gcc/testsuite/gcc.dg/ipa/ipa-clone-1.c.jj	2015-02-16 14:14:39.041625503 +0100
> +++ gcc/testsuite/gcc.dg/ipa/ipa-clone-1.c	2015-02-16 14:15:31.944760949 +0100
> @@ -0,0 +1,20 @@
> +/* PR ipa/64693 */
> +/* { dg-do compile } */
> +/* { dg-require-named-sections "" } */
> +/* { dg-options "-O3 -fipa-cp -fipa-cp-clone -fdump-ipa-cp"  } */
> +
> +static int __attribute__ ((noinline, section ("test_section")))
> +foo (int arg)
> +{
> +  return 7 * arg;
> +}
> +
> +int
> +bar (int arg)
> +{
> +  return foo (5);
> +}
> +
> +/* { dg-final { scan-assembler "test_section" } } */
> +/* { dg-final { scan-ipa-dump "Creating a specialized node of foo" "cp" } } */
> +/* { dg-final { cleanup-ipa-dump "cp" } } */
> 
> 	Jakub