Silence merge warnings on artiical types

Silence merge warnings on artiical types

[Jan Hubicka]

Hi,
when compiling C++ program that define different number of virtual method
in different classes, we output warnings about their virtual tables
being of different type. THese warnings looks ugly and we are able
to diagnose the situation in more sensible way.

This patch simply silences all warnings on artificial decls.

Bootstrapped/regtested x86_64-linux OK?
Honza

	* lto-symtab.c (lto_symtab_merge_decls_2): Silence warnings on
	artificial decls.
Index: lto/lto-symtab.c
===================================================================
--- lto/lto-symtab.c	(revision 221757)
+++ lto/lto-symtab.c	(working copy)
@@ -473,7 +473,8 @@ lto_symtab_merge_decls_2 (symtab_node *f
     if (TREE_PUBLIC (e->decl))
       {
 	if (!lto_symtab_merge (prevailing, e)
-	    && !diagnosed_p)
+	    && !diagnosed_p
+	    && !DECL_ARTIFICIAL (e->decl))
 	  mismatches.safe_push (e->decl);
       }
   if (mismatches.is_empty ())

Re: Silence merge warnings on artiical types

[Richard Biener]

On Mon, 30 Mar 2015, Jan Hubicka wrote:

> Hi,
> when compiling C++ program that define different number of virtual method
> in different classes, we output warnings about their virtual tables
> being of different type. THese warnings looks ugly and we are able
> to diagnose the situation in more sensible way.
> 
> This patch simply silences all warnings on artificial decls.
> 
> Bootstrapped/regtested x86_64-linux OK?

Hum...  but do ODR warnings cover all other cases here?  Well,
the warnings won't be useful to users I suppose.

Thus, ok.

Thanks,
Richard.

> Honza
> 
> 	* lto-symtab.c (lto_symtab_merge_decls_2): Silence warnings on
> 	artificial decls.
> Index: lto/lto-symtab.c
> ===================================================================
> --- lto/lto-symtab.c	(revision 221757)
> +++ lto/lto-symtab.c	(working copy)
> @@ -473,7 +473,8 @@ lto_symtab_merge_decls_2 (symtab_node *f
>      if (TREE_PUBLIC (e->decl))
>        {
>  	if (!lto_symtab_merge (prevailing, e)
> -	    && !diagnosed_p)
> +	    && !diagnosed_p
> +	    && !DECL_ARTIFICIAL (e->decl))
>  	  mismatches.safe_push (e->decl);
>        }
>    if (mismatches.is_empty ())
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)

Re: Silence merge warnings on artiical types

[Jan Hubicka]

> On Mon, 30 Mar 2015, Jan Hubicka wrote:
> 
> > Hi,
> > when compiling C++ program that define different number of virtual method
> > in different classes, we output warnings about their virtual tables
> > being of different type. THese warnings looks ugly and we are able
> > to diagnose the situation in more sensible way.
> > 
> > This patch simply silences all warnings on artificial decls.
> > 
> > Bootstrapped/regtested x86_64-linux OK?
> 
> Hum...  but do ODR warnings cover all other cases here?  Well,

For C++ vtables and typeinfos it should. I am not at all sure what
other kinds of artifical & public declarations we produce.

> the warnings won't be useful to users I suppose.

yep, that was the main motivation.
Perhaps for development purposes we could drop a message into .cgraph dump (and
eventaully break it into .merge dump)

Honza
> 
> Thus, ok.
> 
> Thanks,
> Richard.
> 
> > Honza
> > 
> > 	* lto-symtab.c (lto_symtab_merge_decls_2): Silence warnings on
> > 	artificial decls.
> > Index: lto/lto-symtab.c
> > ===================================================================
> > --- lto/lto-symtab.c	(revision 221757)
> > +++ lto/lto-symtab.c	(working copy)
> > @@ -473,7 +473,8 @@ lto_symtab_merge_decls_2 (symtab_node *f
> >      if (TREE_PUBLIC (e->decl))
> >        {
> >  	if (!lto_symtab_merge (prevailing, e)
> > -	    && !diagnosed_p)
> > +	    && !diagnosed_p
> > +	    && !DECL_ARTIFICIAL (e->decl))
> >  	  mismatches.safe_push (e->decl);
> >        }
> >    if (mismatches.is_empty ())
> > 
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
> Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)

Re: Silence merge warnings on artiical types

[Ilya Verbin]

