* Type comparing TLC
@ 2015-02-18 16:17 Jan Hubicka
2015-02-20 13:16 ` Thomas Schwinge
0 siblings, 1 reply; 6+ messages in thread
From: Jan Hubicka @ 2015-02-18 16:17 UTC (permalink / raw)
To: gcc-patches
Hi,
looking across the ODR violation messages in libreoffice and Chromium I found
some false positives and some confused messages. This patch fixes them. In partiuclar
- I introduced nasty vtable corruption when breaking out my type merging patches,
so we ended up creating separate entries for each copy of type without BINFO :(
- C++ now allows to use enum that has no fields defined. Those needs to match enums
with fields from other unit
- class and vtable layout diffing got confused by presence of extra vptr pointers
and bases. Fixed thus.
Bootstrapped/regtested x86_64-linux, comitted.
Honza
* ipa-devirt.c (odr_subtypes_equivalent_p): Fix formating.
(compare_virtual_tables): Be smarter about skipping typeinfos;
do sane output on virtual table table mismatch.
(warn_odr): Be ready for forward declarations of enums;
output sane info on base mismatch and virtual table mismatch.
(add_type_duplicate): Fix code choosing prevailing type; do not ICE
when only one type is polymorphic.
(get_odr_type): Fix hashtable corruption.
(dump_odr_type): Dump mangled names.
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c (revision 220741)
+++ ipa-devirt.c (working copy)
@@ -551,7 +551,8 @@ set_type_binfo (tree type, tree binfo)
/* Compare T2 and T2 based on name or structure. */
static bool
-odr_subtypes_equivalent_p (tree t1, tree t2, hash_set<type_pair,pair_traits> *visited)
+odr_subtypes_equivalent_p (tree t1, tree t2,
+ hash_set<type_pair,pair_traits> *visited)
{
bool an1, an2;
@@ -618,7 +619,8 @@ compare_virtual_tables (varpool_node *pr
prevailing = vtable;
vtable = tmp;
}
- if (warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (vtable->decl))),
+ if (warning_at (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (vtable->decl))),
OPT_Wodr,
"virtual table of type %qD violates one definition rule",
DECL_CONTEXT (vtable->decl)))
@@ -633,39 +635,118 @@ compare_virtual_tables (varpool_node *pr
{
struct ipa_ref *ref1, *ref2;
bool end1, end2;
+
end1 = !prevailing->iterate_reference (n1, ref1);
end2 = !vtable->iterate_reference (n2, ref2);
- if (end1 && end2)
- return;
- if (!end1 && !end2
- && DECL_ASSEMBLER_NAME (ref1->referred->decl)
- != DECL_ASSEMBLER_NAME (ref2->referred->decl)
- && !n2
- && !DECL_VIRTUAL_P (ref2->referred->decl)
- && DECL_VIRTUAL_P (ref1->referred->decl))
+
+ /* !DECL_VIRTUAL_P means RTTI entry;
+ We warn when RTTI is lost because non-RTTI previals; we silently
+ accept the other case. */
+ while (!end2
+ && (end1
+ || (DECL_ASSEMBLER_NAME (ref1->referred->decl)
+ != DECL_ASSEMBLER_NAME (ref2->referred->decl)
+ && DECL_VIRTUAL_P (ref1->referred->decl)))
+ && !DECL_VIRTUAL_P (ref2->referred->decl))
{
- if (warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (vtable->decl))), 0,
+ if (warning_at (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (vtable->decl))), 0,
"virtual table of type %qD contains RTTI information",
DECL_CONTEXT (vtable->decl)))
{
- inform (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (prevailing->decl))),
- "but is prevailed by one without from other translation unit");
- inform (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (prevailing->decl))),
+ inform (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (prevailing->decl))),
+ "but is prevailed by one without from other translation "
+ "unit");
+ inform (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (prevailing->decl))),
"RTTI will not work on this type");
}
n2++;
end2 = !vtable->iterate_reference (n2, ref2);
}
- if (!end1 && !end2
- && DECL_ASSEMBLER_NAME (ref1->referred->decl)
- != DECL_ASSEMBLER_NAME (ref2->referred->decl)
- && !n1
- && !DECL_VIRTUAL_P (ref1->referred->decl)
- && DECL_VIRTUAL_P (ref2->referred->decl))
+ while (!end1
+ && (end2
+ || (DECL_ASSEMBLER_NAME (ref2->referred->decl)
+ != DECL_ASSEMBLER_NAME (ref1->referred->decl)
+ && DECL_VIRTUAL_P (ref2->referred->decl)))
+ && !DECL_VIRTUAL_P (ref1->referred->decl))
{
n1++;
end1 = !vtable->iterate_reference (n1, ref1);
}
+
+ /* Finished? */
+ if (end1 && end2)
+ {
+ /* Extra paranoia; compare the sizes. We do not have information
+ about virtual inheritance offsets, so just be sure that these
+ match.
+ Do this as very last check so the not very informative error
+ is not output too often. */
+ if (DECL_SIZE (prevailing->decl) != DECL_SIZE (vtable->decl))
+ {
+ if (warning_at (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (vtable->decl))), 0,
+ "virtual table of type %qD violates "
+ "one definition rule ",
+ DECL_CONTEXT (vtable->decl)))
+ {
+ inform (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (prevailing->decl))),
+ "the conflicting type defined in another translation "
+ "unit has virtual table of different size");
+ }
+ }
+ return;
+ }
+
+ if (!end1 && !end2)
+ {
+ if (DECL_ASSEMBLER_NAME (ref1->referred->decl)
+ == DECL_ASSEMBLER_NAME (ref2->referred->decl))
+ continue;
+
+ /* If the loops above stopped on non-virtual pointer, we have
+ mismatch in RTTI information mangling. */
+ if (!DECL_VIRTUAL_P (ref1->referred->decl)
+ && !DECL_VIRTUAL_P (ref2->referred->decl))
+ {
+ if (warning_at (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (vtable->decl))), 0,
+ "virtual table of type %qD violates "
+ "one definition rule ",
+ DECL_CONTEXT (vtable->decl)))
+ {
+ inform (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (prevailing->decl))),
+ "the conflicting type defined in another translation "
+ "unit virtual table with different RTTI information");
+ }
+ return;
+ }
+ /* At this point both REF1 and REF2 points either to virtual table
+ or virtual method. If one points to virtual table and other to
+ method we can complain the same way as if one table was shorter
+ than other pointing out the extra method. */
+ gcc_assert (DECL_VIRTUAL_P (ref1->referred->decl)
+ && (TREE_CODE (ref1->referred->decl) == FUNCTION_DECL
+ || TREE_CODE (ref1->referred->decl) == VAR_DECL));
+ gcc_assert (DECL_VIRTUAL_P (ref2->referred->decl)
+ && (TREE_CODE (ref2->referred->decl) == FUNCTION_DECL
+ || TREE_CODE (ref2->referred->decl) == VAR_DECL));
+ if (TREE_CODE (ref1->referred->decl)
+ != TREE_CODE (ref2->referred->decl))
+ {
+ if (TREE_CODE (ref1->referred->decl) == VAR_DECL)
+ end1 = true;
+ else if (TREE_CODE (ref2->referred->decl) == VAR_DECL)
+ end2 = true;
+ }
+ }
+
+ /* Complain about size mismatch. Either we have too many virutal
+ functions or too many virtual table pointers. */
if (end1 || end2)
{
if (end1)
@@ -681,37 +762,56 @@ compare_virtual_tables (varpool_node *pr
"one definition rule",
DECL_CONTEXT (vtable->decl)))
{
- inform (DECL_SOURCE_LOCATION
- (TYPE_NAME (DECL_CONTEXT (prevailing->decl))),
- "the conflicting type defined in another translation "
- "unit");
- inform (DECL_SOURCE_LOCATION
- (TYPE_NAME (DECL_CONTEXT (ref1->referring->decl))),
- "contains additional virtual method %qD",
- ref1->referred->decl);
+ if (TREE_CODE (ref1->referring->decl) == FUNCTION_DECL)
+ {
+ inform (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (prevailing->decl))),
+ "the conflicting type defined in another translation "
+ "unit");
+ inform (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (ref1->referring->decl))),
+ "contains additional virtual method %qD",
+ ref1->referred->decl);
+ }
+ else
+ {
+ inform (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (prevailing->decl))),
+ "the conflicting type defined in another translation "
+ "unit has virtual table table with more entries");
+ }
}
return;
}
- if (DECL_ASSEMBLER_NAME (ref1->referred->decl)
- != DECL_ASSEMBLER_NAME (ref2->referred->decl))
+
+ /* And in the last case we have either mistmatch in between two virtual
+ methods or two virtual table pointers. */
+ if (warning_at (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (vtable->decl))), 0,
+ "virtual table of type %qD violates "
+ "one definition rule ",
+ DECL_CONTEXT (vtable->decl)))
{
- if (warning_at (DECL_SOURCE_LOCATION
- (TYPE_NAME (DECL_CONTEXT (vtable->decl))), 0,
- "virtual table of type %qD violates "
- "one definition rule ",
- DECL_CONTEXT (vtable->decl)))
+ if (TREE_CODE (ref1->referred->decl) == FUNCTION_DECL)
{
inform (DECL_SOURCE_LOCATION
(TYPE_NAME (DECL_CONTEXT (prevailing->decl))),
"the conflicting type defined in another translation "
"unit");
+ gcc_assert (TREE_CODE (ref2->referred->decl)
+ == FUNCTION_DECL);
inform (DECL_SOURCE_LOCATION (ref1->referred->decl),
"virtual method %qD", ref1->referred->decl);
inform (DECL_SOURCE_LOCATION (ref2->referred->decl),
"ought to match virtual method %qD but does not",
ref2->referred->decl);
- return;
}
+ else
+ inform (DECL_SOURCE_LOCATION
+ (TYPE_NAME (DECL_CONTEXT (prevailing->decl))),
+ "the conflicting type defined in another translation "
+ "unit has virtual table table with different contents");
+ return;
}
}
}
@@ -727,6 +827,8 @@ warn_odr (tree t1, tree t2, tree st1, tr
bool warn, bool *warned, const char *reason)
{
tree decl2 = TYPE_NAME (t2);
+ if (warned)
+ *warned = false;
if (!warn)
return;
@@ -840,7 +942,8 @@ odr_types_equivalent_p (tree t1, tree t2
return false;
}
- if (TREE_CODE (t1) == ENUMERAL_TYPE)
+ if (TREE_CODE (t1) == ENUMERAL_TYPE
+ && TYPE_VALUES (t1) && TYPE_VALUES (t2))
{
tree v1, v2;
for (v1 = TYPE_VALUES (t1), v2 = TYPE_VALUES (t2);
@@ -1056,8 +1159,20 @@ odr_types_equivalent_p (tree t1, tree t2
f2 = TREE_CHAIN (f2);
if (!f1 || !f2)
break;
+ if (DECL_VIRTUAL_P (f1) != DECL_VIRTUAL_P (f2))
+ {
+ warn_odr (t1, t2, NULL, NULL, warn, warned,
+ G_("a type with different virtual table pointers"
+ " is defined in another translation unit"));
+ return false;
+ }
if (DECL_ARTIFICIAL (f1) != DECL_ARTIFICIAL (f2))
- break;
+ {
+ warn_odr (t1, t2, NULL, NULL, warn, warned,
+ G_("a type with different bases is defined "
+ "in another translation unit"));
+ return false;
+ }
if (DECL_NAME (f1) != DECL_NAME (f2)
&& !DECL_ARTIFICIAL (f1))
{
@@ -1066,10 +1181,11 @@ odr_types_equivalent_p (tree t1, tree t2
"in another translation unit"));
return false;
}
- if (!odr_subtypes_equivalent_p (TREE_TYPE (f1), TREE_TYPE (f2), visited))
+ if (!odr_subtypes_equivalent_p (TREE_TYPE (f1),
+ TREE_TYPE (f2), visited))
{
- /* Do not warn about artificial fields and just go into generic
- field mismatch warning. */
+ /* Do not warn about artificial fields and just go into
+ generic field mismatch warning. */
if (DECL_ARTIFICIAL (f1))
break;
@@ -1082,11 +1198,11 @@ odr_types_equivalent_p (tree t1, tree t2
}
if (!gimple_compare_field_offset (f1, f2))
{
- /* Do not warn about artificial fields and just go into generic
- field mismatch warning. */
+ /* Do not warn about artificial fields and just go into
+ generic field mismatch warning. */
if (DECL_ARTIFICIAL (f1))
break;
- warn_odr (t1, t2, t1, t2, warn, warned,
+ warn_odr (t1, t2, f1, f2, warn, warned,
G_("fields has different layout "
"in another translation unit"));
return false;
@@ -1099,18 +1215,18 @@ odr_types_equivalent_p (tree t1, tree t2
are not the same. */
if (f1 || f2)
{
- if (f1 && DECL_ARTIFICIAL (f1))
- f1 = NULL;
- if (f2 && DECL_ARTIFICIAL (f2))
- f2 = NULL;
- if (f1 || f2)
- warn_odr (t1, t2, f1, f2, warn, warned,
- G_("a type with different number of fields "
- "is defined in another translation unit"));
- /* Ideally we should never get this generic message. */
+ if ((f1 && DECL_VIRTUAL_P (f1)) || (f2 && DECL_VIRTUAL_P (f2)))
+ warn_odr (t1, t2, NULL, NULL, warn, warned,
+ G_("a type with different virtual table pointers"
+ " is defined in another translation unit"));
+ if ((f1 && DECL_ARTIFICIAL (f1))
+ || (f2 && DECL_ARTIFICIAL (f2)))
+ warn_odr (t1, t2, NULL, NULL, warn, warned,
+ G_("a type with different bases is defined "
+ "in another translation unit"));
else
warn_odr (t1, t2, f1, f2, warn, warned,
- G_("a type with different memory representation "
+ G_("a type with different number of fields "
"is defined in another translation unit"));
return false;
@@ -1207,12 +1323,24 @@ add_type_duplicate (odr_type val, tree t
val->types_set = new hash_set<tree>;
/* Always prefer complete type to be the leader. */
- if ((!COMPLETE_TYPE_P (val->type) || !TYPE_BINFO (val->type))
- && (COMPLETE_TYPE_P (type) && TYPE_BINFO (val->type)))
+
+ if (!COMPLETE_TYPE_P (val->type) && COMPLETE_TYPE_P (type))
+ build_bases = true;
+ else if (COMPLETE_TYPE_P (val->type) && !COMPLETE_TYPE_P (type))
+ ;
+ else if (TREE_CODE (val->type) == ENUMERAL_TYPE
+ && TREE_CODE (type) == ENUMERAL_TYPE
+ && !TYPE_VALUES (val->type) && TYPE_VALUES (type))
+ build_bases = true;
+ else if (TREE_CODE (val->type) == RECORD_TYPE
+ && TREE_CODE (type) == RECORD_TYPE
+ && TYPE_BINFO (type) && !TYPE_BINFO (val->type))
+ build_bases = true;
+
+ if (build_bases)
{
tree tmp = type;
- build_bases = true;
type = val->type;
val->type = tmp;
}
@@ -1303,11 +1431,14 @@ add_type_duplicate (odr_type val, tree t
if (base_mismatch)
{
if (!warned && !val->odr_violated)
- warn_odr (type, val->type, NULL, NULL,
- !warned, &warned,
- "a type with the same name but different base "
- "type is defined in another translation unit");
- warn_types_mismatch (type1, type2);
+ {
+ warn_odr (type, val->type, NULL, NULL,
+ !warned, &warned,
+ "a type with the same name but different base "
+ "type is defined in another translation unit");
+ if (warned)
+ warn_types_mismatch (type1, type2);
+ }
break;
}
if (BINFO_OFFSET (base1) != BINFO_OFFSET (base2))
@@ -1320,6 +1451,18 @@ add_type_duplicate (odr_type val, tree t
"layout is defined in another translation unit");
break;
}
+ /* One base is polymorphic and the other not.
+ This ought to be diagnosed earlier, but do not ICE in the
+ checking bellow. */
+ if (!TYPE_BINFO (type1) != !TYPE_BINFO (type2)
+ || (TYPE_BINFO (type1)
+ && polymorphic_type_binfo_p (TYPE_BINFO (type1))
+ != polymorphic_type_binfo_p (TYPE_BINFO (type2))))
+ {
+ gcc_assert (val->odr_violated);
+ base_mismatch = true;
+ break;
+ }
}
#ifdef ENABLE_CHECKING
/* Sanity check that all bases will be build same way again. */
@@ -1468,6 +1611,7 @@ get_odr_type (tree type, bool insert)
val->anonymous_namespace = type_in_anonymous_namespace_p (type);
build_bases = COMPLETE_TYPE_P (val->type);
insert_to_odr_array = true;
+ *slot = val;
}
if (build_bases && TREE_CODE (type) == RECORD_TYPE && TYPE_BINFO (type)
@@ -1479,7 +1623,6 @@ get_odr_type (tree type, bool insert)
gcc_assert (BINFO_TYPE (TYPE_BINFO (val->type)) = type);
val->all_derivations_known = type_all_derivations_known_p (type);
- *slot = val;
for (i = 0; i < BINFO_N_BASE_BINFOS (binfo); i++)
/* For now record only polymorphic types. other are
pointless for devirtualization and we can not precisely
@@ -1557,6 +1700,10 @@ dump_odr_type (FILE *f, odr_type t, int
fprintf (f, "%*s defined at: %s:%i\n", indent * 2, "",
DECL_SOURCE_FILE (TYPE_NAME (t->type)),
DECL_SOURCE_LINE (TYPE_NAME (t->type)));
+ if (DECL_ASSEMBLER_NAME_SET_P (TYPE_NAME (t->type)))
+ fprintf (f, "%*s mangled name: %s\n", indent * 2, "",
+ IDENTIFIER_POINTER
+ (DECL_ASSEMBLER_NAME (TYPE_NAME (t->type))));
}
if (t->bases.length ())
{
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Type comparing TLC
2015-02-18 16:17 Type comparing TLC Jan Hubicka
@ 2015-02-20 13:16 ` Thomas Schwinge
2015-02-20 13:35 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Schwinge @ 2015-02-20 13:16 UTC (permalink / raw)
To: Jan Hubicka, Jakub Jelinek; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 3959 bytes --]
Hi!
On Wed, 18 Feb 2015 17:17:08 +0100, Jan Hubicka <hubicka@ucw.cz> wrote:
> looking across the ODR violation messages in libreoffice and Chromium I found
> some false positives and some confused messages. This patch fixes them. In partiuclar
> - I introduced nasty vtable corruption when breaking out my type merging patches,
> so we ended up creating separate entries for each copy of type without BINFO :(
> - C++ now allows to use enum that has no fields defined. Those needs to match enums
> with fields from other unit
> - class and vtable layout diffing got confused by presence of extra vptr pointers
> and bases. Fixed thus.
>
> Bootstrapped/regtested x86_64-linux, comitted.
> * ipa-devirt.c (odr_subtypes_equivalent_p): Fix formating.
> (compare_virtual_tables): Be smarter about skipping typeinfos;
> do sane output on virtual table table mismatch.
> (warn_odr): Be ready for forward declarations of enums;
> output sane info on base mismatch and virtual table mismatch.
> (add_type_duplicate): Fix code choosing prevailing type; do not ICE
> when only one type is polymorphic.
> (get_odr_type): Fix hashtable corruption.
> (dump_odr_type): Dump mangled names.
I find this commit, r220790, cause the following regression in an
offloading-enabled configuration:
[-PASS:-]{+FAIL:+} libgomp.c++/target-3.C (test for excess errors)
PASS: libgomp.c++/target-3.C execution test
[...]/build-gcc/gcc/xgcc -B[...]/build-gcc/gcc/ [...]/source-gcc/libgomp/testsuite/libgomp.c++/target-3.C -B[...]/build-gcc/x86_64-unknown-linux-gnu/./libgomp/ -B[...]/build-gcc/x86_64-unknown-linux-gnu/./libgomp/.libs -I[...]/build-gcc/x86_64-unknown-linux-gnu/./libgomp -I[...]/source-gcc/libgomp/testsuite/../../include -I[...]/source-gcc/libgomp/testsuite/.. -I/usr/local/cuda-5.5/targets/x86_64-linux/include -fmessage-length=0 -fno-diagnostics-show-caret -fdiagnostics-color=never -B[...]/install/offload-nvptx-none/libexec/gcc/x86_64-unknown-linux-gnu/5.0.0 -B[...]/install/offload-nvptx-none/bin -B[...]/install/offload-x86_64-intelmicemul-linux-gnu/libexec/gcc/x86_64-unknown-linux-gnu/5.0.0 -B[...]/install/offload-x86_64-intelmicemul-linux-gnu/bin -fopenmp -nostdinc++ -I[...]/build-gcc/x86_64-unknown-linux-gnu/libstdc++-v3/include/x86_64-unknown-linux-gnu -I[...]/build-gcc/x86_64-unknown-linux-gnu/libstdc++-v3/include -I[...]/source-gcc/libstdc++-v3/libsupc++ -I[...]/source-gcc/libstdc++-v3/include/backward -I[...]/source-gcc/libstdc++-v3/testsuite/util -B[...]/build-gcc/x86_64-unknown-linux-gnu/./libgomp/../libstdc++-v3/src/.libs -L[...]/build-gcc/x86_64-unknown-linux-gnu/./libgomp/.libs -L[...]/build-gcc/x86_64-unknown-linux-gnu/./libgomp/../libstdc++-v3/src/.libs -lstdc++ -lm -o ./target-3.exe
[...]/source-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)
^
[...]/source-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)
^
[...]/source-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];
^
[...]/source-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];
^
[...]/source-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)
^
[...]
Grüße,
Thomas
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Type comparing TLC
2015-02-20 13:16 ` Thomas Schwinge
@ 2015-02-20 13:35 ` Jakub Jelinek
2015-02-20 18:53 ` Jan Hubicka
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2015-02-20 13:35 UTC (permalink / raw)
To: Thomas Schwinge; +Cc: Jan Hubicka, gcc-patches
On Fri, Feb 20, 2015 at 02:15:24PM +0100, Thomas Schwinge wrote:
> > * ipa-devirt.c (odr_subtypes_equivalent_p): Fix formating.
> > (compare_virtual_tables): Be smarter about skipping typeinfos;
> > do sane output on virtual table table mismatch.
> > (warn_odr): Be ready for forward declarations of enums;
> > output sane info on base mismatch and virtual table mismatch.
> > (add_type_duplicate): Fix code choosing prevailing type; do not ICE
> > when only one type is polymorphic.
> > (get_odr_type): Fix hashtable corruption.
> > (dump_odr_type): Dump mangled names.
>
> I find this commit, r220790, cause the following regression in an
> offloading-enabled configuration:
I'd think that we shouldn't report ODR violations for types with
DECL_ARTIFICIAL (or just DECL_NAMELESS?) TYPE_DECLs.
Especially the DECL_NAMELESS ones have names just for debugging purposes.
>
> [-PASS:-]{+FAIL:+} libgomp.c++/target-3.C (test for excess errors)
> PASS: libgomp.c++/target-3.C execution test
>
> [...]/build-gcc/gcc/xgcc -B[...]/build-gcc/gcc/ [...]/source-gcc/libgomp/testsuite/libgomp.c++/target-3.C -B[...]/build-gcc/x86_64-unknown-linux-gnu/./libgomp/ -B[...]/build-gcc/x86_64-unknown-linux-gnu/./libgomp/.libs -I[...]/build-gcc/x86_64-unknown-linux-gnu/./libgomp -I[...]/source-gcc/libgomp/testsuite/../../include -I[...]/source-gcc/libgomp/testsuite/.. -I/usr/local/cuda-5.5/targets/x86_64-linux/include -fmessage-length=0 -fno-diagnostics-show-caret -fdiagnostics-color=never -B[...]/install/offload-nvptx-none/libexec/gcc/x86_64-unknown-linux-gnu/5.0.0 -B[...]/install/offload-nvptx-none/bin -B[...]/install/offload-x86_64-intelmicemul-linux-gnu/libexec/gcc/x86_64-unknown-linux-gnu/5.0.0 -B[...]/install/offload-x86_64-intelmicemul-linux-gnu/bin -fopenmp -nostdinc++ -I[...]/build-gcc/x86_64-unknown-linux-gnu/libstdc++-v3/include/x86_64-unknown-linux-gnu -I[...]/build-gcc/x86_64-unknown-linux-gnu/libstdc++-v3/include -I[...]/source-gcc/libstdc++-v3/libsupc++ -I[...]/source-gcc/libstdc++-v3/include/backward -I[...]/source-gcc/libstdc++-v3/testsuite/util -B[...]/build-gcc/x86_64-unknown-linux-gnu/./libgomp/../libstdc++-v3/src/.libs -L[...]/build-gcc/x86_64-unknown-linux-gnu/./libgomp/.libs -L[...]/build-gcc/x86_64-unknown-linux-gnu/./libgomp/../libstdc++-v3/src/.libs -lstdc++ -lm -o ./target-3.exe
> [...]/source-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)
> ^
> [...]/source-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)
> ^
> [...]/source-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];
> ^
> [...]/source-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];
> ^
> [...]/source-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)
> ^
> [...]
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Type comparing TLC
2015-02-20 13:35 ` Jakub Jelinek
@ 2015-02-20 18:53 ` Jan Hubicka
2015-03-10 12:38 ` Ilya Verbin
0 siblings, 1 reply; 6+ messages in thread
From: Jan Hubicka @ 2015-02-20 18:53 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Thomas Schwinge, Jan Hubicka, gcc-patches
> On Fri, Feb 20, 2015 at 02:15:24PM +0100, Thomas Schwinge wrote:
> > > * ipa-devirt.c (odr_subtypes_equivalent_p): Fix formating.
> > > (compare_virtual_tables): Be smarter about skipping typeinfos;
> > > do sane output on virtual table table mismatch.
> > > (warn_odr): Be ready for forward declarations of enums;
> > > output sane info on base mismatch and virtual table mismatch.
> > > (add_type_duplicate): Fix code choosing prevailing type; do not ICE
> > > when only one type is polymorphic.
> > > (get_odr_type): Fix hashtable corruption.
> > > (dump_odr_type): Dump mangled names.
> >
> > I find this commit, r220790, cause the following regression in an
> > offloading-enabled configuration:
>
> I'd think that we shouldn't report ODR violations for types with
> DECL_ARTIFICIAL (or just DECL_NAMELESS?) TYPE_DECLs.
> Especially the DECL_NAMELESS ones have names just for debugging purposes.
Yes, I was considering to do the same because of warning on types of typeinfos
on Firefox. Firefox links together -fsigned-char and -funsigned-char that is
an ODR violation but reporting this about typeinfo itself is confusing.
I will disable warning on those. Should it be DECL_ARTIFICIAL only or both flags?
I am not really familiar about what types can be DECL_NAMELESS or DECL_ARTIFICIAL.
Honza
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Type comparing TLC
2015-02-20 18:53 ` Jan Hubicka
@ 2015-03-10 12:38 ` Ilya Verbin
2015-03-10 18:12 ` Jan Hubicka
0 siblings, 1 reply; 6+ messages in thread
From: Ilya Verbin @ 2015-03-10 12:38 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Jakub Jelinek, Thomas Schwinge, gcc-patches, Kirill Yukhin
Hi!
On Fri, Feb 20, 2015 at 19:49:03 +0100, Jan Hubicka wrote:
> > On Fri, Feb 20, 2015 at 02:15:24PM +0100, Thomas Schwinge wrote:
> > > > * ipa-devirt.c (odr_subtypes_equivalent_p): Fix formating.
> > > > (compare_virtual_tables): Be smarter about skipping typeinfos;
> > > > do sane output on virtual table table mismatch.
> > > > (warn_odr): Be ready for forward declarations of enums;
> > > > output sane info on base mismatch and virtual table mismatch.
> > > > (add_type_duplicate): Fix code choosing prevailing type; do not ICE
> > > > when only one type is polymorphic.
> > > > (get_odr_type): Fix hashtable corruption.
> > > > (dump_odr_type): Dump mangled names.
> > >
> > > I find this commit, r220790, cause the following regression in an
> > > offloading-enabled configuration:
> >
> > I'd think that we shouldn't report ODR violations for types with
> > DECL_ARTIFICIAL (or just DECL_NAMELESS?) TYPE_DECLs.
> > Especially the DECL_NAMELESS ones have names just for debugging purposes.
>
> Yes, I was considering to do the same because of warning on types of typeinfos
> on Firefox. Firefox links together -fsigned-char and -funsigned-char that is
> an ODR violation but reporting this about typeinfo itself is confusing.
>
> I will disable warning on those. Should it be DECL_ARTIFICIAL only or both flags?
> I am not really familiar about what types can be DECL_NAMELESS or DECL_ARTIFICIAL.
Any plans to fix this in GCC 5?
Thanks,
-- Ilya
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Type comparing TLC
2015-03-10 12:38 ` Ilya Verbin
@ 2015-03-10 18:12 ` Jan Hubicka
0 siblings, 0 replies; 6+ messages in thread
From: Jan Hubicka @ 2015-03-10 18:12 UTC (permalink / raw)
To: Ilya Verbin
Cc: Jan Hubicka, Jakub Jelinek, Thomas Schwinge, gcc-patches, Kirill Yukhin
> Hi!
>
> On Fri, Feb 20, 2015 at 19:49:03 +0100, Jan Hubicka wrote:
> > > On Fri, Feb 20, 2015 at 02:15:24PM +0100, Thomas Schwinge wrote:
> > > > > * ipa-devirt.c (odr_subtypes_equivalent_p): Fix formating.
> > > > > (compare_virtual_tables): Be smarter about skipping typeinfos;
> > > > > do sane output on virtual table table mismatch.
> > > > > (warn_odr): Be ready for forward declarations of enums;
> > > > > output sane info on base mismatch and virtual table mismatch.
> > > > > (add_type_duplicate): Fix code choosing prevailing type; do not ICE
> > > > > when only one type is polymorphic.
> > > > > (get_odr_type): Fix hashtable corruption.
> > > > > (dump_odr_type): Dump mangled names.
> > > >
> > > > I find this commit, r220790, cause the following regression in an
> > > > offloading-enabled configuration:
> > >
> > > I'd think that we shouldn't report ODR violations for types with
> > > DECL_ARTIFICIAL (or just DECL_NAMELESS?) TYPE_DECLs.
> > > Especially the DECL_NAMELESS ones have names just for debugging purposes.
> >
> > Yes, I was considering to do the same because of warning on types of typeinfos
> > on Firefox. Firefox links together -fsigned-char and -funsigned-char that is
> > an ODR violation but reporting this about typeinfo itself is confusing.
> >
> > I will disable warning on those. Should it be DECL_ARTIFICIAL only or both flags?
> > I am not really familiar about what types can be DECL_NAMELESS or DECL_ARTIFICIAL.
>
> Any plans to fix this in GCC 5?
yes, only yesterday I got chromium LTO building up and running and it does
produce some ODR warnings that are either confusing or wrong (among many
correct ones). I plan to check them and fix these issues soon.
Honza
>
> Thanks,
> -- Ilya
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-10 18:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-18 16:17 Type comparing TLC Jan Hubicka
2015-02-20 13:16 ` Thomas Schwinge
2015-02-20 13:35 ` Jakub Jelinek
2015-02-20 18:53 ` Jan Hubicka
2015-03-10 12:38 ` Ilya Verbin
2015-03-10 18:12 ` Jan Hubicka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).