Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Dominique Dhumieres]

r210901 breaks bootstrap on targets not supporting strnlen, e.g., darwin10.

../../_clean/gcc/lto-cgraph.c:976:68: error: 'strnlen' was not declared in this scope

libgfortran/configure defines HAVE_STRNLEN on targets supporting it.

The same revision also breaks the test g++.dg/tls/diag-1.C

/opt/gcc/work/gcc/testsuite/g++.dg/tls/diag-1.C:31:1: internal compiler error: in symtab_get_node, at cgraph.h:1021

TIA,

Dominique

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Andrew Pinski]

On Mon, May 26, 2014 at 1:59 AM, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
> r210901 breaks bootstrap on targets not supporting strnlen, e.g., darwin10.
>
> ../../_clean/gcc/lto-cgraph.c:976:68: error: 'strnlen' was not declared in this scope

strnlen should be declared in include/libiberty.h if there is no
declaration as libiberty support is already there.  That should be a
simple fix.

Thanks,
Andrew Pinski

>
> libgfortran/configure defines HAVE_STRNLEN on targets supporting it.
>
> The same revision also breaks the test g++.dg/tls/diag-1.C
>
> /opt/gcc/work/gcc/testsuite/g++.dg/tls/diag-1.C:31:1: internal compiler error: in symtab_get_node, at cgraph.h:1021
>
> TIA,
>
> Dominique

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Christophe Lyon]

On my side, I can see that r210901 breaks AArch64 compiler build:
gcc/config/aarch64/aarch64.c: In function ‘void
aarch64_elf_asm_named_section(const char*, unsigned int, tree_node*)’:
gcc/config/aarch64/aarch64.c:8136: error: ‘decl_comdat_group’ was not
declared in this scope


Christophe.


On 26 May 2014 11:16, Andrew Pinski <pinskia@gmail.com> wrote:
> On Mon, May 26, 2014 at 1:59 AM, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
>> r210901 breaks bootstrap on targets not supporting strnlen, e.g., darwin10.
>>
>> ../../_clean/gcc/lto-cgraph.c:976:68: error: 'strnlen' was not declared in this scope
>
> strnlen should be declared in include/libiberty.h if there is no
> declaration as libiberty support is already there.  That should be a
> simple fix.
>
> Thanks,
> Andrew Pinski
>
>>
>> libgfortran/configure defines HAVE_STRNLEN on targets supporting it.
>>
>> The same revision also breaks the test g++.dg/tls/diag-1.C
>>
>> /opt/gcc/work/gcc/testsuite/g++.dg/tls/diag-1.C:31:1: internal compiler error: in symtab_get_node, at cgraph.h:1021
>>
>> TIA,
>>
>> Dominique

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Jan Hubicka]

> On my side, I can see that r210901 breaks AArch64 compiler build:
> gcc/config/aarch64/aarch64.c: In function ‘void
> aarch64_elf_asm_named_section(const char*, unsigned int, tree_node*)’:
> gcc/config/aarch64/aarch64.c:8136: error: ‘decl_comdat_group’ was not
> declared in this scope

This sould be fixed by adding #include "cgraph.h" into aarch64.c
I will look into the strnlen issue.

Honza

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

Hi!

On Mon, 26 May 2014 02:16:35 -0700, Andrew Pinski <pinskia@gmail.com> wrote:
> On Mon, May 26, 2014 at 1:59 AM, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
> > r210901 breaks bootstrap on targets not supporting strnlen, e.g., darwin10.
> >
> > ../../_clean/gcc/lto-cgraph.c:976:68: error: 'strnlen' was not declared in this scope

I'm seeing the same on MinGW, which also doesn't have strnlen (which is a
GNU extension).

> strnlen should be declared in include/libiberty.h if there is no
> declaration as libiberty support is already there.  That should be a
> simple fix.

Like this?