On Mon, Mar 30, 2015 at 05:02:57 +0200, Jan Hubicka wrote:
> 	* lto-symtab.c (lto_symtab_merge_decls_2): Silence warnings on
> 	artificial decls.

Shouldn't this patch fix libgomp.c++/target-3.C in an offloading-enabled
configuration?  It still fails...

libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: warning: type 'struct .omp_data_s.7' violates one definition rule [-Wodr]
     #pragma omp parallel for reduction(+:s)
             ^
     #pragma omp parallel for reduction(+:s)
             ^
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^

  -- Ilya

Re: Silence merge warnings on artiical types

[Jan Hubicka]

> On Mon, Mar 30, 2015 at 05:02:57 +0200, Jan Hubicka wrote:
> > 	* lto-symtab.c (lto_symtab_merge_decls_2): Silence warnings on
> > 	artificial decls.
> 
> Shouldn't this patch fix libgomp.c++/target-3.C in an offloading-enabled
> configuration?  It still fails...
> 
> libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: warning: type 'struct .omp_data_s.7' violates one definition rule [-Wodr]
>      #pragma omp parallel for reduction(+:s)
>              ^
>      #pragma omp parallel for reduction(+:s)
>              ^
>    double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
>                  ^
>    double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
>                  ^

No, that is an different warning.  Why those types are different? Is this a full warning output?

Honza
> 
>   -- Ilya

Re: Silence merge warnings on artiical types

[Ilya Verbin]

On Mon, Mar 30, 2015 at 19:06:39 +0200, Jan Hubicka wrote:
> > On Mon, Mar 30, 2015 at 05:02:57 +0200, Jan Hubicka wrote:
> > > 	* lto-symtab.c (lto_symtab_merge_decls_2): Silence warnings on
> > > 	artificial decls.
> > 
> > Shouldn't this patch fix libgomp.c++/target-3.C in an offloading-enabled
> > configuration?  It still fails...
> > 
> > libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: warning: type 'struct .omp_data_s.7' violates one definition rule [-Wodr]
> >      #pragma omp parallel for reduction(+:s)
> >              ^
> >      #pragma omp parallel for reduction(+:s)
> >              ^
> >    double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
> >                  ^
> >    double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
> >                  ^
> 
> No, that is an different warning.  Why those types are different? Is this a full warning output?

Here is full output:

spawn -ignore SIGHUP /gcc_build_host/gcc/xgcc -B/gcc_build_host/gcc/ /gcc/libgomp/testsuite/libgomp.c++/target-3.C -B/gcc_build_host/x86_64-pc-linux-gnu/./libgomp/ -B/gcc_build_host/x86_64-pc-linux-gnu/./libgomp/.libs -I/gcc_build_host/x86_64-pc-linux-gnu/./libgomp -I/gcc/libgomp/testsuite/../../include -I/gcc/libgomp/testsuite/.. -fmessage-length=0 -fno-diagnostics-show-caret -fdiagnostics-color=never -B/gcc_install/usr/local//libexec/gcc/x86_64-pc-linux-gnu/5.0.0 -B/gcc_install/usr/local//gcc -B/gcc_install/usr/local//bin -fopenmp -nostdinc++ -I/gcc_build_host/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu -I/gcc_build_host/x86_64-pc-linux-gnu/libstdc++-v3/include -I/gcc/libstdc++-v3/libsupc++ -I/gcc/libstdc++-v3/include/backward -I/gcc/libstdc++-v3/testsuite/util -B/gcc_build_host/x86_64-pc-linux-gnu/./libgomp/../libstdc++-v3/src/.libs -L/gcc_build_host/x86_64-pc-linux-gnu/./libgomp/.libs -L/gcc_build_host/x86_64-pc-linux-gnu/./libgomp/../libstdc++-v3/src/.libs -lstdc++ -lm -o ./target-3.exe
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: warning: type 'struct .omp_data_s.7' violates one definition rule [-Wodr]
     #pragma omp parallel for reduction(+:s)
             ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: note: a different type is defined in another translation unit
     #pragma omp parallel for reduction(+:s)
             ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:22:17: note: the first difference of corresponding definitions is field 'b.0'
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:22:17: note: a field of same name but different type is defined in another translation unit
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
lto1: note: array types have different bounds
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:42:13: warning: type 'struct .omp_data_s.20' violates one definition rule [-Wodr]
     #pragma omp parallel for reduction(+:s)
             ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:42:13: note: a different type is defined in another translation unit
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:37:17: note: the first difference of corresponding definitions is field 'b.14'
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:37:17: note: a field of same name but different type is defined in another translation unit
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
lto1: note: array types have different bounds
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:60:10: warning: type 'struct .omp_data_s.38' violates one definition rule [-Wodr]
  #pragma omp parallel for reduction(+:s)
          ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:60:10: note: a different type is defined in another translation unit
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:52:17: note: the first difference of corresponding definitions is field 'b.27'
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:52:17: note: a field of same name but different type is defined in another translation unit
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
lto1: note: array types have different bounds
output is:
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: warning: type 'struct .omp_data_s.7' violates one definition rule [-Wodr]
     #pragma omp parallel for reduction(+:s)
             ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: note: a different type is defined in another translation unit
     #pragma omp parallel for reduction(+:s)
             ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:22:17: note: the first difference of corresponding definitions is field 'b.0'
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:22:17: note: a field of same name but different type is defined in another translation unit
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
lto1: note: array types have different bounds
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:42:13: warning: type 'struct .omp_data_s.20' violates one definition rule [-Wodr]
     #pragma omp parallel for reduction(+:s)
             ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:42:13: note: a different type is defined in another translation unit
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:37:17: note: the first difference of corresponding definitions is field 'b.14'
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:37:17: note: a field of same name but different type is defined in another translation unit
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
lto1: note: array types have different bounds
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:60:10: warning: type 'struct .omp_data_s.38' violates one definition rule [-Wodr]
  #pragma omp parallel for reduction(+:s)
          ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:60:10: note: a different type is defined in another translation unit
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:52:17: note: the first difference of corresponding definitions is field 'b.27'
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:52:17: note: a field of same name but different type is defined in another translation unit
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
lto1: note: array types have different bounds

FAIL: libgomp.c++/target-3.C (test for excess errors)
Excess errors:
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: warning: type 'struct .omp_data_s.7' violates one definition rule [-Wodr]
     #pragma omp parallel for reduction(+:s)
             ^
     #pragma omp parallel for reduction(+:s)
             ^
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:42:13: warning: type 'struct .omp_data_s.20' violates one definition rule [-Wodr]
     #pragma omp parallel for reduction(+:s)
             ^
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
/gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:60:10: warning: type 'struct .omp_data_s.38' violates one definition rule [-Wodr]
  #pragma omp parallel for reduction(+:s)
          ^
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^
   double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
                 ^

  -- Ilya

Re: Silence merge warnings on artiical types

[Jan Hubicka]

> On Mon, Mar 30, 2015 at 19:06:39 +0200, Jan Hubicka wrote:
> > > On Mon, Mar 30, 2015 at 05:02:57 +0200, Jan Hubicka wrote:
> > > > 	* lto-symtab.c (lto_symtab_merge_decls_2): Silence warnings on
> > > > 	artificial decls.
> > > 
> > > Shouldn't this patch fix libgomp.c++/target-3.C in an offloading-enabled
> > > configuration?  It still fails...
> > > 
> > > libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: warning: type 'struct .omp_data_s.7' violates one definition rule [-Wodr]
> > >      #pragma omp parallel for reduction(+:s)
> > >              ^
> > >      #pragma omp parallel for reduction(+:s)
> > >              ^
> > >    double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
> > >                  ^
> > >    double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
> > >                  ^
> > 
> > No, that is an different warning.  Why those types are different? Is this a full warning output?
> 
> Here is full output:
> 
> spawn -ignore SIGHUP /gcc_build_host/gcc/xgcc -B/gcc_build_host/gcc/ /gcc/libgomp/testsuite/libgomp.c++/target-3.C -B/gcc_build_host/x86_64-pc-linux-gnu/./libgomp/ -B/gcc_build_host/x86_64-pc-linux-gnu/./libgomp/.libs -I/gcc_build_host/x86_64-pc-linux-gnu/./libgomp -I/gcc/libgomp/testsuite/../../include -I/gcc/libgomp/testsuite/.. -fmessage-length=0 -fno-diagnostics-show-caret -fdiagnostics-color=never -B/gcc_install/usr/local//libexec/gcc/x86_64-pc-linux-gnu/5.0.0 -B/gcc_install/usr/local//gcc -B/gcc_install/usr/local//bin -fopenmp -nostdinc++ -I/gcc_build_host/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu -I/gcc_build_host/x86_64-pc-linux-gnu/libstdc++-v3/include -I/gcc/libstdc++-v3/libsupc++ -I/gcc/libstdc++-v3/include/backward -I/gcc/libstdc++-v3/testsuite/util -B/gcc_build_host/x86_64-pc-linux-gnu/./libgomp/../libstdc++-v3/src/.libs -L/gcc_build_host/x86_64-pc-linux-gnu/./libgomp/.libs -L/gcc_build_host/x86_64-pc-linux-gnu/./libgomp/../libstdc++-v3/src/.libs -lstdc++ -lm -o ./target-3.exe
> /gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: warning: type 'struct .omp_data_s.7' violates one definition rule [-Wodr]
>      #pragma omp parallel for reduction(+:s)
>              ^
> /gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: note: a different type is defined in another translation unit
>      #pragma omp parallel for reduction(+:s)
>              ^
> /gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:22:17: note: the first difference of corresponding definitions is field 'b.0'
>    double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
>                  ^
> /gcc/libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:22:17: note: a field of same name but different type is defined in another translation unit
>    double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
>                  ^
> lto1: note: array types have different bounds