--- gcc/config.in
+++ gcc/config.in
[Regenerate.]
--- gcc/configure
+++ gcc/configure
[Regenerate.]
--- gcc/configure.ac
+++ gcc/configure.ac
@@ -1136,7 +1136,7 @@ CFLAGS="$CFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
 saved_CXXFLAGS="$CXXFLAGS"
 CXXFLAGS="$CXXFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
 gcc_AC_CHECK_DECLS(getenv atol asprintf sbrk abort atof getcwd getwd \
-	strsignal strstr stpcpy strverscmp \
+	stpcpy strnlen strsignal strstr strverscmp \
 	errno snprintf vsnprintf vasprintf malloc realloc calloc \
 	free basename getopt clock getpagesize ffs gcc_UNLOCKED_FUNCS, , ,[
 #include "ansidecl.h"
diff --git include/libiberty.h include/libiberty.h
index 7fd0703..56b8b43 100644
--- include/libiberty.h
+++ include/libiberty.h
@@ -636,6 +636,10 @@ extern int snprintf (char *, size_t, const char *, ...) ATTRIBUTE_PRINTF_3;
 extern int vsnprintf (char *, size_t, const char *, va_list) ATTRIBUTE_PRINTF(3,0);
 #endif
 
+#if defined (HAVE_DECL_STRNLEN) && !HAVE_DECL_STRNLEN
+extern size_t strnlen (const char *, size_t);
+#endif
+
 #if defined(HAVE_DECL_STRVERSCMP) && !HAVE_DECL_STRVERSCMP
 /* Compare version strings.  */
 extern int strverscmp (const char *, const char *);


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Jan Hubicka]

> Hi!
> 
> On Mon, 26 May 2014 02:16:35 -0700, Andrew Pinski <pinskia@gmail.com> wrote:
> > On Mon, May 26, 2014 at 1:59 AM, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
> > > r210901 breaks bootstrap on targets not supporting strnlen, e.g., darwin10.
> > >
> > > ../../_clean/gcc/lto-cgraph.c:976:68: error: 'strnlen' was not declared in this scope
> 
> I'm seeing the same on MinGW, which also doesn't have strnlen (which is a
> GNU extension).
> 
> > strnlen should be declared in include/libiberty.h if there is no
> > declaration as libiberty support is already there.  That should be a
> > simple fix.
> 
> Like this?

This looks good to me (thoguh strnlen is posix).  I can not approve the patch
but I would preffer it over just hand implementing strnlen there (that is easy,
too)

Honza
> 
> --- gcc/config.in
> +++ gcc/config.in
> [Regenerate.]
> --- gcc/configure
> +++ gcc/configure
> [Regenerate.]
> --- gcc/configure.ac
> +++ gcc/configure.ac
> @@ -1136,7 +1136,7 @@ CFLAGS="$CFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
>  saved_CXXFLAGS="$CXXFLAGS"
>  CXXFLAGS="$CXXFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
>  gcc_AC_CHECK_DECLS(getenv atol asprintf sbrk abort atof getcwd getwd \
> -	strsignal strstr stpcpy strverscmp \
> +	stpcpy strnlen strsignal strstr strverscmp \
>  	errno snprintf vsnprintf vasprintf malloc realloc calloc \
>  	free basename getopt clock getpagesize ffs gcc_UNLOCKED_FUNCS, , ,[
>  #include "ansidecl.h"
> diff --git include/libiberty.h include/libiberty.h
> index 7fd0703..56b8b43 100644
> --- include/libiberty.h
> +++ include/libiberty.h
> @@ -636,6 +636,10 @@ extern int snprintf (char *, size_t, const char *, ...) ATTRIBUTE_PRINTF_3;
>  extern int vsnprintf (char *, size_t, const char *, va_list) ATTRIBUTE_PRINTF(3,0);
>  #endif
>  
> +#if defined (HAVE_DECL_STRNLEN) && !HAVE_DECL_STRNLEN
> +extern size_t strnlen (const char *, size_t);
> +#endif
> +
>  #if defined(HAVE_DECL_STRVERSCMP) && !HAVE_DECL_STRVERSCMP
>  /* Compare version strings.  */
>  extern int strverscmp (const char *, const char *);
> 
> 
> GrĂźĂŸe,
>  Thomas

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 2422 bytes --]

Hi!

Ping -- OK to commit to trunk?

On Wed, 28 May 2014 23:55:31 +0200, Jan Hubicka <hubicka@ucw.cz> wrote:
> > On Mon, 26 May 2014 02:16:35 -0700, Andrew Pinski <pinskia@gmail.com> wrote:
> > > On Mon, May 26, 2014 at 1:59 AM, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
> > > > r210901 breaks bootstrap on targets not supporting strnlen, e.g., darwin10.
> > > >
> > > > ../../_clean/gcc/lto-cgraph.c:976:68: error: 'strnlen' was not declared in this scope
> > 
> > I'm seeing the same on MinGW, which also doesn't have strnlen (which is a
> > GNU extension).
> > 
> > > strnlen should be declared in include/libiberty.h if there is no
> > > declaration as libiberty support is already there.  That should be a
> > > simple fix.
> > 
> > Like this?
> 
> This looks good to me (thoguh strnlen is posix).  I can not approve the patch
> but I would preffer it over just hand implementing strnlen there (that is easy,
> too)

Patch is also considered good by testers in
<https://gcc.gnu.org/PR61334>.

> > --- gcc/config.in
> > +++ gcc/config.in
> > [Regenerate.]
> > --- gcc/configure
> > +++ gcc/configure
> > [Regenerate.]
> > --- gcc/configure.ac
> > +++ gcc/configure.ac
> > @@ -1136,7 +1136,7 @@ CFLAGS="$CFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
> >  saved_CXXFLAGS="$CXXFLAGS"
> >  CXXFLAGS="$CXXFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
> >  gcc_AC_CHECK_DECLS(getenv atol asprintf sbrk abort atof getcwd getwd \
> > -	strsignal strstr stpcpy strverscmp \
> > +	stpcpy strnlen strsignal strstr strverscmp \
> >  	errno snprintf vsnprintf vasprintf malloc realloc calloc \
> >  	free basename getopt clock getpagesize ffs gcc_UNLOCKED_FUNCS, , ,[
> >  #include "ansidecl.h"
> > diff --git include/libiberty.h include/libiberty.h
> > index 7fd0703..56b8b43 100644
> > --- include/libiberty.h
> > +++ include/libiberty.h
> > @@ -636,6 +636,10 @@ extern int snprintf (char *, size_t, const char *, ...) ATTRIBUTE_PRINTF_3;
> >  extern int vsnprintf (char *, size_t, const char *, va_list) ATTRIBUTE_PRINTF(3,0);
> >  #endif
> >  
> > +#if defined (HAVE_DECL_STRNLEN) && !HAVE_DECL_STRNLEN
> > +extern size_t strnlen (const char *, size_t);
> > +#endif
> > +
> >  #if defined(HAVE_DECL_STRVERSCMP) && !HAVE_DECL_STRVERSCMP
> >  /* Compare version strings.  */
> >  extern int strverscmp (const char *, const char *);


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

Commit policy? Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 2941 bytes --]

Hi!

On Tue, 3 Jun 2014 11:55:44 +0200, I wrote:
> Ping -- OK to commit to trunk?

Even though several of those who I'd consider regular GCC developers do
agree with this patch (see also <https://gcc.gnu.org/PR61334>), that is
not enough "consensus" to consider this patch approved, right?  Wouldn't
it be a good idea for GCC to move to a more "liberal" policy in this
regard?  (That seems to work nicely for glibc.)


Ping.


> On Wed, 28 May 2014 23:55:31 +0200, Jan Hubicka <hubicka@ucw.cz> wrote:
> > > On Mon, 26 May 2014 02:16:35 -0700, Andrew Pinski <pinskia@gmail.com> wrote:
> > > > On Mon, May 26, 2014 at 1:59 AM, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
> > > > > r210901 breaks bootstrap on targets not supporting strnlen, e.g., darwin10.
> > > > >
> > > > > ../../_clean/gcc/lto-cgraph.c:976:68: error: 'strnlen' was not declared in this scope
> > > 
> > > I'm seeing the same on MinGW, which also doesn't have strnlen (which is a
> > > GNU extension).
> > > 
> > > > strnlen should be declared in include/libiberty.h if there is no
> > > > declaration as libiberty support is already there.  That should be a
> > > > simple fix.
> > > 
> > > Like this?
> > 
> > This looks good to me (thoguh strnlen is posix).  I can not approve the patch
> > but I would preffer it over just hand implementing strnlen there (that is easy,
> > too)
> 
> Patch is also considered good by testers in
> <https://gcc.gnu.org/PR61334>.
> 
> > > --- gcc/config.in
> > > +++ gcc/config.in
> > > [Regenerate.]
> > > --- gcc/configure
> > > +++ gcc/configure
> > > [Regenerate.]
> > > --- gcc/configure.ac
> > > +++ gcc/configure.ac
> > > @@ -1136,7 +1136,7 @@ CFLAGS="$CFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
> > >  saved_CXXFLAGS="$CXXFLAGS"
> > >  CXXFLAGS="$CXXFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
> > >  gcc_AC_CHECK_DECLS(getenv atol asprintf sbrk abort atof getcwd getwd \
> > > -	strsignal strstr stpcpy strverscmp \
> > > +	stpcpy strnlen strsignal strstr strverscmp \
> > >  	errno snprintf vsnprintf vasprintf malloc realloc calloc \
> > >  	free basename getopt clock getpagesize ffs gcc_UNLOCKED_FUNCS, , ,[
> > >  #include "ansidecl.h"
> > > diff --git include/libiberty.h include/libiberty.h
> > > index 7fd0703..56b8b43 100644
> > > --- include/libiberty.h
> > > +++ include/libiberty.h
> > > @@ -636,6 +636,10 @@ extern int snprintf (char *, size_t, const char *, ...) ATTRIBUTE_PRINTF_3;
> > >  extern int vsnprintf (char *, size_t, const char *, va_list) ATTRIBUTE_PRINTF(3,0);
> > >  #endif
> > >  
> > > +#if defined (HAVE_DECL_STRNLEN) && !HAVE_DECL_STRNLEN
> > > +extern size_t strnlen (const char *, size_t);
> > > +#endif
> > > +
> > >  #if defined(HAVE_DECL_STRVERSCMP) && !HAVE_DECL_STRVERSCMP
> > >  /* Compare version strings.  */
> > >  extern int strverscmp (const char *, const char *);
> 
> 
> Grüße,
>  Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

Re: Commit policy? Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Andrew Pinski]

On Mon, Jun 9, 2014 at 11:47 PM, Thomas Schwinge
<thomas@codesourcery.com> wrote:
> Hi!
>
> On Tue, 3 Jun 2014 11:55:44 +0200, I wrote:
>> Ping -- OK to commit to trunk?
>
> Even though several of those who I'd consider regular GCC developers do
> agree with this patch (see also <https://gcc.gnu.org/PR61334>), that is
> not enough "consensus" to consider this patch approved, right?  Wouldn't
> it be a good idea for GCC to move to a more "liberal" policy in this
> regard?  (That seems to work nicely for glibc.)

Really I think this is an obvious patch and would be the exact same
patch I would have came up with.

Thanks,
Andrew Pinski

>
>
> Ping.
>
>
>> On Wed, 28 May 2014 23:55:31 +0200, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > > On Mon, 26 May 2014 02:16:35 -0700, Andrew Pinski <pinskia@gmail.com> wrote:
>> > > > On Mon, May 26, 2014 at 1:59 AM, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
>> > > > > r210901 breaks bootstrap on targets not supporting strnlen, e.g., darwin10.
>> > > > >
>> > > > > ../../_clean/gcc/lto-cgraph.c:976:68: error: 'strnlen' was not declared in this scope
>> > >
>> > > I'm seeing the same on MinGW, which also doesn't have strnlen (which is a
>> > > GNU extension).
>> > >
>> > > > strnlen should be declared in include/libiberty.h if there is no
>> > > > declaration as libiberty support is already there.  That should be a
>> > > > simple fix.
>> > >
>> > > Like this?
>> >
>> > This looks good to me (thoguh strnlen is posix).  I can not approve the patch
>> > but I would preffer it over just hand implementing strnlen there (that is easy,
>> > too)
>>
>> Patch is also considered good by testers in
>> <https://gcc.gnu.org/PR61334>.
>>
>> > > --- gcc/config.in
>> > > +++ gcc/config.in
>> > > [Regenerate.]
>> > > --- gcc/configure
>> > > +++ gcc/configure
>> > > [Regenerate.]
>> > > --- gcc/configure.ac
>> > > +++ gcc/configure.ac
>> > > @@ -1136,7 +1136,7 @@ CFLAGS="$CFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
>> > >  saved_CXXFLAGS="$CXXFLAGS"
>> > >  CXXFLAGS="$CXXFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
>> > >  gcc_AC_CHECK_DECLS(getenv atol asprintf sbrk abort atof getcwd getwd \
>> > > - strsignal strstr stpcpy strverscmp \
>> > > + stpcpy strnlen strsignal strstr strverscmp \
>> > >   errno snprintf vsnprintf vasprintf malloc realloc calloc \
>> > >   free basename getopt clock getpagesize ffs gcc_UNLOCKED_FUNCS, , ,[
>> > >  #include "ansidecl.h"
>> > > diff --git include/libiberty.h include/libiberty.h
>> > > index 7fd0703..56b8b43 100644
>> > > --- include/libiberty.h
>> > > +++ include/libiberty.h
>> > > @@ -636,6 +636,10 @@ extern int snprintf (char *, size_t, const char *, ...) ATTRIBUTE_PRINTF_3;
>> > >  extern int vsnprintf (char *, size_t, const char *, va_list) ATTRIBUTE_PRINTF(3,0);
>> > >  #endif
>> > >
>> > > +#if defined (HAVE_DECL_STRNLEN) && !HAVE_DECL_STRNLEN
>> > > +extern size_t strnlen (const char *, size_t);
>> > > +#endif
>> > > +
>> > >  #if defined(HAVE_DECL_STRVERSCMP) && !HAVE_DECL_STRVERSCMP
>> > >  /* Compare version strings.  */
>> > >  extern int strverscmp (const char *, const char *);
>>
>>
>> Grüße,
>>  Thomas

Re: Commit policy? Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Richard Biener]

On Tue, Jun 10, 2014 at 8:47 AM, Thomas Schwinge
<thomas@codesourcery.com> wrote:
> Hi!
>
> On Tue, 3 Jun 2014 11:55:44 +0200, I wrote:
>> Ping -- OK to commit to trunk?
>
> Even though several of those who I'd consider regular GCC developers do
> agree with this patch (see also <https://gcc.gnu.org/PR61334>), that is
> not enough "consensus" to consider this patch approved, right?  Wouldn't
> it be a good idea for GCC to move to a more "liberal" policy in this
> regard?  (That seems to work nicely for glibc.)

Ok.

Thanks,
Richard.

>
> Ping.
>
>
>> On Wed, 28 May 2014 23:55:31 +0200, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > > On Mon, 26 May 2014 02:16:35 -0700, Andrew Pinski <pinskia@gmail.com> wrote:
>> > > > On Mon, May 26, 2014 at 1:59 AM, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
>> > > > > r210901 breaks bootstrap on targets not supporting strnlen, e.g., darwin10.
>> > > > >
>> > > > > ../../_clean/gcc/lto-cgraph.c:976:68: error: 'strnlen' was not declared in this scope
>> > >
>> > > I'm seeing the same on MinGW, which also doesn't have strnlen (which is a
>> > > GNU extension).
>> > >
>> > > > strnlen should be declared in include/libiberty.h if there is no
>> > > > declaration as libiberty support is already there.  That should be a
>> > > > simple fix.
>> > >
>> > > Like this?
>> >
>> > This looks good to me (thoguh strnlen is posix).  I can not approve the patch
>> > but I would preffer it over just hand implementing strnlen there (that is easy,
>> > too)
>>
>> Patch is also considered good by testers in
>> <https://gcc.gnu.org/PR61334>.
>>
>> > > --- gcc/config.in
>> > > +++ gcc/config.in
>> > > [Regenerate.]
>> > > --- gcc/configure
>> > > +++ gcc/configure
>> > > [Regenerate.]
>> > > --- gcc/configure.ac
>> > > +++ gcc/configure.ac
>> > > @@ -1136,7 +1136,7 @@ CFLAGS="$CFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
>> > >  saved_CXXFLAGS="$CXXFLAGS"
>> > >  CXXFLAGS="$CXXFLAGS -I${srcdir} -I${srcdir}/../include $GMPINC"
>> > >  gcc_AC_CHECK_DECLS(getenv atol asprintf sbrk abort atof getcwd getwd \
>> > > - strsignal strstr stpcpy strverscmp \
>> > > + stpcpy strnlen strsignal strstr strverscmp \
>> > >   errno snprintf vsnprintf vasprintf malloc realloc calloc \
>> > >   free basename getopt clock getpagesize ffs gcc_UNLOCKED_FUNCS, , ,[
>> > >  #include "ansidecl.h"
>> > > diff --git include/libiberty.h include/libiberty.h
>> > > index 7fd0703..56b8b43 100644
>> > > --- include/libiberty.h
>> > > +++ include/libiberty.h
>> > > @@ -636,6 +636,10 @@ extern int snprintf (char *, size_t, const char *, ...) ATTRIBUTE_PRINTF_3;
>> > >  extern int vsnprintf (char *, size_t, const char *, va_list) ATTRIBUTE_PRINTF(3,0);
>> > >  #endif
>> > >
>> > > +#if defined (HAVE_DECL_STRNLEN) && !HAVE_DECL_STRNLEN
>> > > +extern size_t strnlen (const char *, size_t);
>> > > +#endif
>> > > +
>> > >  #if defined(HAVE_DECL_STRVERSCMP) && !HAVE_DECL_STRVERSCMP
>> > >  /* Compare version strings.  */
>> > >  extern int strverscmp (const char *, const char *);
>>
>>
>> Grüße,
>>  Thomas

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Rainer Orth]

Jan Hubicka <hubicka@ucw.cz> writes:

>> This patch broke Solaris bootstrap:
>> 
>> /vol/gcc/src/hg/trunk/local/gcc/config/sol2.c: In function 'void solaris_elf_asm_comdat_section(const char*, unsigned int, tree)':
>> /vol/gcc/src/hg/trunk/local/gcc/config/sol2.c:213:17: error: 'decl_comdat_group' was not declared in this scope
>> /vol/gcc/src/hg/trunk/local/gcc/config/sol2.c: In function 'int solaris_define_comdat_signature(comdat_entry**, void*)':
>> /vol/gcc/src/hg/trunk/local/gcc/config/sol2.c:267:12: error: 'decl_comdat_group' was not declared in this scope
>> 
>> The following snippet allows a sparc-sun-solaris2.11 bootstrap to go
>> along further, only to break again later in libjava for what seems to be
>> unrelated reasons.
>> 
>> 2014-05-26  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
>> 
>> 	* config/sol2.c: Include cgraph.h.
>
> I moved decl_comdat_group offline that should fix the problem. Does it work now?

It does indeed.

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Jan Hubicka]

> 
> This patch broke Solaris bootstrap:
> 
> /vol/gcc/src/hg/trunk/local/gcc/config/sol2.c: In function 'void solaris_elf_asm_comdat_section(const char*, unsigned int, tree)':
> /vol/gcc/src/hg/trunk/local/gcc/config/sol2.c:213:17: error: 'decl_comdat_group' was not declared in this scope
> /vol/gcc/src/hg/trunk/local/gcc/config/sol2.c: In function 'int solaris_define_comdat_signature(comdat_entry**, void*)':
> /vol/gcc/src/hg/trunk/local/gcc/config/sol2.c:267:12: error: 'decl_comdat_group' was not declared in this scope
> 
> The following snippet allows a sparc-sun-solaris2.11 bootstrap to go
> along further, only to break again later in libjava for what seems to be
> unrelated reasons.
> 
> 2014-05-26  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	* config/sol2.c: Include cgraph.h.

I moved decl_comdat_group offline that should fix the problem. Does it work now?

Honza

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Rainer Orth]

[-- Attachment #1: Type: text/plain, Size: 2334 bytes --]

Jan Hubicka <hubicka@ucw.cz> writes:

>> I'm fine with enlarging tree_function_decl for now - ideally we'd push
>> stuff from it elsewhere (like target and optimization option tree nodes,
>> or most of the visibility and symbol related stuff).  Not sure why
>> tree_type_decl inherits from tree_decl_non_common (and thus
>> tree_decl_with_vis).  Probably because of the non-common parts
>> being (ab-)used by FEs.  Otherwise I'd say simply put a symtab
>> node pointer into tree_decl_with_vis ... (can we move
>> section_name and comdat_group more easily than assembler_name?)
>
> Hi,
> this patch removes comdat_group pointer and adds direct symtab pointer.  As
> expected, the change is not completely easy. The main uglyness in C++'s version
> of duplicate_decl that creates a duplicated decl with duplicated symtab node
> now and needs to remove it.  Other problem is copy_node and c's duplicate_decl
> that does memcpy on a node and thus also copie the symtab pointer that is not
> the right thing to do.
>
> On the other hand on middle-end side several things simplify, so I think overall
> the approach works relatively well.
>
> I have bootstrapped/regtested x86_64-linux and I plan to give it more testing
> tomorrow and commit if there are no complains.  Incrementally I would like then
> to cleanup way the decl_with_vis.symtab_node pointer is maintained.  I do not
> want to allow users to tamper with it, so I did not make accessor macro for
> it, however there are more direct uses than I would like: I will need to figure
> out how to reduce those.

This patch broke Solaris bootstrap:

/vol/gcc/src/hg/trunk/local/gcc/config/sol2.c: In function 'void solaris_elf_asm_comdat_section(const char*, unsigned int, tree)':
/vol/gcc/src/hg/trunk/local/gcc/config/sol2.c:213:17: error: 'decl_comdat_group' was not declared in this scope
/vol/gcc/src/hg/trunk/local/gcc/config/sol2.c: In function 'int solaris_define_comdat_signature(comdat_entry**, void*)':
/vol/gcc/src/hg/trunk/local/gcc/config/sol2.c:267:12: error: 'decl_comdat_group' was not declared in this scope

The following snippet allows a sparc-sun-solaris2.11 bootstrap to go
along further, only to break again later in libjava for what seems to be
unrelated reasons.

2014-05-26  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* config/sol2.c: Include cgraph.h.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2.patch --]
[-- Type: text/x-patch, Size: 332 bytes --]

diff --git a/gcc/config/sol2.c b/gcc/config/sol2.c
--- a/gcc/config/sol2.c
+++ b/gcc/config/sol2.c
@@ -32,6 +32,7 @@ along with GCC; see the file COPYING3.  
 #include "diagnostic-core.h"
 #include "ggc.h"
 #include "hash-table.h"
+#include "cgraph.h"
 
 tree solaris_pending_aligns, solaris_pending_inits, solaris_pending_finis;
 

[-- Attachment #3: Type: text/plain, Size: 553 bytes --]


I'm not sure if this is the right approach, though, using
get_comdat_group seems to be preferred!?

ISTM that other ports might have similar problems: darwin.c, mep/mep.c,
and mips/mips.c all use DECL_COMDAT_GROUP without including cgraph.h.

> 	* mips.c (mips_start_unique_function): Likewise.
> 	(ix86_code_end): Likewise.
> 	(rs6000_code_end): Likweise.

The last two entries lack the file names.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Jan Hubicka]

> On Thu, May 22, 2014 at 2:49 PM, Martin Jambor <mjambor@suse.cz> wrote:
> > Hi,
> >
> > On Wed, May 21, 2014 at 04:27:32PM +0200, Richard Biener wrote:
> >> On Wed, May 21, 2014 at 3:16 PM, Martin Jambor <mjambor@suse.cz> wrote:
> >> > Hi,
> >> >
> >> > this demonstrates how results of ipa-prop escape analysis from
> >> > previous patches can be used at a later stage of compilation by
> >> > directly returning them from gimple_call_arg_flags which currently
> >> > relies on fnspec annotations.
> >> >
> >> > Bootstrapped and tested on x86_64-linux and also passes LTO bootstrap.
> >> > I have only had a brief look at behavior of this in SPEC 2006 and for
> >> > example in astar 1.19% of invocations of gimple_call_arg_flags return
> >> > noescape where we previously never did and in calculix this increases
> >> > from 15.62% (from annotations) to 18.14%.  Noclobber flag is reported
> >> > far less often still but for example in gamess that number raises from
> >> > 5.21% to 7.66%.
> >> >
> >> > Thanks,
> >> >
> >> > Martin
> >> >
> >> >
> >> > 2014-04-30  Martin Jambor  <mjambor@suse.cz>
> >> >
> >> >         * gimple.c: Include cgraph.h.
> >> >         (gimple_call_arg_flags): Also query bitmaps in cgraph_node.
> >> >
> >> > Index: src/gcc/gimple.c
> >> > ===================================================================
> >> > --- src.orig/gcc/gimple.c
> >> > +++ src/gcc/gimple.c
> >> > @@ -47,7 +47,7 @@ along with GCC; see the file COPYING3.
> >> >  #include "demangle.h"
> >> >  #include "langhooks.h"
> >> >  #include "bitmap.h"
> >> > -
> >> > +#include "cgraph.h"
> >> >
> >> >  /* All the tuples have their operand vector (if present) at the very bottom
> >> >     of the structure.  Therefore, the offset required to find the
> >> > @@ -1349,32 +1349,50 @@ int
> >> >  gimple_call_arg_flags (const_gimple stmt, unsigned arg)
> >> >  {
> >> >    tree attr = gimple_call_fnspec (stmt);
> >> > +  int ret;
> >> >
> >> > -  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
> >> > -    return 0;
> >> > -
> >> > -  switch (TREE_STRING_POINTER (attr)[1 + arg])
> >> > +  if (attr && 1 + arg < (unsigned) TREE_STRING_LENGTH (attr))
> >> >      {
> >> > -    case 'x':
> >> > -    case 'X':
> >> > -      return EAF_UNUSED;
> >> > -
> >> > -    case 'R':
> >> > -      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> >> > -
> >> > -    case 'r':
> >> > -      return EAF_NOCLOBBER | EAF_NOESCAPE;
> >> > -
> >> > -    case 'W':
> >> > -      return EAF_DIRECT | EAF_NOESCAPE;
> >> > -
> >> > -    case 'w':
> >> > -      return EAF_NOESCAPE;
> >> > +      switch (TREE_STRING_POINTER (attr)[1 + arg])
> >> > +       {
> >> > +       case 'x':
> >> > +       case 'X':
> >> > +         ret = EAF_UNUSED;
> >> > +         break;
> >> > +       case 'R':
> >> > +         ret = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> >> > +         break;
> >> > +       case 'r':
> >> > +         ret = EAF_NOCLOBBER | EAF_NOESCAPE;
> >> > +         break;
> >> > +       case 'W':
> >> > +         ret = EAF_DIRECT | EAF_NOESCAPE;
> >> > +         break;
> >> > +       case 'w':
> >> > +         ret = EAF_NOESCAPE;
> >> > +         break;
> >> > +       case '.':
> >> > +       default:
> >> > +         ret = 0;
> >> > +       }
> >> > +    }
> >> > +  else
> >> > +    ret = 0;
> >> >
> >> > -    case '.':
> >> > -    default:
> >> > -      return 0;
> >> > +  tree callee_decl = gimple_call_fndecl (stmt);
> >> > +  if (callee_decl)
> >> > +    {
> >> > +      cgraph_node *callee_node = cgraph_get_node (callee_decl);
> >> > +      if (callee_node)
> >> > +       {
> >> > +         if (cgraph_param_noescape_p (callee_node, arg))
> >> > +           ret |= EAF_NOESCAPE;
> >> > +         if (cgraph_param_noclobber_p (callee_node, arg))
> >> > +           ret |= EAF_NOCLOBBER;
> >>
> >> That's quite expensive.  I guess we need a better way to store
> >> those?
> >
> > if we want to avoid the cgraph_node lookup, then I think we need to
> > store this information in the decl or struct function.  That is
> > certainly possible and might even be more appropriate.
> 
> Can we?  If the body is not readily available we only have decl and
> cgraph-node, not struct function.

Yep, indeed I think cgraph_node is good place to home this so it can
work with partitioning.   With the weekend changes to have direct pointer,
I guess the performance problems are gone.

My plan is to add a template that makes it easy to annotate symbol nodes
either by array (as we do by hand now in ipa-inline/ipa-prop and other places)
or by hashtable (for sparse data, like thunk information, or comdat information)
and move some of stuff we currently have in cgraph there
(rtl info, thunks, aliases, comdats, nested function tree can all go). For
performance critical stuff I have no problem adding it into cgraph nodes
themselves.

If someone wants to beat me and write GGC friendly template for this,
I would very welcome it.
> 
> I suppose we could exchange the struct function pointer in
> tree_function_decl for a cgraph_node pointer and put
> the struct function pointer into the cgraph_node.
> 
> Of course that may have impacts on FEs who might create
> struct function before creating a cgraph node.  But at least
> it would avoid enlarging FUNCTION_DECL.
> 
> In the end most of the tree_decl_with_vis stuff should move over to symtab
> and var-decls should get a varpool_node pointer as well.
> 
> Back to the call flags stuff - I also meant the representation of the
> "fn spec" attribute.  Rather than parsing that again and again move
> it to a better place (which you seem to invent?) and better unified
> representation.
> 
> Can you try if removing the cgraph hash is possible with the
> struct function pointer idea?

If there are no problems, I plan to move also DECL_SECTION and do
some cleanups to my weekend change (in particular I do not like the
way it works with duplicate_decl. Perhaps we don't really need to
duplicate this info).
We can experiment with assembler name and struct function next.

Honza
> 
> Thanks,
> Richard.
> 
> > Thanks,
> >
> > Martin
> >
> >>
> >> > +       }
> >> >      }
> >> > +
> >> > +  return ret;
> >> >  }
> >> >
> >> >  /* Detects return flags for the call STMT.  */
> >> >

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Jan Hubicka]

> I'm fine with enlarging tree_function_decl for now - ideally we'd push
> stuff from it elsewhere (like target and optimization option tree nodes,
> or most of the visibility and symbol related stuff).  Not sure why
> tree_type_decl inherits from tree_decl_non_common (and thus
> tree_decl_with_vis).  Probably because of the non-common parts
> being (ab-)used by FEs.  Otherwise I'd say simply put a symtab
> node pointer into tree_decl_with_vis ... (can we move
> section_name and comdat_group more easily than assembler_name?)

Hi,
this patch removes comdat_group pointer and adds direct symtab pointer.  As
expected, the change is not completely easy. The main uglyness in C++'s version
of duplicate_decl that creates a duplicated decl with duplicated symtab node
now and needs to remove it.  Other problem is copy_node and c's duplicate_decl
that does memcpy on a node and thus also copie the symtab pointer that is not
the right thing to do.

On the other hand on middle-end side several things simplify, so I think overall
the approach works relatively well.

I have bootstrapped/regtested x86_64-linux and I plan to give it more testing
tomorrow and commit if there are no complains.  Incrementally I would like then
to cleanup way the decl_with_vis.symtab_node pointer is maintained.  I do not
want to allow users to tamper with it, so I did not make accessor macro for
it, however there are more direct uses than I would like: I will need to figure
out how to reduce those.

I can also incrementally move DECL_SECTION and some other stuff I think now.

Honza

	* tree-core.h (tree_decl_with_vis): Replace comdat_group by
	symtab_node pointer.
	* tree.c (copy_node_stat): Be sure tonot copy
	symtab_node pointer.
	(find_decls_types_r): Do not walk COMDAT_GROUP.
	* tree.h (DECL_COMDAT_GROUP): Revamp to use decl_comdat_group.
	* varasm.c (make_decl_one_only): Use set_comdat_group;
	create node if needed.
	* ipa-inline-transform.c (save_inline_function_body): Update
	way we decl->symtab mapping.
	* symtab.c (symtab_hash, hash_node, eq_node
	symtab_insert_node_to_hashtable): Remove.
	(symtab_register_node): Update.
	(symtab_unregister_node): Update.
	(symtab_get_node): Reimplement as inline function.
	(symtab_add_to_same_comdat_group): Update.
	(symtab_dissolve_same_comdat_group_list): Update.
	(dump_symtab_base): Update.
	(verify_symtab_base): Update.
	(symtab_make_decl_local): Update.
	(fixup_same_cpp_alias_visibility): Update.
	(symtab_nonoverwritable_alias): Update.
	* cgraphclones.c (set_new_clone_decl_and_node_flags): Update.
	* ipa.c (update_visibility_by_resolution_info): UPdate.
	* bb-reorder.c: Include cgraph.h
	* lto-streamer-out.c (DFS_write_tree_body, hash_tree): Do not deal
	with comdat groups.
	* ipa-comdats.c (set_comdat_group, ipa_comdats): Update.
	* cgraph.c (cgraph_get_create_node): Update.
	* cgraph.h (struct symtab_node): Add get_comdat_group, set_comdat_group
	and comdat_group_.
	(symtab_get_node): Make inline.
	(symtab_insert_node_to_hashtable): Remove.
	(symtab_can_be_discarded): Update.
	(decl_comdat_group): New function.
	* tree-streamer-in.c (lto_input_ts_decl_with_vis_tree_pointers): Update.
	* lto-cgraph.c (lto_output_node, lto_output_varpool_node): Stream out
	comdat group name.
	(read_comdat_group): New function.
	(input_node, input_varpool_node): Use it.
	* trans-mem.c (ipa_tm_create_version_alias): Update code creating
	comdat groups.
	* mips.c (mips_start_unique_function): Likewise.
	(ix86_code_end): Likewise.
	(rs6000_code_end): Likweise.
	* tree-streamer-out.c (DECL_COMDAT_GROUP): Do not stream
	comdat group.

	* lto-symtab.c (lto_symtab_merge_symbols): Update code setting
	symtab pointer.
	* lto.c (compare_tree_sccs_1): Do not compare comdat groups.

	* optmize.c (maybe_thunk_body): Use set_comdat_group.
	(maybe_clone_body): Likewise.
	* decl.c (duplicate_decls): Update code duplicating comdat group;
	do not copy symtab pointer; before freeing newdecl remove it
	from symtab.
	* decl2.c (constrain_visibility): Use set_comdat_group.

	* c-decl.c (merge_decls): Preserve symtab node pointers.
	(duplicate_decls): Free new decl.
Index: tree-core.h
===================================================================
--- tree-core.h	(revision 210887)
+++ tree-core.h	(working copy)
@@ -1442,7 +1442,7 @@ struct GTY(()) tree_decl_with_vis {
  struct tree_decl_with_rtl common;
  tree assembler_name;
  tree section_name;
- tree comdat_group;
+ struct symtab_node *symtab_node;
 
  /* Belong to VAR_DECL exclusively.  */
  unsigned defer_output : 1;
Index: tree.c
===================================================================
--- tree.c	(revision 210887)
+++ tree.c	(working copy)
@@ -972,14 +972,20 @@ copy_node_stat (tree node MEM_STAT_DECL)
 	}
       /* DECL_DEBUG_EXPR is copied explicitely by callers.  */
       if (TREE_CODE (node) == VAR_DECL)
-	DECL_HAS_DEBUG_EXPR_P (t) = 0;
+	{
+	  DECL_HAS_DEBUG_EXPR_P (t) = 0;
+	  t->decl_with_vis.symtab_node = NULL;
+	}
       if (TREE_CODE (node) == VAR_DECL && DECL_HAS_INIT_PRIORITY_P (node))
 	{
 	  SET_DECL_INIT_PRIORITY (t, DECL_INIT_PRIORITY (node));
 	  DECL_HAS_INIT_PRIORITY_P (t) = 1;
 	}
       if (TREE_CODE (node) == FUNCTION_DECL)
-	DECL_STRUCT_FUNCTION (t) = NULL;
+	{
+	  DECL_STRUCT_FUNCTION (t) = NULL;
+	  t->decl_with_vis.symtab_node = NULL;
+	}
     }
   else if (TREE_CODE_CLASS (code) == tcc_type)
     {
@@ -5238,7 +5244,6 @@ find_decls_types_r (tree *tp, int *ws, v
       else if (TREE_CODE (t) == VAR_DECL)
 	{
 	  fld_worklist_push (DECL_SECTION_NAME (t), fld);
-	  fld_worklist_push (DECL_COMDAT_GROUP (t), fld);
 	}
 
       if ((TREE_CODE (t) == VAR_DECL || TREE_CODE (t) == PARM_DECL)
Index: tree.h
===================================================================
--- tree.h	(revision 210887)
+++ tree.h	(working copy)
@@ -2323,7 +2323,7 @@ extern void decl_value_expr_insert (tree
   (DECL_WITH_VIS_CHECK (NODE)->decl_with_vis.comdat_flag)
 
 #define DECL_COMDAT_GROUP(NODE) \
-  (DECL_WITH_VIS_CHECK (NODE)->decl_with_vis.comdat_group)
+  decl_comdat_group (NODE)
 
 /* Used in TREE_PUBLIC decls to indicate that copies of this DECL in
    multiple translation units should be merged.  */
Index: varasm.c
===================================================================
--- varasm.c	(revision 210887)
+++ varasm.c	(working copy)
@@ -5919,17 +5919,23 @@ supports_one_only (void)
 void
 make_decl_one_only (tree decl, tree comdat_group)
 {
+  struct symtab_node *symbol;
   gcc_assert (TREE_CODE (decl) == VAR_DECL
 	      || TREE_CODE (decl) == FUNCTION_DECL);
 
   TREE_PUBLIC (decl) = 1;
 
+  if (TREE_CODE (decl) == VAR_DECL)
+    symbol = varpool_node_for_decl (decl);
+  else
+    symbol = cgraph_get_create_node (decl);
+
   if (SUPPORTS_ONE_ONLY)
     {
 #ifdef MAKE_DECL_ONE_ONLY
       MAKE_DECL_ONE_ONLY (decl);
 #endif
-      DECL_COMDAT_GROUP (decl) = comdat_group;
+      symbol->set_comdat_group (comdat_group);
     }
   else if (TREE_CODE (decl) == VAR_DECL
       && (DECL_INITIAL (decl) == 0 || DECL_INITIAL (decl) == error_mark_node))
Index: ipa-inline-transform.c
===================================================================
--- ipa-inline-transform.c	(revision 210887)
+++ ipa-inline-transform.c	(working copy)
@@ -341,7 +341,7 @@ save_inline_function_body (struct cgraph
   /* first_clone will be turned into real function.  */
   first_clone = node->clones;
   first_clone->decl = copy_node (node->decl);
-  symtab_insert_node_to_hashtable (first_clone);
+  first_clone->decl->decl_with_vis.symtab_node = first_clone;
   gcc_assert (first_clone == cgraph_get_node (first_clone->decl));
 
   /* Now reshape the clone tree, so all other clones descends from
Index: cp/optimize.c
===================================================================
--- cp/optimize.c	(revision 210887)
+++ cp/optimize.c	(working copy)
@@ -285,7 +285,7 @@ maybe_thunk_body (tree fn, bool force)
   else if (HAVE_COMDAT_GROUP)
     {
       tree comdat_group = cdtor_comdat_group (fns[1], fns[0]);
-      DECL_COMDAT_GROUP (fns[0]) = comdat_group;
+      cgraph_get_create_node (fns[0])->set_comdat_group (comdat_group);
       symtab_add_to_same_comdat_group (cgraph_get_create_node (fns[1]),
 				       cgraph_get_create_node (fns[0]));
       symtab_add_to_same_comdat_group (symtab_get_node (fn),
@@ -473,7 +473,7 @@ maybe_clone_body (tree fn)
 	 name of fn was corrupted by write_mangled_name by adding *INTERNAL*
 	 to it. By doing so, it also corrupted the comdat group. */
       if (DECL_ONE_ONLY (fn))
-	DECL_COMDAT_GROUP (clone) = cxx_comdat_group (clone);
+	cgraph_get_create_node (clone)->set_comdat_group (cxx_comdat_group (clone));
       DECL_SECTION_NAME (clone) = DECL_SECTION_NAME (fn);
       DECL_USE_TEMPLATE (clone) = DECL_USE_TEMPLATE (fn);
       DECL_EXTERNAL (clone) = DECL_EXTERNAL (fn);
@@ -550,7 +550,7 @@ maybe_clone_body (tree fn)
 		 into the same, *[CD]5* comdat group instead of
 		 *[CD][12]*.  */
 	      comdat_group = cdtor_comdat_group (fns[1], fns[0]);
-	      DECL_COMDAT_GROUP (fns[0]) = comdat_group;
+	      cgraph_get_create_node (fns[0])->set_comdat_group (comdat_group);
 	      symtab_add_to_same_comdat_group (symtab_get_node (clone),
 					       symtab_get_node (fns[0]));
 	    }
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 210887)
+++ cp/decl.c	(working copy)
@@ -2065,8 +2065,17 @@ duplicate_decls (tree newdecl, tree oldd
   /* Merge the storage class information.  */
   merge_weak (newdecl, olddecl);
 
-  if (DECL_ONE_ONLY (olddecl))
-    DECL_COMDAT_GROUP (newdecl) = DECL_COMDAT_GROUP (olddecl);
+  if ((TREE_CODE (olddecl) == FUNCTION_DECL || TREE_CODE (olddecl) == VAR_DECL)
+      && (DECL_EXTERNAL (olddecl) || TREE_PUBLIC (olddecl) || TREE_STATIC (olddecl))
+      && DECL_ONE_ONLY (olddecl))
+    {
+      struct symtab_node *symbol;
+      if (TREE_CODE (olddecl) == FUNCTION_DECL)
+	symbol = cgraph_get_create_node (newdecl);
+      else
+	symbol = varpool_node_for_decl (newdecl);
+      symbol->set_comdat_group (symtab_get_node (olddecl)->get_comdat_group ());
+    }
 
   DECL_DEFER_OUTPUT (newdecl) |= DECL_DEFER_OUTPUT (olddecl);
   TREE_PUBLIC (newdecl) = TREE_PUBLIC (olddecl);
@@ -2376,6 +2385,7 @@ duplicate_decls (tree newdecl, tree oldd
   if (TREE_CODE (newdecl) == FUNCTION_DECL)
     {
       int function_size;
+      struct symtab_node *snode = symtab_get_node (olddecl);
 
       function_size = sizeof (struct tree_decl_common);
 
@@ -2386,6 +2396,10 @@ duplicate_decls (tree newdecl, tree oldd
       memcpy ((char *) olddecl + sizeof (struct tree_decl_common),
 	      (char *) newdecl + sizeof (struct tree_decl_common),
 	      sizeof (struct tree_function_decl) - sizeof (struct tree_decl_common));
+
+      /* Preserve symtab node mapping.  */
+      olddecl->decl_with_vis.symtab_node = snode;
+
       if (new_template_info)
 	/* If newdecl is a template instantiation, it is possible that
 	   the following sequence of events has occurred:
@@ -2415,6 +2429,7 @@ duplicate_decls (tree newdecl, tree oldd
   else
     {
       size_t size = tree_code_size (TREE_CODE (olddecl));
+
       memcpy ((char *) olddecl + sizeof (struct tree_common),
 	      (char *) newdecl + sizeof (struct tree_common),
 	      sizeof (struct tree_decl_common) - sizeof (struct tree_common));
@@ -2428,10 +2443,17 @@ duplicate_decls (tree newdecl, tree oldd
 	case TYPE_DECL:
 	case CONST_DECL:
 	  {
+            struct symtab_node *snode = NULL;
+
+            if (TREE_CODE (olddecl) == VAR_DECL
+		&& (TREE_STATIC (olddecl) || TREE_PUBLIC (olddecl) || DECL_EXTERNAL (olddecl)))
+	      snode = symtab_get_node (olddecl);
 	    memcpy ((char *) olddecl + sizeof (struct tree_decl_common),
 		    (char *) newdecl + sizeof (struct tree_decl_common),
 		    size - sizeof (struct tree_decl_common)
 		    + TREE_CODE_LENGTH (TREE_CODE (newdecl)) * sizeof (char *));
+            if (TREE_CODE (olddecl) == VAR_DECL)
+	      olddecl->decl_with_vis.symtab_node = snode;
 	  }
 	  break;
 	default:
@@ -2466,7 +2488,21 @@ duplicate_decls (tree newdecl, tree oldd
 
   /* The NEWDECL will no longer be needed.  Because every out-of-class
      declaration of a member results in a call to duplicate_decls,
-     freeing these nodes represents in a significant savings.  */
+     freeing these nodes represents in a significant savings.
+
+     Before releasing the node, be sore to remove function from symbol
+     table that might have been inserted there to record comdat group.
+     Be sure to however do not free DECL_STRUCT_FUNCTION becuase this
+     structure is shared in between newdecl and oldecl.  */
+  if (TREE_CODE (newdecl) == FUNCTION_DECL)
+    DECL_STRUCT_FUNCTION (newdecl) = NULL;
+  if (TREE_CODE (newdecl) == FUNCTION_DECL
+      || TREE_CODE (newdecl) == VAR_DECL)
+    {
+      struct symtab_node *snode = symtab_get_node (newdecl);
+      if (snode)
+	symtab_remove_node (snode);
+    }
   ggc_free (newdecl);
 
   return olddecl;
Index: cp/decl2.c
===================================================================
--- cp/decl2.c	(revision 210887)
+++ cp/decl2.c	(working copy)
@@ -2093,7 +2093,14 @@ constrain_visibility (tree decl, int vis
 	  TREE_PUBLIC (decl) = 0;
 	  DECL_WEAK (decl) = 0;
 	  DECL_COMMON (decl) = 0;
-	  DECL_COMDAT_GROUP (decl) = NULL_TREE;
+	  if (TREE_CODE (decl) == FUNCTION_DECL
+	      || TREE_CODE (decl) == VAR_DECL)
+	    {
+	      struct symtab_node *snode = symtab_get_node (decl);
+
+	      if (snode)
+	        snode->set_comdat_group (NULL);
+	    }
 	  DECL_INTERFACE_KNOWN (decl) = 1;
 	  if (DECL_LANG_SPECIFIC (decl))
 	    DECL_NOT_REALLY_EXTERN (decl) = 1;
Index: symtab.c
===================================================================
--- symtab.c	(revision 210887)
+++ symtab.c	(working copy)
@@ -57,8 +57,6 @@ const char * const ld_plugin_symbol_reso
   "prevailing_def_ironly_exp"
 };
 
-/* Hash table used to convert declarations into nodes.  */
-static GTY((param_is (symtab_node))) htab_t symtab_hash;
 /* Hash table used to convert assembler names into nodes.  */
 static GTY((param_is (symtab_node))) htab_t assembler_name_hash;
 
@@ -70,26 +68,6 @@ symtab_node *symtab_nodes;
    them, to support -fno-toplevel-reorder.  */
 int symtab_order;
 
-/* Returns a hash code for P.  */
-
-static hashval_t
-hash_node (const void *p)
-{
-  const symtab_node *n = (const symtab_node *) p;
-  return (hashval_t) DECL_UID (n->decl);
-}
-
-
-/* Returns nonzero if P1 and P2 are equal.  */
-
-static int
-eq_node (const void *p1, const void *p2)
-{
-  const symtab_node *n1 = (const symtab_node *) p1;
-  const symtab_node *n2 = (const symtab_node *) p2;
-  return DECL_UID (n1->decl) == DECL_UID (n2->decl);
-}
-
 /* Hash asmnames ignoring the user specified marks.  */
 
 static hashval_t
@@ -282,21 +260,14 @@ symtab_prevail_in_asm_name_hash (symtab_
 void
 symtab_register_node (symtab_node *node)
 {
-  struct symtab_node key;
-  symtab_node **slot;
-
   node->next = symtab_nodes;
   node->previous = NULL;
   if (symtab_nodes)
     symtab_nodes->previous = node;
   symtab_nodes = node;
 
-  if (!symtab_hash)
-    symtab_hash = htab_create_ggc (10, hash_node, eq_node, NULL);
-  key.decl = node->decl;
-  slot = (symtab_node **) htab_find_slot (symtab_hash, &key, INSERT);
-  if (*slot == NULL)
-    *slot = node;
+  if (!node->decl->decl_with_vis.symtab_node)
+    node->decl->decl_with_vis.symtab_node = node;
 
   ipa_empty_ref_list (&node->ref_list);
 
@@ -307,22 +278,6 @@ symtab_register_node (symtab_node *node)
   insert_to_assembler_name_hash (node, false);
 }
 
-/* Make NODE to be the one symtab hash is pointing to.  Used when reshaping tree
-   of inline clones.  */
-
-void
-symtab_insert_node_to_hashtable (symtab_node *node)
-{
-  struct symtab_node key;
-  symtab_node **slot;
-
-  if (!symtab_hash)
-    symtab_hash = htab_create_ggc (10, hash_node, eq_node, NULL);
-  key.decl = node->decl;
-  slot = (symtab_node **) htab_find_slot (symtab_hash, &key, INSERT);
-  *slot = node;
-}
-
 /* Remove NODE from same comdat group.   */
 
 void
@@ -349,7 +304,6 @@ symtab_remove_from_same_comdat_group (sy
 void
 symtab_unregister_node (symtab_node *node)
 {
-  void **slot;
   ipa_remove_all_references (&node->ref_list);
   ipa_remove_all_referring (&node->ref_list);
 
@@ -364,55 +318,20 @@ symtab_unregister_node (symtab_node *nod
   node->next = NULL;
   node->previous = NULL;
 
-  slot = htab_find_slot (symtab_hash, node, NO_INSERT);
-
   /* During LTO symtab merging we temporarily corrupt decl to symtab node
      hash.  */
-  gcc_assert ((slot && *slot) || in_lto_p);
-  if (slot && *slot && *slot == node)
+  gcc_assert (node->decl->decl_with_vis.symtab_node || in_lto_p);
+  if (node->decl->decl_with_vis.symtab_node == node)
     {
       symtab_node *replacement_node = NULL;
       if (cgraph_node *cnode = dyn_cast <cgraph_node *> (node))
 	replacement_node = cgraph_find_replacement_node (cnode);
-      if (!replacement_node)
-	htab_clear_slot (symtab_hash, slot);
-      else
-	*slot = replacement_node;
+      node->decl->decl_with_vis.symtab_node = replacement_node;
     }
   if (!is_a <varpool_node *> (node) || !DECL_HARD_REGISTER (node->decl))
     unlink_from_assembler_name_hash (node, false);
 }
 
-/* Return symbol table node associated with DECL, if any,
-   and NULL otherwise.  */
-
-symtab_node *
-symtab_get_node (const_tree decl)
-{
-  symtab_node **slot;
-  struct symtab_node key;
-
-#ifdef ENABLE_CHECKING
-  /* Check that we are called for sane type of object - functions
-     and static or external variables.  */
-  gcc_checking_assert (TREE_CODE (decl) == FUNCTION_DECL
-		       || (TREE_CODE (decl) == VAR_DECL
-			   && (TREE_STATIC (decl) || DECL_EXTERNAL (decl)
-			       || in_lto_p)));
-#endif
-
-  if (!symtab_hash)
-    return NULL;
-
-  key.decl = CONST_CAST2 (tree, const_tree, decl);
-
-  slot = (symtab_node **) htab_find_slot (symtab_hash, &key,
-					 NO_INSERT);
-
-  if (slot)
-    return *slot;
-  return NULL;
-}
 
 /* Remove symtab NODE from the symbol table.  */
 
@@ -513,11 +432,11 @@ void
 symtab_add_to_same_comdat_group (symtab_node *new_node,
 				 symtab_node *old_node)
 {
-  gcc_assert (DECL_COMDAT_GROUP (old_node->decl));
+  gcc_assert (old_node->get_comdat_group ());
   gcc_assert (!new_node->same_comdat_group);
   gcc_assert (new_node != old_node);
 
-  DECL_COMDAT_GROUP (new_node->decl) = DECL_COMDAT_GROUP (old_node->decl);
+  new_node->set_comdat_group (old_node->get_comdat_group ());
   new_node->same_comdat_group = old_node;
   if (!old_node->same_comdat_group)
     old_node->same_comdat_group = new_node;
@@ -546,10 +465,10 @@ symtab_dissolve_same_comdat_group_list (
     {
       next = n->same_comdat_group;
       n->same_comdat_group = NULL;
-      /* Clear DECL_COMDAT_GROUP for comdat locals, since
+      /* Clear comdat_group for comdat locals, since
          make_decl_local doesn't.  */
       if (!TREE_PUBLIC (n->decl))
-	DECL_COMDAT_GROUP (n->decl) = NULL_TREE;
+	n->set_comdat_group (NULL);
       n = next;
     }
   while (n != node);
@@ -639,9 +558,9 @@ dump_symtab_base (FILE *f, symtab_node *
     fprintf (f, " dll_import");
   if (DECL_COMDAT (node->decl))
     fprintf (f, " comdat");
-  if (DECL_COMDAT_GROUP (node->decl))
+  if (node->get_comdat_group ())
     fprintf (f, " comdat_group:%s",
-	     IDENTIFIER_POINTER (DECL_COMDAT_GROUP (node->decl)));
+	     IDENTIFIER_POINTER (node->get_comdat_group ()));
   if (DECL_ONE_ONLY (node->decl))
     fprintf (f, " one_only");
   if (DECL_SECTION_NAME (node->decl))
@@ -766,7 +685,7 @@ verify_symtab_base (symtab_node *node)
       hashed_node = symtab_get_node (node->decl);
       if (!hashed_node)
 	{
-	  error ("node not found in symtab decl hashtable");
+	  error ("node not found node->decl->decl_with_vis.symtab_node");
 	  error_found = true;
 	}
       if (hashed_node != node
@@ -775,7 +694,7 @@ verify_symtab_base (symtab_node *node)
 	      || dyn_cast <cgraph_node *> (node)->clone_of->decl
 		 != node->decl))
 	{
-	  error ("node differs from symtab decl hashtable");
+	  error ("node differs from node->decl->decl_with_vis.symtab_node");
 	  error_found = true;
 	}
     }
@@ -832,12 +751,12 @@ verify_symtab_base (symtab_node *node)
     {
       symtab_node *n = node->same_comdat_group;
 
-      if (!DECL_COMDAT_GROUP (n->decl))
+      if (!n->get_comdat_group ())
 	{
-	  error ("node is in same_comdat_group list but has no DECL_COMDAT_GROUP");
+	  error ("node is in same_comdat_group list but has no comdat_group");
 	  error_found = true;
 	}
-      if (DECL_COMDAT_GROUP (n->decl) != DECL_COMDAT_GROUP (node->same_comdat_group->decl))
+      if (n->get_comdat_group () != node->get_comdat_group ())
 	{
 	  error ("same_comdat_group list across different groups");
 	  error_found = true;
@@ -950,7 +869,7 @@ symtab_make_decl_local (tree decl)
 {
   rtx rtl, symbol;
 
-  /* Avoid clearing DECL_COMDAT_GROUP on comdat-local decls.  */
+  /* Avoid clearing comdat_groups on comdat-local decls.  */
   if (TREE_PUBLIC (decl) == 0)
     return;
 
@@ -958,12 +877,11 @@ symtab_make_decl_local (tree decl)
     DECL_COMMON (decl) = 0;
   else gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
 
-  if (DECL_COMDAT_GROUP (decl) || DECL_COMDAT (decl))
+  if (DECL_COMDAT (decl))
     {
       DECL_SECTION_NAME (decl) = 0;
       DECL_COMDAT (decl) = 0;
     }
-  DECL_COMDAT_GROUP (decl) = 0;
   DECL_WEAK (decl) = 0;
   DECL_EXTERNAL (decl) = 0;
   DECL_VISIBILITY_SPECIFIED (decl) = 0;
@@ -1097,11 +1015,13 @@ fixup_same_cpp_alias_visibility (symtab_
   DECL_VIRTUAL_P (node->decl) = DECL_VIRTUAL_P (target->decl);
   if (TREE_PUBLIC (node->decl))
     {
+      tree group;
+
       DECL_EXTERNAL (node->decl) = DECL_EXTERNAL (target->decl);
       DECL_COMDAT (node->decl) = DECL_COMDAT (target->decl);
-      DECL_COMDAT_GROUP (node->decl)
-	 = DECL_COMDAT_GROUP (target->decl);
-      if (DECL_COMDAT_GROUP (target->decl)
+      group = target->get_comdat_group ();
+      node->set_comdat_group (group);
+      if (group
 	  && !node->same_comdat_group)
 	symtab_add_to_same_comdat_group (node, target);
     }
@@ -1231,9 +1151,6 @@ symtab_nonoverwritable_alias (symtab_nod
 
   /* Update the properties.  */
   DECL_EXTERNAL (new_decl) = 0;
-  if (DECL_COMDAT_GROUP (node->decl))
-    DECL_SECTION_NAME (new_decl) = NULL;
-  DECL_COMDAT_GROUP (new_decl) = 0;
   TREE_PUBLIC (new_decl) = 0;
   DECL_COMDAT (new_decl) = 0;
   DECL_WEAK (new_decl) = 0;
@@ -1246,8 +1163,7 @@ symtab_nonoverwritable_alias (symtab_nod
 				 (new_decl, node->decl);
     }
   else
-    new_node = varpool_create_variable_alias (new_decl,
-							    node->decl);
+    new_node = varpool_create_variable_alias (new_decl, node->decl);
   symtab_resolve_alias (new_node, node);  
   gcc_assert (decl_binds_to_current_def_p (new_decl));
   return new_node;
Index: cgraphclones.c
===================================================================
--- cgraphclones.c	(revision 210887)
+++ cgraphclones.c	(working copy)
@@ -283,7 +283,6 @@ static void
 set_new_clone_decl_and_node_flags (cgraph_node *new_node)
 {
   DECL_EXTERNAL (new_node->decl) = 0;
-  DECL_COMDAT_GROUP (new_node->decl) = 0;
   TREE_PUBLIC (new_node->decl) = 0;
   DECL_COMDAT (new_node->decl) = 0;
   DECL_WEAK (new_node->decl) = 0;
@@ -558,7 +557,7 @@ cgraph_create_virtual_clone (struct cgra
      that is not weak also.
      ??? We cannot use COMDAT linkage because there is no
      ABI support for this.  */
-  if (DECL_COMDAT_GROUP (old_decl))
+  if (old_node->get_comdat_group ())
     DECL_SECTION_NAME (new_node->decl) = NULL;
   set_new_clone_decl_and_node_flags (new_node);
   new_node->clone.tree_map = tree_map;
Index: ipa.c
===================================================================
--- ipa.c	(revision 210887)
+++ ipa.c	(working copy)
@@ -1021,13 +1021,13 @@ update_visibility_by_resolution_info (sy
     for (symtab_node *next = node->same_comdat_group;
 	 next != node; next = next->same_comdat_group)
       {
-	DECL_COMDAT_GROUP (next->decl) = NULL;
+	next->set_comdat_group (NULL);
 	DECL_WEAK (next->decl) = false;
 	if (next->externally_visible
 	    && !define)
 	  DECL_EXTERNAL (next->decl) = true;
       }
-  DECL_COMDAT_GROUP (node->decl) = NULL;
+  node->set_comdat_group (NULL);
   DECL_WEAK (node->decl) = false;
   if (!define)
     DECL_EXTERNAL (node->decl) = true;
@@ -1163,8 +1163,7 @@ function_and_variable_visibility (bool w
 	    {
 	      gcc_checking_assert (DECL_COMDAT (node->decl)
 				   == DECL_COMDAT (decl_node->decl));
-	      gcc_checking_assert (DECL_COMDAT_GROUP (node->decl)
-				   == DECL_COMDAT_GROUP (decl_node->decl));
+	      gcc_checking_assert (symtab_in_same_comdat_p (node, decl_node));
 	      gcc_checking_assert (node->same_comdat_group);
 	    }
 	  node->forced_by_abi = decl_node->forced_by_abi;
Index: bb-reorder.c
===================================================================
--- bb-reorder.c	(revision 210887)
+++ bb-reorder.c	(working copy)
@@ -99,6 +99,7 @@
 #include "tree-pass.h"
 #include "df.h"
 #include "bb-reorder.h"
+#include "cgraph.h"
 #include "except.h"
 
 /* The number of rounds.  In most cases there will only be 4 rounds, but
Index: lto-streamer-out.c
===================================================================
--- lto-streamer-out.c	(revision 210887)
+++ lto-streamer-out.c	(working copy)
@@ -535,7 +535,6 @@ DFS_write_tree_body (struct output_block
       if (DECL_ASSEMBLER_NAME_SET_P (expr))
 	DFS_follow_tree_edge (DECL_ASSEMBLER_NAME (expr));
       DFS_follow_tree_edge (DECL_SECTION_NAME (expr));
-      DFS_follow_tree_edge (DECL_COMDAT_GROUP (expr));
     }
 
   if (CODE_CONTAINS_STRUCT (code, TS_FIELD_DECL))
@@ -974,7 +973,6 @@ hash_tree (struct streamer_tree_cache_d
       if (DECL_ASSEMBLER_NAME_SET_P (t))
 	visit (DECL_ASSEMBLER_NAME (t));
       visit (DECL_SECTION_NAME (t));
-      visit (DECL_COMDAT_GROUP (t));
     }
 
   if (CODE_CONTAINS_STRUCT (code, TS_FIELD_DECL))
Index: ipa-comdats.c
===================================================================
--- ipa-comdats.c	(revision 210887)
+++ ipa-comdats.c	(working copy)
@@ -202,8 +202,8 @@ set_comdat_group (symtab_node *symbol,
 {
   symtab_node *head = (symtab_node *)head_p;
 
-  gcc_assert (!DECL_COMDAT_GROUP (symbol->decl));
-  DECL_COMDAT_GROUP (symbol->decl) = DECL_COMDAT_GROUP (head->decl);
+  gcc_assert (!symbol->get_comdat_group ());
+  symbol->set_comdat_group (head->get_comdat_group ());
   symtab_add_to_same_comdat_group (symbol, head);
   return false;
 }
@@ -218,6 +218,7 @@ ipa_comdats (void)
   symtab_node *symbol;
   bool comdat_group_seen = false;
   symtab_node *first = (symtab_node *) (void *) 1;
+  tree group;
 
   /* Start the dataflow by assigning comdat group to symbols that are in comdat
      groups already.  All other externally visible symbols must stay, we use
@@ -226,10 +227,10 @@ ipa_comdats (void)
   FOR_EACH_DEFINED_SYMBOL (symbol)
     if (!symtab_real_symbol_p (symbol))
       ;
-    else if (DECL_COMDAT_GROUP (symbol->decl))
+    else if ((group = symbol->get_comdat_group ()) != NULL)
       {
-        *map.insert (symbol) = DECL_COMDAT_GROUP (symbol->decl);
-        *comdat_head_map.insert (DECL_COMDAT_GROUP (symbol->decl)) = symbol;
+        *map.insert (symbol) = group;
+        *comdat_head_map.insert (group) = symbol;
 	comdat_group_seen = true;
 
 	/* Mark the symbol so we won't waste time visiting it for dataflow.  */
@@ -313,7 +314,7 @@ ipa_comdats (void)
   FOR_EACH_DEFINED_SYMBOL (symbol)
     {
       symbol->aux = NULL; 
-      if (!DECL_COMDAT_GROUP (symbol->decl)
+      if (!symbol->get_comdat_group ()
 	  && !symbol->alias
 	  && symtab_real_symbol_p (symbol))
 	{
Index: cgraph.c
===================================================================
--- cgraph.c	(revision 210887)
+++ cgraph.c	(working copy)
@@ -565,7 +565,7 @@ cgraph_get_create_node (tree decl)
       first_clone->clone_of = node;
       node->clones = first_clone;
       symtab_prevail_in_asm_name_hash (node);
-      symtab_insert_node_to_hashtable (node);
+      node->decl->decl_with_vis.symtab_node = node;
       if (dump_file)
 	fprintf (dump_file, "Introduced new external node "
 		 "(%s/%i) and turned into root of the clone tree.\n",
Index: cgraph.h
===================================================================
--- cgraph.h	(revision 210887)
+++ cgraph.h	(working copy)
@@ -141,6 +141,18 @@ public:
   /* Circular list of nodes in the same comdat group if non-NULL.  */
   symtab_node *same_comdat_group;
 
+  /* Return comdat group.  */
+  tree get_comdat_group ()
+    {
+      return comdat_group_;
+    }
+
+  /* Set comdat group.  */
+  void set_comdat_group (tree group)
+    {
+      comdat_group_ = group;
+    }
+
   /* Vectors of referring and referenced entities.  */
   struct ipa_ref_list ref_list;
 
@@ -153,6 +165,9 @@ public:
   struct lto_file_decl_data * lto_file_data;
 
   PTR GTY ((skip)) aux;
+
+  /* Comdat group the symbol is in.  Can be private if GGC allowed that.  */
+  tree comdat_group_;
 };
 
 enum availability
@@ -727,9 +742,7 @@ void symtab_register_node (symtab_node *
 void symtab_unregister_node (symtab_node *);
 void symtab_remove_from_same_comdat_group (symtab_node *);
 void symtab_remove_node (symtab_node *);
-symtab_node *symtab_get_node (const_tree);
 symtab_node *symtab_node_for_asm (const_tree asmname);
-void symtab_insert_node_to_hashtable (symtab_node *);
 void symtab_add_to_same_comdat_group (symtab_node *, symtab_node *);
 void symtab_dissolve_same_comdat_group_list (symtab_node *node);
 void dump_symtab (FILE *);
@@ -989,6 +1002,28 @@ void varpool_remove_initializer (varpool
 /* In cgraph.c */
 extern void change_decl_assembler_name (tree, tree);
 
+/* Return symbol table node associated with DECL, if any,
+   and NULL otherwise.  */
+
+static inline symtab_node *
+symtab_get_node (const_tree decl)
+{
+#ifdef ENABLE_CHECKING
+  /* Check that we are called for sane type of object - functions
+     and static or external variables.  */
+  gcc_checking_assert (TREE_CODE (decl) == FUNCTION_DECL
+		       || (TREE_CODE (decl) == VAR_DECL
+			   && (TREE_STATIC (decl) || DECL_EXTERNAL (decl)
+			       || in_lto_p)));
+  /* Check that the mapping is sane - perhaps this check can go away,
+     but at the moment frontends tends to corrupt the mapping by calling
+     memcpy/memset on the tree nodes.  */
+  gcc_checking_assert (!decl->decl_with_vis.symtab_node
+		       || decl->decl_with_vis.symtab_node->decl == decl);
+#endif
+  return decl->decl_with_vis.symtab_node;
+}
+
 /* Return callgraph node for given symbol and check it is a function. */
 static inline struct cgraph_node *
 cgraph (symtab_node *node)
@@ -1548,7 +1583,7 @@ static inline bool
 symtab_can_be_discarded (symtab_node *node)
 {
   return (DECL_EXTERNAL (node->decl)
-	  || (DECL_ONE_ONLY (node->decl)
+	  || (node->get_comdat_group ()
 	      && node->resolution != LDPR_PREVAILING_DEF
 	      && node->resolution != LDPR_PREVAILING_DEF_IRONLY
 	      && node->resolution != LDPR_PREVAILING_DEF_IRONLY_EXP));
@@ -1580,6 +1615,16 @@ symtab_in_same_comdat_p (symtab_node *on
 	two = cn->global.inlined_to;
     }
 
-  return DECL_COMDAT_GROUP (one->decl) == DECL_COMDAT_GROUP (two->decl);
+  return one->get_comdat_group () == two->get_comdat_group ();
+}
+
+/* Return comdat group of DECL.  */
+static inline tree
+decl_comdat_group (tree node)
+{
+  struct symtab_node *snode = symtab_get_node (node);
+  if (!snode)
+    return NULL;
+  return snode->get_comdat_group ();
 }
 #endif  /* GCC_CGRAPH_H  */
Index: tree-streamer-in.c
===================================================================
--- tree-streamer-in.c	(revision 210887)
+++ tree-streamer-in.c	(working copy)
@@ -760,7 +760,6 @@ lto_input_ts_decl_with_vis_tree_pointers
     }
 
   DECL_SECTION_NAME (expr) = stream_read_tree (ib, data_in);
-  DECL_COMDAT_GROUP (expr) = stream_read_tree (ib, data_in);
 }
 
 
Index: lto-cgraph.c
===================================================================
--- lto-cgraph.c	(revision 210887)
+++ lto-cgraph.c	(working copy)
@@ -395,6 +395,8 @@ lto_output_node (struct lto_simple_outpu
   ipa_opt_pass_d *pass;
   int i;
   bool alias_p;
+  const char *comdat;
+  tree group;
 
   boundary_p = !lto_symtab_encoder_in_partition_p (encoder, node);
 
@@ -478,15 +480,24 @@ lto_output_node (struct lto_simple_outpu
       streamer_write_hwi_stream (ob->main_stream, ref);
     }
 
-  if (node->same_comdat_group && !boundary_p)
+  group = node->get_comdat_group ();
+  if (group)
+    comdat = IDENTIFIER_POINTER (group);
+  else
+    comdat = "";
+  lto_output_data_stream (ob->main_stream, comdat, strlen (comdat) + 1);
+  if (group)
     {
-      ref = lto_symtab_encoder_lookup (encoder,
-				       node->same_comdat_group);
-      gcc_assert (ref != LCC_NOT_FOUND);
+      if (node->same_comdat_group && !boundary_p)
+	{
+	  ref = lto_symtab_encoder_lookup (encoder,
+					   node->same_comdat_group);
+	  gcc_assert (ref != LCC_NOT_FOUND);
+	}
+      else
+	ref = LCC_NOT_FOUND;
+      streamer_write_hwi_stream (ob->main_stream, ref);
     }
-  else
-    ref = LCC_NOT_FOUND;
-  streamer_write_hwi_stream (ob->main_stream, ref);
 
   streamer_write_hwi_stream (ob->main_stream, node->tp_first_run);
 
@@ -551,6 +562,8 @@ lto_output_varpool_node (struct lto_simp
   struct bitpack_d bp;
   int ref;
   bool alias_p;
+  const char *comdat;
+  tree group;
 
   streamer_write_enum (ob->main_stream, LTO_symtab_tags, LTO_symtab_last_tag,
 		       LTO_symtab_variable);
@@ -587,15 +600,24 @@ lto_output_varpool_node (struct lto_simp
 	  /* in_other_partition.  */
     }
   streamer_write_bitpack (&bp);
-  if (node->same_comdat_group && !boundary_p)
+  group = node->get_comdat_group ();
+  if (group)
+    comdat = IDENTIFIER_POINTER (group);
+  else
+    comdat = "";
+  lto_output_data_stream (ob->main_stream, comdat, strlen (comdat) + 1);
+  if (group)
     {
-      ref = lto_symtab_encoder_lookup (encoder,
-				       node->same_comdat_group);
-      gcc_assert (ref != LCC_NOT_FOUND);
+      if (node->same_comdat_group && !boundary_p)
+	{
+	  ref = lto_symtab_encoder_lookup (encoder,
+					   node->same_comdat_group);
+	  gcc_assert (ref != LCC_NOT_FOUND);
+	}
+      else
+	ref = LCC_NOT_FOUND;
+      streamer_write_hwi_stream (ob->main_stream, ref);
     }
-  else
-    ref = LCC_NOT_FOUND;
-  streamer_write_hwi_stream (ob->main_stream, ref);
   streamer_write_enum (ob->main_stream, ld_plugin_symbol_resolution,
 		       LDPR_NUM_KNOWN, node->resolution);
 }
@@ -946,6 +968,26 @@ output_symtab (void)
   output_refs (encoder);
 }
 
+/* Return COMDAT_GROUP encoded in IB as a plain string.  */
+
+static tree
+read_comdat_group (struct lto_input_block *ib)
+{
+  unsigned int len = strnlen (ib->data + ib->p, ib->len - ib->p - 1);
+  tree group;
+
+  if (ib->data[ib->p + len])
+    lto_section_overrun (ib);
+  if (!len)
+    {
+      ib->p++;
+      return NULL;
+    }
+  group = get_identifier (ib->data + ib->p);
+  ib->p += len;
+  return group;
+}
+
 /* Overwrite the information in NODE based on FILE_DATA, TAG, FLAGS,
    STACK_SIZE, SELF_TIME and SELF_SIZE.  This is called either to initialize
    NODE or to replace the values in it, for instance because the first
@@ -1034,6 +1076,7 @@ input_node (struct lto_file_decl_data *f
   int clone_ref;
   int order;
   int i, count;
+  tree group;
 
   order = streamer_read_hwi (ib) + order_base;
   clone_ref = streamer_read_hwi (ib);
@@ -1079,7 +1122,9 @@ input_node (struct lto_file_decl_data *f
   if (tag == LTO_symtab_analyzed_node)
     ref = streamer_read_hwi (ib);
 
-  ref2 = streamer_read_hwi (ib);
+  group = read_comdat_group (ib);
+  if (group)
+    ref2 = streamer_read_hwi (ib);
 
   /* Make sure that we have not read this node before.  Nodes that
      have already been read will have their tag stored in the 'aux'
@@ -1098,8 +1143,14 @@ input_node (struct lto_file_decl_data *f
   /* Store a reference for now, and fix up later to be a pointer.  */
   node->global.inlined_to = (cgraph_node_ptr) (intptr_t) ref;
 
-  /* Store a reference for now, and fix up later to be a pointer.  */
-  node->same_comdat_group = (symtab_node *) (intptr_t) ref2;
+  if (group)
+    {
+      node->set_comdat_group (group);
+      /* Store a reference for now, and fix up later to be a pointer.  */
+      node->same_comdat_group = (symtab_node *) (intptr_t) ref2;
+    }
+  else
+    node->same_comdat_group = (symtab_node *) (intptr_t) LCC_NOT_FOUND;
 
   if (node->thunk.thunk_p)
     {
@@ -1131,6 +1182,7 @@ input_varpool_node (struct lto_file_decl
   struct bitpack_d bp;
   int ref = LCC_NOT_FOUND;
   int order;
+  tree group;
 
   order = streamer_read_hwi (ib) + order_base;
   decl_index = streamer_read_uhwi (ib);
@@ -1168,9 +1220,16 @@ input_varpool_node (struct lto_file_decl
     }
   if (node->alias && !node->analyzed && node->weakref)
     node->alias_target = get_alias_symbol (node->decl);
-  ref = streamer_read_hwi (ib);
-  /* Store a reference for now, and fix up later to be a pointer.  */
-  node->same_comdat_group = (symtab_node *) (intptr_t) ref;
+  group = read_comdat_group (ib);
+  if (group)
+    {
+      node->set_comdat_group (group);
+      ref = streamer_read_hwi (ib);
+      /* Store a reference for now, and fix up later to be a pointer.  */
+      node->same_comdat_group = (symtab_node *) (intptr_t) ref;
+    }
+  else
+    node->same_comdat_group = (symtab_node *) (intptr_t) LCC_NOT_FOUND;
   node->resolution = streamer_read_enum (ib, ld_plugin_symbol_resolution,
 					        LDPR_NUM_KNOWN);
   gcc_assert (flag_ltrans
Index: c/c-decl.c
===================================================================
--- c/c-decl.c	(revision 210887)
+++ c/c-decl.c	(working copy)
@@ -2507,8 +2507,18 @@ merge_decls (tree newdecl, tree olddecl,
     switch (TREE_CODE (olddecl))
       {
       case FUNCTION_DECL:
-      case FIELD_DECL:
       case VAR_DECL:
+	{
+	  struct symtab_node *snode = olddecl->decl_with_vis.symtab_node;
+
+	  memcpy ((char *) olddecl + sizeof (struct tree_decl_common),
+		  (char *) newdecl + sizeof (struct tree_decl_common),
+		  tree_code_size (TREE_CODE (olddecl)) - sizeof (struct tree_decl_common));
+	  olddecl->decl_with_vis.symtab_node = snode;
+	  break;
+	}
+
+      case FIELD_DECL:
       case PARM_DECL:
       case LABEL_DECL:
       case RESULT_DECL:
@@ -2561,6 +2571,9 @@ duplicate_decls (tree newdecl, tree oldd
     }
 
   merge_decls (newdecl, olddecl, newtype, oldtype);
+
+  /* The NEWDECL will no longer be needed.  */
+  ggc_free (newdecl);
   return true;
 }
 
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 210887)
+++ trans-mem.c	(working copy)
@@ -4852,7 +4852,7 @@ ipa_tm_create_version_alias (struct cgra
 
   /* Perform the same remapping to the comdat group.  */
   if (DECL_ONE_ONLY (new_decl))
-    DECL_COMDAT_GROUP (new_decl) = tm_mangle (DECL_COMDAT_GROUP (old_decl));
+    varpool_get_node (new_decl)->set_comdat_group (tm_mangle (DECL_COMDAT_GROUP (old_decl)));
 
   new_node = cgraph_same_body_alias (NULL, new_decl, info->new_decl);
   new_node->tm_clone = true;
@@ -4892,7 +4892,7 @@ ipa_tm_create_version (struct cgraph_nod
 
   /* Perform the same remapping to the comdat group.  */
   if (DECL_ONE_ONLY (new_decl))
-    DECL_COMDAT_GROUP (new_decl) = tm_mangle (DECL_COMDAT_GROUP (old_decl));
+    varpool_get_node (new_decl)->set_comdat_group (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);
Index: config/mips/mips.c
===================================================================
--- config/mips/mips.c	(revision 210887)
+++ config/mips/mips.c	(working copy)
@@ -6275,7 +6275,7 @@ mips_start_unique_function (const char *
   TREE_PUBLIC (decl) = 1;
   TREE_STATIC (decl) = 1;
 
-  DECL_COMDAT_GROUP (decl) = DECL_ASSEMBLER_NAME (decl);
+  cgraph_create_node (decl)->set_comdat_group (DECL_ASSEMBLER_NAME (decl));
 
   targetm.asm_out.unique_section (decl, 0);
   switch_to_section (get_named_section (decl, NULL, 0));
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 210887)
+++ config/i386/i386.c	(working copy)
@@ -9183,7 +9183,7 @@ ix86_code_end (void)
 #endif
       if (USE_HIDDEN_LINKONCE)
 	{
-	  DECL_COMDAT_GROUP (decl) = DECL_ASSEMBLER_NAME (decl);
+	  cgraph_create_node (decl)->set_comdat_group (DECL_ASSEMBLER_NAME (decl));
 
 	  targetm.asm_out.unique_section (decl, 0);
 	  switch_to_section (get_named_section (decl, NULL, 0));
Index: config/rs6000/rs6000.c
===================================================================
--- config/rs6000/rs6000.c	(revision 210887)
+++ config/rs6000/rs6000.c	(working copy)
@@ -32331,7 +32331,7 @@ rs6000_code_end (void)
 #if RS6000_WEAK
   if (USE_HIDDEN_LINKONCE)
     {
-      DECL_COMDAT_GROUP (decl) = DECL_ASSEMBLER_NAME (decl);
+      cgraph_create_node (decl)->set_comdat_group (DECL_ASSEMBLER_NAME (decl));
       targetm.asm_out.unique_section (decl, 0);
       switch_to_section (get_named_section (decl, NULL, 0));
       DECL_WEAK (decl) = 1;
Index: lto/lto-symtab.c
===================================================================
--- lto/lto-symtab.c	(revision 210887)
+++ lto/lto-symtab.c	(working copy)
@@ -644,7 +644,7 @@ lto_symtab_merge_symbols (void)
 		       && cnode2 != cnode)
 		cgraph_remove_node (cnode2);
 
-	      symtab_insert_node_to_hashtable (node);
+	      node->decl->decl_with_vis.symtab_node = node;
 	    }
 	}
     }
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 210887)
+++ lto/lto.c	(working copy)
@@ -1530,7 +1530,6 @@ compare_tree_sccs_1 (tree t1, tree t2, t
 	compare_tree_edges (DECL_ASSEMBLER_NAME (t1),
 			    DECL_ASSEMBLER_NAME (t2));
       compare_tree_edges (DECL_SECTION_NAME (t1), DECL_SECTION_NAME (t2));
-      compare_tree_edges (DECL_COMDAT_GROUP (t1), DECL_COMDAT_GROUP (t2));
     }
 
   if (CODE_CONTAINS_STRUCT (code, TS_FIELD_DECL))
Index: tree-streamer-out.c
===================================================================
--- tree-streamer-out.c	(revision 210887)
+++ tree-streamer-out.c	(working copy)
@@ -662,7 +662,6 @@ write_ts_decl_with_vis_tree_pointers (st
     stream_write_tree (ob, NULL_TREE, false);
 
   stream_write_tree (ob, DECL_SECTION_NAME (expr), ref_p);
-  stream_write_tree (ob, DECL_COMDAT_GROUP (expr), ref_p);
 }
 
 

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Jan Hubicka]

> Well, allocate_struct_function has a abstract_p argument for that.  But
> yes, a simple patch like
> 
> Index: gcc/function.c
> ===================================================================
> --- gcc/function.c      (revision 210845)
> +++ gcc/function.c      (working copy)
> @@ -64,6 +64,7 @@ along with GCC; see the file COPYING3.
>  #include "params.h"
>  #include "bb-reorder.h"
>  #include "shrink-wrap.h"
> +#include "cgraph.h"
> 
>  /* So we can assign to cfun in this file.  */
>  #undef cfun
> @@ -4512,6 +4513,8 @@ allocate_struct_function (tree fndecl, b
> 
>    if (fndecl != NULL_TREE)
>      {
> +      if (!abstract_p)
> +       cgraph_get_create_node (fndecl);
>        DECL_STRUCT_FUNCTION (fndecl) = cfun;
>        cfun->decl = fndecl;
>        current_function_funcdef_no = get_next_funcdef_no ();
> 
> ICEs during bootstrap with (at least)
> 
> /space/rguenther/src/svn/trunk/libgcc/config/i386/cpuinfo.c:405:1:
> error: node differs from symtab decl hashtable
>  }
>  ^
> __get_cpuid_max.constprop.0/42 (__get_cpuid_max.constprop) @0x7ff486232290
>   Type: function definition analyzed
>   Visibility: artificial
>   previous sharing asm name: 43
>   References:
>   Referring:
>   Function __get_cpuid_max.constprop/42 is inline copy in __get_cpuid_output/40
>   Availability: local
>   First run: 0
>   Function flags: local only_called_at_startup
>   Called by: __get_cpuid_output/40 (1.00 per call) (inlined)
>   Calls:
> /space/rguenther/src/svn/trunk/libgcc/config/i386/cpuinfo.c:405:1:
> internal compiler error: verify_cgraph_node failed
> 
> so I guess we would need to have a way to create a "dummy" cgraph
> node first and later populate it properly.

Yep, I was wondering about following:

struct GTY(()) tree_decl_with_vis {
 struct tree_decl_with_rtl common;
 tree assembler_name;
 tree section_name;
 tree comdat_group;

for majority of decl_with_vis we create we won't have section name/assembler
name and comdat group.

We already do memory optimization for INIT_PRIORITY in the following way:
#define DECL_HAS_INIT_PRIORITY_P(NODE) \
  (VAR_DECL_CHECK (NODE)->decl_with_vis.init_priority_p)
#define DECL_INIT_PRIORITY(NODE) \
  (decl_init_priority_lookup (NODE))

where init priority sits on separate hashtab.

I think at the moment we can't move assembler_name/section_name/comat_group
all to symbol table, because we do need the for off-symbol table things
(CONST_DECL, LABEL_DECL, off symtab VAR_DECL).
But I would suggest dropping them to off-tree hashtables, too, and have
pointer in symtab.
The accessor would be then something like

tree get_comdat_group (tree node)
{
  if (!node->decl_with_vis.comdat_group_p)
    return NULL;
  if (node->decl_with_vis.symtab_node)
    return node->decl_with_vis.symtab_node->comdat_group;
  decl_comdat_group_loop (node);
}
> 
> But as we currently have a back-pointer from struct function to fndecl
> it would be nice to hook the cgraph node in there - that way we get
> away without any extra pointer (we could even save symtab decl
> pointer and create a cyclic fndecl -> cgraph -> function -> fndecl
> chain ...).
> 
> I'm fine with enlarging tree_function_decl for now - ideally we'd push
> stuff from it elsewhere (like target and optimization option tree nodes,
> or most of the visibility and symbol related stuff).  Not sure why
> tree_type_decl inherits from tree_decl_non_common (and thus
> tree_decl_with_vis).  Probably because of the non-common parts
> being (ab-)used by FEs.  Otherwise I'd say simply put a symtab
> node pointer into tree_decl_with_vis ... (can we move
> section_name and comdat_group more easily than assembler_name?)

Lets see, I suppose putting it on side first is good incremental step.
Then we need to revisit frontends to avoid defining those too early,
that might be relatively simple to do (at least for C++ FE, I think it all
is defined at a time we call import/export decl.)

Honza
> 
> Richard.
> 
> > Honza
> >>
> >> Richard.
> >>
> >> >I think we may be on better track moving DECL_ASSEMBLER_NAME that is
> >> >calculated later,
> >> >but then we have problem with DECL_ASSEMBLER_NAME being set for
> >> >assembler names and
> >> >const decls, too that still go around symtab.
> >> >Given that decl_assembler_name is a function, I suppose we could go
> >> >with extra conditoinal
> >> >in there.
> >> >
> >> >Getting struct function out of frontend busyness would be nice indeed,
> >> >too, but probably
> >> >should be independent of Martin's work here.
> >> >
> >> >Honza
> >> >>
> >> >> Thanks,
> >> >> Richard.
> >> >>
> >> >> > Thanks,
> >> >> >
> >> >> > Martin
> >> >> >
> >> >> >>
> >> >> >> > +       }
> >> >> >> >      }
> >> >> >> > +
> >> >> >> > +  return ret;
> >> >> >> >  }
> >> >> >> >
> >> >> >> >  /* Detects return flags for the call STMT.  */
> >> >> >> >
> >>

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Richard Biener]

On Thu, May 22, 2014 at 8:11 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >It won't be so easy, because struct function is really built at
>> >relatively convoluted
>> >places within frontend before cgraph node is assigned to them (I tried
>> >that few years
>> >back).
>>
>> Well, just call cgraph create node from struct Funktion allocation.
>
> That will make uninstantiated templates to land symbol table (and if you have
> aliases, also do the assembler name mangling) that is not that cool either :(

Well, allocate_struct_function has a abstract_p argument for that.  But
yes, a simple patch like

Index: gcc/function.c
===================================================================
--- gcc/function.c      (revision 210845)
+++ gcc/function.c      (working copy)
@@ -64,6 +64,7 @@ along with GCC; see the file COPYING3.
 #include "params.h"
 #include "bb-reorder.h"
 #include "shrink-wrap.h"
+#include "cgraph.h"

 /* So we can assign to cfun in this file.  */
 #undef cfun
@@ -4512,6 +4513,8 @@ allocate_struct_function (tree fndecl, b

   if (fndecl != NULL_TREE)
     {
+      if (!abstract_p)
+       cgraph_get_create_node (fndecl);
       DECL_STRUCT_FUNCTION (fndecl) = cfun;
       cfun->decl = fndecl;
       current_function_funcdef_no = get_next_funcdef_no ();

ICEs during bootstrap with (at least)

/space/rguenther/src/svn/trunk/libgcc/config/i386/cpuinfo.c:405:1:
error: node differs from symtab decl hashtable
 }
 ^
__get_cpuid_max.constprop.0/42 (__get_cpuid_max.constprop) @0x7ff486232290
  Type: function definition analyzed
  Visibility: artificial
  previous sharing asm name: 43
  References:
  Referring:
  Function __get_cpuid_max.constprop/42 is inline copy in __get_cpuid_output/40
  Availability: local
  First run: 0
  Function flags: local only_called_at_startup
  Called by: __get_cpuid_output/40 (1.00 per call) (inlined)
  Calls:
/space/rguenther/src/svn/trunk/libgcc/config/i386/cpuinfo.c:405:1:
internal compiler error: verify_cgraph_node failed

so I guess we would need to have a way to create a "dummy" cgraph
node first and later populate it properly.

But as we currently have a back-pointer from struct function to fndecl
it would be nice to hook the cgraph node in there - that way we get
away without any extra pointer (we could even save symtab decl
pointer and create a cyclic fndecl -> cgraph -> function -> fndecl
chain ...).

I'm fine with enlarging tree_function_decl for now - ideally we'd push
stuff from it elsewhere (like target and optimization option tree nodes,
or most of the visibility and symbol related stuff).  Not sure why
tree_type_decl inherits from tree_decl_non_common (and thus
tree_decl_with_vis).  Probably because of the non-common parts
being (ab-)used by FEs.  Otherwise I'd say simply put a symtab
node pointer into tree_decl_with_vis ... (can we move
section_name and comdat_group more easily than assembler_name?)

Richard.

> Honza
>>
>> Richard.
>>
>> >I think we may be on better track moving DECL_ASSEMBLER_NAME that is
>> >calculated later,
>> >but then we have problem with DECL_ASSEMBLER_NAME being set for
>> >assembler names and
>> >const decls, too that still go around symtab.
>> >Given that decl_assembler_name is a function, I suppose we could go
>> >with extra conditoinal
>> >in there.
>> >
>> >Getting struct function out of frontend busyness would be nice indeed,
>> >too, but probably
>> >should be independent of Martin's work here.
>> >
>> >Honza
>> >>
>> >> Thanks,
>> >> Richard.
>> >>
>> >> > Thanks,
>> >> >
>> >> > Martin
>> >> >
>> >> >>
>> >> >> > +       }
>> >> >> >      }
>> >> >> > +
>> >> >> > +  return ret;
>> >> >> >  }
>> >> >> >
>> >> >> >  /* Detects return flags for the call STMT.  */
>> >> >> >
>>

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Jan Hubicka]

> >It won't be so easy, because struct function is really built at
> >relatively convoluted
> >places within frontend before cgraph node is assigned to them (I tried
> >that few years
> >back).
> 
> Well, just call cgraph create node from struct Funktion allocation.

That will make uninstantiated templates to land symbol table (and if you have
aliases, also do the assembler name mangling) that is not that cool either :(

Honza
> 
> Richard.
> 
> >I think we may be on better track moving DECL_ASSEMBLER_NAME that is
> >calculated later,
> >but then we have problem with DECL_ASSEMBLER_NAME being set for
> >assembler names and
> >const decls, too that still go around symtab.
> >Given that decl_assembler_name is a function, I suppose we could go
> >with extra conditoinal
> >in there.
> >
> >Getting struct function out of frontend busyness would be nice indeed,
> >too, but probably
> >should be independent of Martin's work here.
> >
> >Honza
> >> 
> >> Thanks,
> >> Richard.
> >> 
> >> > Thanks,
> >> >
> >> > Martin
> >> >
> >> >>
> >> >> > +       }
> >> >> >      }
> >> >> > +
> >> >> > +  return ret;
> >> >> >  }
> >> >> >
> >> >> >  /* Detects return flags for the call STMT.  */
> >> >> >
> 

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Richard Biener]

On May 22, 2014 5:24:33 PM CEST, Jan Hubicka <hubicka@ucw.cz> wrote:
>> 
>> Can we?  If the body is not readily available we only have decl and
>> cgraph-node, not struct function.
>> 
>> I suppose we could exchange the struct function pointer in
>> tree_function_decl for a cgraph_node pointer and put
>> the struct function pointer into the cgraph_node.
>> 
>> Of course that may have impacts on FEs who might create
>> struct function before creating a cgraph node.  But at least
>> it would avoid enlarging FUNCTION_DECL.
>> 
>> In the end most of the tree_decl_with_vis stuff should move over to
>symtab
>> and var-decls should get a varpool_node pointer as well.
>> 
>> Back to the call flags stuff - I also meant the representation of the
>> "fn spec" attribute.  Rather than parsing that again and again move
>> it to a better place (which you seem to invent?) and better unified
>> representation.
>> 
>> Can you try if removing the cgraph hash is possible with the
>> struct function pointer idea?
>
>It won't be so easy, because struct function is really built at
>relatively convoluted
>places within frontend before cgraph node is assigned to them (I tried
>that few years
>back).

Well, just call cgraph create node from struct Funktion allocation.

Richard.

>I think we may be on better track moving DECL_ASSEMBLER_NAME that is
>calculated later,
>but then we have problem with DECL_ASSEMBLER_NAME being set for
>assembler names and
>const decls, too that still go around symtab.
>Given that decl_assembler_name is a function, I suppose we could go
>with extra conditoinal
>in there.
>
>Getting struct function out of frontend busyness would be nice indeed,
>too, but probably
>should be independent of Martin's work here.
>
>Honza
>> 
>> Thanks,
>> Richard.
>> 
>> > Thanks,
>> >
>> > Martin
>> >
>> >>
>> >> > +       }
>> >> >      }
>> >> > +
>> >> > +  return ret;
>> >> >  }
>> >> >
>> >> >  /* Detects return flags for the call STMT.  */
>> >> >

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Jan Hubicka]

> 
> Can we?  If the body is not readily available we only have decl and
> cgraph-node, not struct function.
> 
> I suppose we could exchange the struct function pointer in
> tree_function_decl for a cgraph_node pointer and put
> the struct function pointer into the cgraph_node.
> 
> Of course that may have impacts on FEs who might create
> struct function before creating a cgraph node.  But at least
> it would avoid enlarging FUNCTION_DECL.
> 
> In the end most of the tree_decl_with_vis stuff should move over to symtab
> and var-decls should get a varpool_node pointer as well.
> 
> Back to the call flags stuff - I also meant the representation of the
> "fn spec" attribute.  Rather than parsing that again and again move
> it to a better place (which you seem to invent?) and better unified
> representation.
> 
> Can you try if removing the cgraph hash is possible with the
> struct function pointer idea?

It won't be so easy, because struct function is really built at relatively convoluted
places within frontend before cgraph node is assigned to them (I tried that few years
back).
I think we may be on better track moving DECL_ASSEMBLER_NAME that is calculated later,
but then we have problem with DECL_ASSEMBLER_NAME being set for assembler names and
const decls, too that still go around symtab.
Given that decl_assembler_name is a function, I suppose we could go with extra conditoinal
in there.

Getting struct function out of frontend busyness would be nice indeed, too, but probably
should be independent of Martin's work here.

Honza
> 
> Thanks,
> Richard.
> 
> > Thanks,
> >
> > Martin
> >
> >>
> >> > +       }
> >> >      }
> >> > +
> >> > +  return ret;
> >> >  }
> >> >
> >> >  /* Detects return flags for the call STMT.  */
> >> >

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Richard Biener]

On Thu, May 22, 2014 at 2:49 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> On Wed, May 21, 2014 at 04:27:32PM +0200, Richard Biener wrote:
>> On Wed, May 21, 2014 at 3:16 PM, Martin Jambor <mjambor@suse.cz> wrote:
>> > Hi,
>> >
>> > this demonstrates how results of ipa-prop escape analysis from
>> > previous patches can be used at a later stage of compilation by
>> > directly returning them from gimple_call_arg_flags which currently
>> > relies on fnspec annotations.
>> >
>> > Bootstrapped and tested on x86_64-linux and also passes LTO bootstrap.
>> > I have only had a brief look at behavior of this in SPEC 2006 and for
>> > example in astar 1.19% of invocations of gimple_call_arg_flags return
>> > noescape where we previously never did and in calculix this increases
>> > from 15.62% (from annotations) to 18.14%.  Noclobber flag is reported
>> > far less often still but for example in gamess that number raises from
>> > 5.21% to 7.66%.
>> >
>> > Thanks,
>> >
>> > Martin
>> >
>> >
>> > 2014-04-30  Martin Jambor  <mjambor@suse.cz>
>> >
>> >         * gimple.c: Include cgraph.h.
>> >         (gimple_call_arg_flags): Also query bitmaps in cgraph_node.
>> >
>> > Index: src/gcc/gimple.c
>> > ===================================================================
>> > --- src.orig/gcc/gimple.c
>> > +++ src/gcc/gimple.c
>> > @@ -47,7 +47,7 @@ along with GCC; see the file COPYING3.
>> >  #include "demangle.h"
>> >  #include "langhooks.h"
>> >  #include "bitmap.h"
>> > -
>> > +#include "cgraph.h"
>> >
>> >  /* All the tuples have their operand vector (if present) at the very bottom
>> >     of the structure.  Therefore, the offset required to find the
>> > @@ -1349,32 +1349,50 @@ int
>> >  gimple_call_arg_flags (const_gimple stmt, unsigned arg)
>> >  {
>> >    tree attr = gimple_call_fnspec (stmt);
>> > +  int ret;
>> >
>> > -  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
>> > -    return 0;
>> > -
>> > -  switch (TREE_STRING_POINTER (attr)[1 + arg])
>> > +  if (attr && 1 + arg < (unsigned) TREE_STRING_LENGTH (attr))
>> >      {
>> > -    case 'x':
>> > -    case 'X':
>> > -      return EAF_UNUSED;
>> > -
>> > -    case 'R':
>> > -      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
>> > -
>> > -    case 'r':
>> > -      return EAF_NOCLOBBER | EAF_NOESCAPE;
>> > -
>> > -    case 'W':
>> > -      return EAF_DIRECT | EAF_NOESCAPE;
>> > -
>> > -    case 'w':
>> > -      return EAF_NOESCAPE;
>> > +      switch (TREE_STRING_POINTER (attr)[1 + arg])
>> > +       {
>> > +       case 'x':
>> > +       case 'X':
>> > +         ret = EAF_UNUSED;
>> > +         break;
>> > +       case 'R':
>> > +         ret = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
>> > +         break;
>> > +       case 'r':
>> > +         ret = EAF_NOCLOBBER | EAF_NOESCAPE;
>> > +         break;
>> > +       case 'W':
>> > +         ret = EAF_DIRECT | EAF_NOESCAPE;
>> > +         break;
>> > +       case 'w':
>> > +         ret = EAF_NOESCAPE;
>> > +         break;
>> > +       case '.':
>> > +       default:
>> > +         ret = 0;
>> > +       }
>> > +    }
>> > +  else
>> > +    ret = 0;
>> >
>> > -    case '.':
>> > -    default:
>> > -      return 0;
>> > +  tree callee_decl = gimple_call_fndecl (stmt);
>> > +  if (callee_decl)
>> > +    {
>> > +      cgraph_node *callee_node = cgraph_get_node (callee_decl);
>> > +      if (callee_node)
>> > +       {
>> > +         if (cgraph_param_noescape_p (callee_node, arg))
>> > +           ret |= EAF_NOESCAPE;
>> > +         if (cgraph_param_noclobber_p (callee_node, arg))
>> > +           ret |= EAF_NOCLOBBER;
>>
>> That's quite expensive.  I guess we need a better way to store
>> those?
>
> if we want to avoid the cgraph_node lookup, then I think we need to
> store this information in the decl or struct function.  That is
> certainly possible and might even be more appropriate.

Can we?  If the body is not readily available we only have decl and
cgraph-node, not struct function.

I suppose we could exchange the struct function pointer in
tree_function_decl for a cgraph_node pointer and put
the struct function pointer into the cgraph_node.

Of course that may have impacts on FEs who might create
struct function before creating a cgraph node.  But at least
it would avoid enlarging FUNCTION_DECL.

In the end most of the tree_decl_with_vis stuff should move over to symtab
and var-decls should get a varpool_node pointer as well.

Back to the call flags stuff - I also meant the representation of the
"fn spec" attribute.  Rather than parsing that again and again move
it to a better place (which you seem to invent?) and better unified
representation.

Can you try if removing the cgraph hash is possible with the
struct function pointer idea?

Thanks,
Richard.

> Thanks,
>
> Martin
>
>>
>> > +       }
>> >      }
>> > +
>> > +  return ret;
>> >  }
>> >
>> >  /* Detects return flags for the call STMT.  */
>> >

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Martin Jambor]

Hi,

On Wed, May 21, 2014 at 04:27:32PM +0200, Richard Biener wrote:
> On Wed, May 21, 2014 at 3:16 PM, Martin Jambor <mjambor@suse.cz> wrote:
> > Hi,
> >
> > this demonstrates how results of ipa-prop escape analysis from
> > previous patches can be used at a later stage of compilation by
> > directly returning them from gimple_call_arg_flags which currently
> > relies on fnspec annotations.
> >
> > Bootstrapped and tested on x86_64-linux and also passes LTO bootstrap.
> > I have only had a brief look at behavior of this in SPEC 2006 and for
> > example in astar 1.19% of invocations of gimple_call_arg_flags return
> > noescape where we previously never did and in calculix this increases
> > from 15.62% (from annotations) to 18.14%.  Noclobber flag is reported
> > far less often still but for example in gamess that number raises from
> > 5.21% to 7.66%.
> >
> > Thanks,
> >
> > Martin
> >
> >
> > 2014-04-30  Martin Jambor  <mjambor@suse.cz>
> >
> >         * gimple.c: Include cgraph.h.
> >         (gimple_call_arg_flags): Also query bitmaps in cgraph_node.
> >
> > Index: src/gcc/gimple.c
> > ===================================================================
> > --- src.orig/gcc/gimple.c
> > +++ src/gcc/gimple.c
> > @@ -47,7 +47,7 @@ along with GCC; see the file COPYING3.
> >  #include "demangle.h"
> >  #include "langhooks.h"
> >  #include "bitmap.h"
> > -
> > +#include "cgraph.h"
> >
> >  /* All the tuples have their operand vector (if present) at the very bottom
> >     of the structure.  Therefore, the offset required to find the
> > @@ -1349,32 +1349,50 @@ int
> >  gimple_call_arg_flags (const_gimple stmt, unsigned arg)
> >  {
> >    tree attr = gimple_call_fnspec (stmt);
> > +  int ret;
> >
> > -  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
> > -    return 0;
> > -
> > -  switch (TREE_STRING_POINTER (attr)[1 + arg])
> > +  if (attr && 1 + arg < (unsigned) TREE_STRING_LENGTH (attr))
> >      {
> > -    case 'x':
> > -    case 'X':
> > -      return EAF_UNUSED;
> > -
> > -    case 'R':
> > -      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> > -
> > -    case 'r':
> > -      return EAF_NOCLOBBER | EAF_NOESCAPE;
> > -
> > -    case 'W':
> > -      return EAF_DIRECT | EAF_NOESCAPE;
> > -
> > -    case 'w':
> > -      return EAF_NOESCAPE;
> > +      switch (TREE_STRING_POINTER (attr)[1 + arg])
> > +       {
> > +       case 'x':
> > +       case 'X':
> > +         ret = EAF_UNUSED;
> > +         break;
> > +       case 'R':
> > +         ret = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> > +         break;
> > +       case 'r':
> > +         ret = EAF_NOCLOBBER | EAF_NOESCAPE;
> > +         break;
> > +       case 'W':
> > +         ret = EAF_DIRECT | EAF_NOESCAPE;
> > +         break;
> > +       case 'w':
> > +         ret = EAF_NOESCAPE;
> > +         break;
> > +       case '.':
> > +       default:
> > +         ret = 0;
> > +       }
> > +    }
> > +  else
> > +    ret = 0;
> >
> > -    case '.':
> > -    default:
> > -      return 0;
> > +  tree callee_decl = gimple_call_fndecl (stmt);
> > +  if (callee_decl)
> > +    {
> > +      cgraph_node *callee_node = cgraph_get_node (callee_decl);
> > +      if (callee_node)
> > +       {
> > +         if (cgraph_param_noescape_p (callee_node, arg))
> > +           ret |= EAF_NOESCAPE;
> > +         if (cgraph_param_noclobber_p (callee_node, arg))
> > +           ret |= EAF_NOCLOBBER;
> 
> That's quite expensive.  I guess we need a better way to store
> those?

if we want to avoid the cgraph_node lookup, then I think we need to
store this information in the decl or struct function.  That is
certainly possible and might even be more appropriate.

Thanks,

Martin

> 
> > +       }
> >      }
> > +
> > +  return ret;
> >  }
> >
> >  /* Detects return flags for the call STMT.  */
> >

Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Richard Biener]

On Wed, May 21, 2014 at 3:16 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> this demonstrates how results of ipa-prop escape analysis from
> previous patches can be used at a later stage of compilation by
> directly returning them from gimple_call_arg_flags which currently
> relies on fnspec annotations.
>
> Bootstrapped and tested on x86_64-linux and also passes LTO bootstrap.
> I have only had a brief look at behavior of this in SPEC 2006 and for
> example in astar 1.19% of invocations of gimple_call_arg_flags return
> noescape where we previously never did and in calculix this increases
> from 15.62% (from annotations) to 18.14%.  Noclobber flag is reported
> far less often still but for example in gamess that number raises from
> 5.21% to 7.66%.
>
> Thanks,
>
> Martin
>
>
> 2014-04-30  Martin Jambor  <mjambor@suse.cz>
>
>         * gimple.c: Include cgraph.h.
>         (gimple_call_arg_flags): Also query bitmaps in cgraph_node.
>
> Index: src/gcc/gimple.c
> ===================================================================
> --- src.orig/gcc/gimple.c
> +++ src/gcc/gimple.c
> @@ -47,7 +47,7 @@ along with GCC; see the file COPYING3.
>  #include "demangle.h"
>  #include "langhooks.h"
>  #include "bitmap.h"
> -
> +#include "cgraph.h"
>
>  /* All the tuples have their operand vector (if present) at the very bottom
>     of the structure.  Therefore, the offset required to find the
> @@ -1349,32 +1349,50 @@ int
>  gimple_call_arg_flags (const_gimple stmt, unsigned arg)
>  {
>    tree attr = gimple_call_fnspec (stmt);
> +  int ret;
>
> -  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
> -    return 0;
> -
> -  switch (TREE_STRING_POINTER (attr)[1 + arg])
> +  if (attr && 1 + arg < (unsigned) TREE_STRING_LENGTH (attr))
>      {
> -    case 'x':
> -    case 'X':
> -      return EAF_UNUSED;
> -
> -    case 'R':
> -      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> -
> -    case 'r':
> -      return EAF_NOCLOBBER | EAF_NOESCAPE;
> -
> -    case 'W':
> -      return EAF_DIRECT | EAF_NOESCAPE;
> -
> -    case 'w':
> -      return EAF_NOESCAPE;
> +      switch (TREE_STRING_POINTER (attr)[1 + arg])
> +       {
> +       case 'x':
> +       case 'X':
> +         ret = EAF_UNUSED;
> +         break;
> +       case 'R':
> +         ret = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> +         break;
> +       case 'r':
> +         ret = EAF_NOCLOBBER | EAF_NOESCAPE;
> +         break;
> +       case 'W':
> +         ret = EAF_DIRECT | EAF_NOESCAPE;
> +         break;
> +       case 'w':
> +         ret = EAF_NOESCAPE;
> +         break;
> +       case '.':
> +       default:
> +         ret = 0;
> +       }
> +    }
> +  else
> +    ret = 0;
>
> -    case '.':
> -    default:
> -      return 0;
> +  tree callee_decl = gimple_call_fndecl (stmt);
> +  if (callee_decl)
> +    {
> +      cgraph_node *callee_node = cgraph_get_node (callee_decl);
> +      if (callee_node)
> +       {
> +         if (cgraph_param_noescape_p (callee_node, arg))
> +           ret |= EAF_NOESCAPE;
> +         if (cgraph_param_noclobber_p (callee_node, arg))
> +           ret |= EAF_NOCLOBBER;

That's quite expensive.  I guess we need a better way to store
those?

> +       }
>      }
> +
> +  return ret;
>  }
>
>  /* Detects return flags for the call STMT.  */
>

[PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Martin Jambor]

[-- Attachment #1: plug_ipa_escape_into_pta.diff --]
[-- Type: text/plain, Size: 2782 bytes --]

Hi,

this demonstrates how results of ipa-prop escape analysis from
previous patches can be used at a later stage of compilation by
directly returning them from gimple_call_arg_flags which currently
relies on fnspec annotations.

Bootstrapped and tested on x86_64-linux and also passes LTO bootstrap.
I have only had a brief look at behavior of this in SPEC 2006 and for
example in astar 1.19% of invocations of gimple_call_arg_flags return
noescape where we previously never did and in calculix this increases
from 15.62% (from annotations) to 18.14%.  Noclobber flag is reported
far less often still but for example in gamess that number raises from
5.21% to 7.66%.

Thanks,

Martin


2014-04-30  Martin Jambor  <mjambor@suse.cz>

	* gimple.c: Include cgraph.h.
	(gimple_call_arg_flags): Also query bitmaps in cgraph_node.

Index: src/gcc/gimple.c
===================================================================
--- src.orig/gcc/gimple.c
+++ src/gcc/gimple.c
@@ -47,7 +47,7 @@ along with GCC; see the file COPYING3.
 #include "demangle.h"
 #include "langhooks.h"
 #include "bitmap.h"
-
+#include "cgraph.h"
 
 /* All the tuples have their operand vector (if present) at the very bottom
    of the structure.  Therefore, the offset required to find the
@@ -1349,32 +1349,50 @@ int
 gimple_call_arg_flags (const_gimple stmt, unsigned arg)
 {
   tree attr = gimple_call_fnspec (stmt);
+  int ret;
 
-  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
-    return 0;
-
-  switch (TREE_STRING_POINTER (attr)[1 + arg])
+  if (attr && 1 + arg < (unsigned) TREE_STRING_LENGTH (attr))
     {
-    case 'x':
-    case 'X':
-      return EAF_UNUSED;
-
-    case 'R':
-      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
-
-    case 'r':
-      return EAF_NOCLOBBER | EAF_NOESCAPE;
-
-    case 'W':
-      return EAF_DIRECT | EAF_NOESCAPE;
-
-    case 'w':
-      return EAF_NOESCAPE;
+      switch (TREE_STRING_POINTER (attr)[1 + arg])
+	{
+	case 'x':
+	case 'X':
+	  ret = EAF_UNUSED;
+	  break;
+	case 'R':
+	  ret = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
+	  break;
+	case 'r':
+	  ret = EAF_NOCLOBBER | EAF_NOESCAPE;
+	  break;
+	case 'W':
+	  ret = EAF_DIRECT | EAF_NOESCAPE;
+	  break;
+	case 'w':
+	  ret = EAF_NOESCAPE;
+	  break;
+	case '.':
+	default:
+	  ret = 0;
+	}
+    }
+  else
+    ret = 0;
 
-    case '.':
-    default:
-      return 0;
+  tree callee_decl = gimple_call_fndecl (stmt);
+  if (callee_decl)
+    {
+      cgraph_node *callee_node = cgraph_get_node (callee_decl);
+      if (callee_node)
+	{
+	  if (cgraph_param_noescape_p (callee_node, arg))
+	    ret |= EAF_NOESCAPE;
+	  if (cgraph_param_noclobber_p (callee_node, arg))
+	    ret |= EAF_NOCLOBBER;
+	}
     }
+
+  return ret;
 }
 
 /* Detects return flags for the call STMT.  */