I see, .omp_data_s is artificial type with variable array length.  I suppose we
do not want to consider this one an ODR type like this?

Honza

Index: tree.c
===================================================================
--- tree.c      (revision 221768)
+++ tree.c      (working copy)
@@ -5139,6 +5140,7 @@
       && decl == TYPE_NAME (TREE_TYPE (decl))
       && !is_lang_specific (TREE_TYPE (decl))
       && AGGREGATE_TYPE_P (TREE_TYPE (decl))
+      && !DECL_ARTIFICIAL (decl)
       && !variably_modified_type_p (TREE_TYPE (decl), NULL_TREE)
       && !type_in_anonymous_namespace_p (TREE_TYPE (decl)))
     return !DECL_ASSEMBLER_NAME_SET_P (decl);

Re: Silence merge warnings on artiical types

[Jakub Jelinek]

On Mon, Mar 30, 2015 at 07:06:39PM +0200, Jan Hubicka wrote:
> > On Mon, Mar 30, 2015 at 05:02:57 +0200, Jan Hubicka wrote:
> > > 	* lto-symtab.c (lto_symtab_merge_decls_2): Silence warnings on
> > > 	artificial decls.
> > 
> > Shouldn't this patch fix libgomp.c++/target-3.C in an offloading-enabled
> > configuration?  It still fails...
> > 
> > libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: warning: type 'struct .omp_data_s.7' violates one definition rule [-Wodr]
> >      #pragma omp parallel for reduction(+:s)
> >              ^
> >      #pragma omp parallel for reduction(+:s)
> >              ^
> >    double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
> >                  ^
> >    double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
> >                  ^
> 
> No, that is an different warning.  Why those types are different? Is this a full warning output?

.omp_data_s.7 is a name of a really unnamed struct (DECL_NAMELESS) and also
DECL_ARTIFICIAL, contains compiler generated fields and should never be
checked for ODR violations.

I wonder if you have any DECL_ARTIFICIAL structs you want to warn on, if
yes, then at least ignore DECL_NAMELESS ones.

	Jakub

Re: Silence merge warnings on artiical types

[Jan Hubicka]

> On Mon, Mar 30, 2015 at 07:06:39PM +0200, Jan Hubicka wrote:
> > > On Mon, Mar 30, 2015 at 05:02:57 +0200, Jan Hubicka wrote:
> > > > 	* lto-symtab.c (lto_symtab_merge_decls_2): Silence warnings on
> > > > 	artificial decls.
> > > 
> > > Shouldn't this patch fix libgomp.c++/target-3.C in an offloading-enabled
> > > configuration?  It still fails...
> > > 
> > > libgomp/testsuite/libgomp.c++/../libgomp.c/target-2.c:27:13: warning: type 'struct .omp_data_s.7' violates one definition rule [-Wodr]
> > >      #pragma omp parallel for reduction(+:s)
> > >              ^
> > >      #pragma omp parallel for reduction(+:s)
> > >              ^
> > >    double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
> > >                  ^
> > >    double b[3 * x], c[3 * x], d[3 * x], e[3 * x];
> > >                  ^
> > 
> > No, that is an different warning.  Why those types are different? Is this a full warning output?
> 
> .omp_data_s.7 is a name of a really unnamed struct (DECL_NAMELESS) and also
> DECL_ARTIFICIAL, contains compiler generated fields and should never be
> checked for ODR violations.
> 
> I wonder if you have any DECL_ARTIFICIAL structs you want to warn on, if
> yes, then at least ignore DECL_NAMELESS ones.

Jason probably knows better, but I think only real C++ types comply the One Defintion
Type and should be merged. Anything we create artifically in compiler is probably
not covered by this.

Honza
> 
> 	Jakub

Re: Silence merge warnings on artiical types

[Jason Merrill]

On 03/30/2015 01:23 PM, Jan Hubicka wrote:
> Jason probably knows better, but I think only real C++ types comply the One Defintion
> Type and should be merged. Anything we create artifically in compiler is probably
> not covered by this.

Agreed, compiler internals are outside the scope of the language.  :)

Jason

Re: Silence merge warnings on artificial types

[Jan Hubicka]

> On 03/30/2015 01:23 PM, Jan Hubicka wrote:
> >Jason probably knows better, but I think only real C++ types comply the One Defintion
> >Type and should be merged. Anything we create artifically in compiler is probably
> >not covered by this.
> 
> Agreed, compiler internals are outside the scope of the language.  :)

:)

Hi,
this patch adds the ARTIFICIAL flag check to avoid ODR merging to these.
I oriignally tested DECL_ARTIFICIAL (decl) (that is TYPE_NAME) that randomly
dropped type names on some classes but not all.

Jason, please do you know what is meaning of DECL_ARTIFICIAL on class type
names? Perhaps we can drop them to 0 in free lang data?

With this bug I triggered wrong devirtualization because we no longer insert
non-odr types into a type inheritance graph.  This is fixed by the lto_read_decls
change and finally I triggered an ICE in ipa-devirt that due to the bug
output a warning late and ICEd on streamer cache being NULL.  I guess it is
better to guard it even though all wanrings should be output early.

Bootsrapped/regtested x86_64-linux, will commit it after chromium rebuild.

Honza

	* tree.c (need_assembler_name_p): Artificial types have no ODR
	names.
	* ipa-devirt.c (warn_odr): Do not try to apply ODR cache when
	no caching is done.

	* lto.c (lto_read_decls): Move code registering odr types out
	of TYPE_CANONICAL conditional and also register polymorphic types.
Index: tree.c
===================================================================
--- tree.c	(revision 221777)
@@ -5139,6 +5145,7 @@ need_assembler_name_p (tree decl)
       && decl == TYPE_NAME (TREE_TYPE (decl))
       && !is_lang_specific (TREE_TYPE (decl))
       && AGGREGATE_TYPE_P (TREE_TYPE (decl))
+      && !TYPE_ARTIFICIAL (TREE_TYPE (decl))
       && !variably_modified_type_p (TREE_TYPE (decl), NULL_TREE)
       && !type_in_anonymous_namespace_p (TREE_TYPE (decl)))
     return !DECL_ASSEMBLER_NAME_SET_P (decl);
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 221777)
+++ lto/lto.c	(working copy)
@@ -1944,13 +1944,24 @@ lto_read_decls (struct lto_file_decl_dat
 		  lto_fixup_prevailing_type (t);
 		}
 	      /* Compute the canonical type of all types.
-		 ???  Should be able to assert that !TYPE_CANONICAL.  */
+
+		 Inside a strongly connected component
+		 gimple_register_canonical_type may recurse and insert
+		 main variant ahead of time.  Thus the need to check
+		 TYPE_CANONICAL. */
 	      if (TYPE_P (t) && !TYPE_CANONICAL (t))
-		{
-		  gimple_register_canonical_type (t);
-		  if (odr_type_p (t))
-		    register_odr_type (t);
-		}
+		gimple_register_canonical_type (t);
+
+	      /* Reigster types to ODR hash.  If we compile unit w/o
+		 -fno-lto-odr-type-merging, also insert types with virtual
+		 tables to keep type inheritance graph complete on
+		 polymorphic types.  */
+	      if (TYPE_P (t)
+		  && (odr_type_p (t)
+		      || (TYPE_MAIN_VARIANT (t) == t
+			  && TREE_CODE (t) == RECORD_TYPE
+			  && TYPE_BINFO (t) && BINFO_VTABLE (TYPE_BINFO (t)))))
+		register_odr_type (t);
 	      /* Link shared INTEGER_CSTs into TYPE_CACHED_VALUEs of its
 		 type which is also member of this SCC.  */
 	      if (TREE_CODE (t) == INTEGER_CST
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 221777)
+++ ipa-devirt.c	(working copy)
@@ -939,7 +939,8 @@ warn_odr (tree t1, tree t2, tree st1, tr
 
   /* ODR warnings are output druing LTO streaming; we must apply location
      cache for potential warnings to be output correctly.  */
-  lto_location_cache::current_cache->apply_location_cache ();
+  if (lto_location_cache::current_cache)
+    lto_location_cache::current_cache->apply_location_cache ();
 
   if (!warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (t1)), OPT_Wodr,
 		   "type %qT violates one definition rule",

Re: Silence merge warnings on artificial types

[Jason Merrill]

On 03/31/2015 03:51 AM, Jan Hubicka wrote:
> Jason, please do you know what is meaning of DECL_ARTIFICIAL on class type
> names? Perhaps we can drop them to 0 in free lang data?

It indicates the implicit typedef that let's you say 'S' instead of 
'struct S' without writing 'typedef struct S S' yourself.

dwarf2out.c uses this information in TYPE_DECL_IS_STUB and 
is_redundant_typedef, so we can't clear it until after debug info 
generation.

Jason

Re: Silence merge warnings on artificial types

[Ilya Verbin]

On Tue, Mar 31, 2015 at 09:51:21 +0200, Jan Hubicka wrote:
> this patch adds the ARTIFICIAL flag check to avoid ODR merging to these.
> I oriignally tested DECL_ARTIFICIAL (decl) (that is TYPE_NAME) that randomly
> dropped type names on some classes but not all.
> 
> Jason, please do you know what is meaning of DECL_ARTIFICIAL on class type
> names? Perhaps we can drop them to 0 in free lang data?
> 
> With this bug I triggered wrong devirtualization because we no longer insert
> non-odr types into a type inheritance graph.  This is fixed by the lto_read_decls
> change and finally I triggered an ICE in ipa-devirt that due to the bug
> output a warning late and ICEd on streamer cache being NULL.  I guess it is
> better to guard it even though all wanrings should be output early.
> 
> Bootsrapped/regtested x86_64-linux, will commit it after chromium rebuild.
> 
> Honza
> 
> 	* tree.c (need_assembler_name_p): Artificial types have no ODR
> 	names.
> 	* ipa-devirt.c (warn_odr): Do not try to apply ODR cache when
> 	no caching is done.
> 
> 	* lto.c (lto_read_decls): Move code registering odr types out
> 	of TYPE_CANONICAL conditional and also register polymorphic types.
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 221777)
> @@ -5139,6 +5145,7 @@ need_assembler_name_p (tree decl)
>        && decl == TYPE_NAME (TREE_TYPE (decl))
>        && !is_lang_specific (TREE_TYPE (decl))
>        && AGGREGATE_TYPE_P (TREE_TYPE (decl))
> +      && !TYPE_ARTIFICIAL (TREE_TYPE (decl))
>        && !variably_modified_type_p (TREE_TYPE (decl), NULL_TREE)
>        && !type_in_anonymous_namespace_p (TREE_TYPE (decl)))
>      return !DECL_ASSEMBLER_NAME_SET_P (decl);

Hmm, libgomp.c++/target-3.C still fails.
Here is what I see in need_assembler_name_p:

 <type_decl 0x2b5a8751aed8 .omp_data_s.3
    type <record_type 0x2b5a87536690 .omp_data_s.3 BLK
        size <integer_cst 0x2b5a873ce318 constant 192>
        unit size <integer_cst 0x2b5a873ce2e8 constant 24>
        align 64 symtab 0 alias set -1 canonical type 0x2b5a87536690
        fields <field_decl 0x2b5a87537000 b.0 type <pointer_type 0x2b5a87536150>
            unsigned DI file test.cpp line 8 col 13
            size <integer_cst 0x2b5a873ace58 constant 64>
            unit size <integer_cst 0x2b5a873ace70 constant 8>
            align 64 offset_align 128
            offset <integer_cst 0x2b5a873ace88 constant 0>
            bit offset <integer_cst 0x2b5a873aced0 constant 0> context <record_type 0x2b5a87536690 .omp_data_s.3> chain <field_decl 0x2b5a87537098 s>> reference_to_this <reference_type 0x2b5a87536738>>
    VOID file test.cpp line 11 col 13
    align 1>
(TYPE_ARTIFICIAL = 0)

 <type_decl 0x2b5a8751a850 .omp_data_s.3
    type <record_type 0x2b5a8752cdc8 .omp_data_s.3 BLK
        size <integer_cst 0x2b5a873ce318 constant 192>
        unit size <integer_cst 0x2b5a873ce2e8 constant 24>
        align 64 symtab 0 alias set -1 canonical type 0x2b5a8752cdc8
        fields <field_decl 0x2b5a8751a980 b.0 type <pointer_type 0x2b5a8752c498>
            unsigned DI file test.cpp line 8 col 13
            size <integer_cst 0x2b5a873ace58 constant 64>
            unit size <integer_cst 0x2b5a873ace70 constant 8>
            align 64 offset_align 128
            offset <integer_cst 0x2b5a873ace88 constant 0>
            bit offset <integer_cst 0x2b5a873aced0 constant 0> context <record_type 0x2b5a8752cdc8 .omp_data_s.3> chain <field_decl 0x2b5a8751a8e8 s>>
        pointer_to_this <pointer_type 0x2b5a87536888>>
    VOID file test.cpp line 11 col 13
    align 1>
(TYPE_ARTIFICIAL = 0)

  -- Ilya

Re: Silence merge warnings on artificial types

[Jan Hubicka]

> On Tue, Mar 31, 2015 at 09:51:21 +0200, Jan Hubicka wrote:
> > this patch adds the ARTIFICIAL flag check to avoid ODR merging to these.
> > I oriignally tested DECL_ARTIFICIAL (decl) (that is TYPE_NAME) that randomly
> > dropped type names on some classes but not all.
> > 
> > Jason, please do you know what is meaning of DECL_ARTIFICIAL on class type
> > names? Perhaps we can drop them to 0 in free lang data?
> > 
> > With this bug I triggered wrong devirtualization because we no longer insert
> > non-odr types into a type inheritance graph.  This is fixed by the lto_read_decls
> > change and finally I triggered an ICE in ipa-devirt that due to the bug
> > output a warning late and ICEd on streamer cache being NULL.  I guess it is
> > better to guard it even though all wanrings should be output early.
> > 
> > Bootsrapped/regtested x86_64-linux, will commit it after chromium rebuild.
> > 
> > Honza
> > 
> > 	* tree.c (need_assembler_name_p): Artificial types have no ODR
> > 	names.
> > 	* ipa-devirt.c (warn_odr): Do not try to apply ODR cache when
> > 	no caching is done.
> > 
> > 	* lto.c (lto_read_decls): Move code registering odr types out
> > 	of TYPE_CANONICAL conditional and also register polymorphic types.
> > Index: tree.c
> > ===================================================================
> > --- tree.c	(revision 221777)
> > @@ -5139,6 +5145,7 @@ need_assembler_name_p (tree decl)
> >        && decl == TYPE_NAME (TREE_TYPE (decl))
> >        && !is_lang_specific (TREE_TYPE (decl))
> >        && AGGREGATE_TYPE_P (TREE_TYPE (decl))
> > +      && !TYPE_ARTIFICIAL (TREE_TYPE (decl))
> >        && !variably_modified_type_p (TREE_TYPE (decl), NULL_TREE)
> >        && !type_in_anonymous_namespace_p (TREE_TYPE (decl)))
> >      return !DECL_ASSEMBLER_NAME_SET_P (decl);
> 
> Hmm, libgomp.c++/target-3.C still fails.
> Here is what I see in need_assembler_name_p:
> 
>  <type_decl 0x2b5a8751aed8 .omp_data_s.3
>     type <record_type 0x2b5a87536690 .omp_data_s.3 BLK
>         size <integer_cst 0x2b5a873ce318 constant 192>
>         unit size <integer_cst 0x2b5a873ce2e8 constant 24>
>         align 64 symtab 0 alias set -1 canonical type 0x2b5a87536690
>         fields <field_decl 0x2b5a87537000 b.0 type <pointer_type 0x2b5a87536150>
>             unsigned DI file test.cpp line 8 col 13
>             size <integer_cst 0x2b5a873ace58 constant 64>
>             unit size <integer_cst 0x2b5a873ace70 constant 8>
>             align 64 offset_align 128
>             offset <integer_cst 0x2b5a873ace88 constant 0>
>             bit offset <integer_cst 0x2b5a873aced0 constant 0> context <record_type 0x2b5a87536690 .omp_data_s.3> chain <field_decl 0x2b5a87537098 s>> reference_to_this <reference_type 0x2b5a87536738>>
>     VOID file test.cpp line 11 col 13
>     align 1>
> (TYPE_ARTIFICIAL = 0)
> 
>  <type_decl 0x2b5a8751a850 .omp_data_s.3
>     type <record_type 0x2b5a8752cdc8 .omp_data_s.3 BLK
>         size <integer_cst 0x2b5a873ce318 constant 192>
>         unit size <integer_cst 0x2b5a873ce2e8 constant 24>
>         align 64 symtab 0 alias set -1 canonical type 0x2b5a8752cdc8
>         fields <field_decl 0x2b5a8751a980 b.0 type <pointer_type 0x2b5a8752c498>
>             unsigned DI file test.cpp line 8 col 13
>             size <integer_cst 0x2b5a873ace58 constant 64>
>             unit size <integer_cst 0x2b5a873ace70 constant 8>
>             align 64 offset_align 128
>             offset <integer_cst 0x2b5a873ace88 constant 0>
>             bit offset <integer_cst 0x2b5a873aced0 constant 0> context <record_type 0x2b5a8752cdc8 .omp_data_s.3> chain <field_decl 0x2b5a8751a8e8 s>>
>         pointer_to_this <pointer_type 0x2b5a87536888>>
>     VOID file test.cpp line 11 col 13
>     align 1>
> (TYPE_ARTIFICIAL = 0)

I see, the DECL_ARTIFICIAL is set on TYPE_DECL but it is not set on type
itself.  Jakub, this looks like an artificial type, so perhaps we want to set
it in omp-low.c? 
Otherwise I can add extra test for DECL_NAMELESS, but marking type as artifical
looks like correct fix to me.

Honza
> 
>   -- Ilya

Re: Silence merge warnings on artificial types

[Jakub Jelinek]

On Thu, Apr 02, 2015 at 09:23:03PM +0300, Ilya Verbin wrote:
> Hmm, libgomp.c++/target-3.C still fails.
> Here is what I see in need_assembler_name_p:

Guess we should make the .omp_data_s.* types TYPE_ARTIFICIAL too.
Will take care of that tomorrow.

	Jakub

Re: Silence merge warnings on artificial types

[Jakub Jelinek]

On Thu, Apr 02, 2015 at 09:05:53PM +0200, Jakub Jelinek wrote:
> On Thu, Apr 02, 2015 at 09:23:03PM +0300, Ilya Verbin wrote:
> > Hmm, libgomp.c++/target-3.C still fails.
> > Here is what I see in need_assembler_name_p:
> 
> Guess we should make the .omp_data_s.* types TYPE_ARTIFICIAL too.
> Will take care of that tomorrow.

Tested on x86_64-linux, committed to trunk.

2015-04-03  Jakub Jelinek  <jakub@redhat.com>

	* omp-low.c (scan_omp_parallel, scan_omp_task, scan_omp_target): Set
	TYPE_ARTIFICIAL on the .omp_data* types.

--- gcc/omp-low.c.jj	2015-03-23 08:47:51.000000000 +0100
+++ gcc/omp-low.c	2015-04-02 22:06:30.547850197 +0200
@@ -2351,6 +2351,7 @@ scan_omp_parallel (gimple_stmt_iterator
   DECL_ARTIFICIAL (name) = 1;
   DECL_NAMELESS (name) = 1;
   TYPE_NAME (ctx->record_type) = name;
+  TYPE_ARTIFICIAL (ctx->record_type) = 1;
   create_omp_child_function (ctx, false);
   gimple_omp_parallel_set_child_fn (stmt, ctx->cb.dst_fn);
 
@@ -2391,6 +2392,7 @@ scan_omp_task (gimple_stmt_iterator *gsi
   DECL_ARTIFICIAL (name) = 1;
   DECL_NAMELESS (name) = 1;
   TYPE_NAME (ctx->record_type) = name;
+  TYPE_ARTIFICIAL (ctx->record_type) = 1;
   create_omp_child_function (ctx, false);
   gimple_omp_task_set_child_fn (stmt, ctx->cb.dst_fn);
 
@@ -2404,6 +2406,7 @@ scan_omp_task (gimple_stmt_iterator *gsi
       DECL_ARTIFICIAL (name) = 1;
       DECL_NAMELESS (name) = 1;
       TYPE_NAME (ctx->srecord_type) = name;
+      TYPE_ARTIFICIAL (ctx->srecord_type) = 1;
       create_omp_child_function (ctx, true);
     }
 
@@ -2671,6 +2674,7 @@ scan_omp_target (gomp_target *stmt, omp_
   DECL_ARTIFICIAL (name) = 1;
   DECL_NAMELESS (name) = 1;
   TYPE_NAME (ctx->record_type) = name;
+  TYPE_ARTIFICIAL (ctx->record_type) = 1;
   if (offloaded)
     {
       if (is_gimple_omp_oacc (stmt))


	Jakub