| Message ID | 20130511001601.GB29273@kam.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
> seems to bring a lot of positives and by quick inspection most of them seem > like same classes. I will try to find some time to debug this more. But > glancing over the the dumps, I see many of them just have different name > spaces. Do we even attempt to merge namespace_decl? How types from same > namespaces in different units are supposed to match? Just as example <record_type 0x7ff129ff6bd0 X86Assembler addressable needs-constructing BLK size <integer_cst 0x7ff136ee2760 type <integer_type 0x7ff1370af0a8 bitsizetype> constant 2368> unit size <integer_cst 0x7ff136ee2780 type <integer_type 0x7ff1370af000 sizetype> constant 296> align 64 symtab 0 alias set -1 canonical type 0x7ff136efc498 fields <field_decl 0x7ff129fe2850 D.752229 type <record_type 0x7ff129ff6150 GenericAssembler needs-constructing TI size <integer_cst 0x7ff13709eda0 constant 128> unit size <integer_cst 0x7ff13709edc0 constant 16> align 64 symtab 0 alias set -1 canonical type 0x7ff136ec6b28 fields <field_decl 0x7ff129fe2098 printer> context <namespace_decl 0x7ff129e45e60 JSC> pointer_to_this <pointer_type 0x7ff129c7f888> chain <type_decl 0x7ff129fd7e60 GenericAssembler>> ignored TI file /abuild/jh/mozilla-central2/mozilla-central/js/src/assembler/assembler/X86Assembler.h line 140 col 7 size <integer_cst 0x7ff137227400 constant 72> unit size <integer_cst 0x7ff136eb50a0 constant 9> align 64 offset_align 128 offset <integer_cst 0x7ff13709ed60 constant 0> bit offset <integer_cst 0x7ff13709ede0 constant 0> context <record_type 0x7ff129ff6bd0 X86Assembler> chain <field_decl 0x7ff129fe28e8 m_formatter type <record_type 0x7ff129ff6b28 X86InstructionFormatter> used private nonlocal BLK file /abuild/jh/mozilla-central2/mozilla-central/js/src/assembler/assembler/X86Assembler.h line 3207 col 7 size <integer_cst 0x7ff136e64d00 constant 2240> unit size <integer_cst 0x7ff136e64d20 constant 280> align 64 offset_align 128 offset <integer_cst 0x7ff13709edc0 16> bit offset <integer_cst 0x7ff13709ede0 0> context <record_type 0x7ff129ff6bd0 X86Assembler> chain <type_decl 0x7ff129ff75c0 X86Assembler>>> context <namespace_decl 0x7ff129e45e60 JSC> pointer_to_this <pointer_type 0x7ff129c7fa80> chain <type_decl 0x7ff129ff7508 X86Assembler>> <record_type 0x7ff12c9e7690 X86Assembler addressable needs-constructing BLK size <integer_cst 0x7ff136ee2760 type <integer_type 0x7ff1370af0a8 bitsizetype> constant 2368> unit size <integer_cst 0x7ff136ee2780 type <integer_type 0x7ff1370af000 sizetype> constant 296> align 64 symtab 0 alias set -1 canonical type 0x7ff136efc498 fields <field_decl 0x7ff12db56000 D.789661 type <record_type 0x7ff12e6b4000 GenericAssembler needs-constructing TI size <integer_cst 0x7ff13709eda0 constant 128> unit size <integer_cst 0x7ff13709edc0 constant 16> align 64 symtab 0 alias set -1 canonical type 0x7ff136ec6b28 fields <field_decl 0x7ff12c72e8e8 printer> context <namespace_decl 0x7ff1315587e8 JSC> pointer_to_this <pointer_type 0x7ff12c6cb738> chain <type_decl 0x7ff13155f228 GenericAssembler>> ignored TI file /abuild/jh/mozilla-central2/mozilla-central/js/src/assembler/assembler/X86Assembler.h line 140 col 0 size <integer_cst 0x7ff137227400 constant 72> unit size <integer_cst 0x7ff136eb50a0 constant 9> align 64 offset_align 128 offset <integer_cst 0x7ff13709ed60 constant 0> bit offset <integer_cst 0x7ff13709ede0 constant 0> context <record_type 0x7ff12c9e7690 X86Assembler> chain <field_decl 0x7ff12db56098 m_formatter type <record_type 0x7ff12c9e75e8 X86InstructionFormatter> used private nonlocal BLK file /abuild/jh/mozilla-central2/mozilla-central/js/src/assembler/assembler/X86Assembler.h line 3207 col 0 size <integer_cst 0x7ff136e64d00 constant 2240> unit size <integer_cst 0x7ff136e64d20 constant 280> align 64 offset_align 128 offset <integer_cst 0x7ff13709edc0 16> bit offset <integer_cst 0x7ff13709ede0 0> context <record_type 0x7ff12c9e7690 X86Assembler> chain <type_decl 0x7ff12c673678 X86Assembler>>> context <namespace_decl 0x7ff1315587e8 JSC> pointer_to_this <pointer_type 0x7ff12c6cbf18> chain <type_decl 0x7ff12c6732e0 X86Assembler>> Honza > > Honza > > > > Richard. > > > > >Honza > >
Jan Hubicka <hubicka@ucw.cz> wrote: >> >> The existing check should work ok with lto. If not then we should >figure out why we do not merge the main variants properly. >Hmm, adding: >Index: tree.c >=================================================================== >--- tree.c (revision 198796) >+++ tree.c (working copy) >@@ -11572,6 +11572,12 @@ get_binfo_at_offset (tree binfo, HOST_WI > > if (TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (expected_type)) > return binfo; >+ else >+ if (types_compatible_p (type, expected_type)) >+ { >+ debug_tree (TYPE_MAIN_VARIANT (type)); >+ debug_tree (TYPE_MAIN_VARIANT (expected_type)); >+ } > if (offset < 0) > return NULL_TREE; > >@@ -11605,6 +11611,12 @@ get_binfo_at_offset (tree binfo, HOST_WI > found_binfo = base_binfo; > break; > } >+ else >+ if (types_compatible_p (TREE_TYPE (base_binfo), >TREE_TYPE (fld))) >+ { >+ debug_tree (TREE_TYPE (base_binfo)); >+ debug_tree (TREE_TYPE (fld)); >+ } > if (!found_binfo) > return NULL_TREE; > binfo = found_binfo; > >seems to bring a lot of positives and by quick inspection most of them >seem >like same classes. I will try to find some time to debug this more. >But >glancing over the the dumps, I see many of them just have different >name >spaces. Do we even attempt to merge namespace_decl? How types from >same >namespaces in different units are supposed to match? > We do not merge namespace decls, which is likely the issue here. My in-progress tree merging should eventually fix this... Richard. >Honza >> >> Richard. >> >> >Honza >>
> >But > >glancing over the the dumps, I see many of them just have different > >name > >spaces. Do we even attempt to merge namespace_decl? How types from > >same > >namespaces in different units are supposed to match? > > > We do not merge namespace decls, which is likely the issue here. My in-progress tree merging should eventually fix this... Yep, I think namespaces should to be merged. They are however (always?) refereed by TYPE_CONTEXT and I do not see that to be part of type merging rules. So I do not think it prevents us from merging types from different namespace decls. On the other hand, I alwas saw a lot of duplicates of the same structure/class getting unmerged even without namespaces. I am not sure how much of those are just bugs and how much we keep for valid reasons (such as slightly different debug info attached to them because of different #include order or so) I uploaded the list of false negatives when compiling Mozilla's JS interpretter to http://atrey.karlin.mff.cuni.cz/~hubicka/binfo.txt I suppose any bigger C++ project will give such a list and it looks like good list of cases to analyze. Can we resonably expect those to be all merged? Perhaps C++ one decl rule allows us to simply match names+namespaces (after merging) here? Honza > > Richard. > > >Honza > >> > >> Richard. > >> > >> >Honza > >> >
On Sat, May 11, 2013 at 1:39 PM, Jan Hubicka <hubicka@ucw.cz> wrote: >> >But >> >glancing over the the dumps, I see many of them just have different >> >name >> >spaces. Do we even attempt to merge namespace_decl? How types from >> >same >> >namespaces in different units are supposed to match? >> > >> We do not merge namespace decls, which is likely the issue here. My in-progress tree merging should eventually fix this... > > Yep, I think namespaces should to be merged. They are however (always?) > refereed by TYPE_CONTEXT and I do not see that to be part of type merging > rules. So I do not think it prevents us from merging types from different > namespace decls. Note that only TYPE_DECLs are siblings of NAMESPACE_DECLs and we do stream DECL_CONTEXT (and also factor in that at least for types). So in the end we should only merge same types from the same namespace which means we _should_ factor in the types overall context, even if we fail to do that now. > On the other hand, I alwas saw a lot of duplicates of the same structure/class > getting unmerged even without namespaces. I am not sure how much of those are > just bugs and how much we keep for valid reasons (such as slightly different > debug info attached to them because of different #include order or so) Bugs are certainly possible ;) > I uploaded the list of false negatives when compiling Mozilla's JS interpretter > to http://atrey.karlin.mff.cuni.cz/~hubicka/binfo.txt > I suppose any bigger C++ project will give such a list and it looks like good > list of cases to analyze. > > Can we resonably expect those to be all merged? Perhaps C++ one decl rule > allows us to simply match names+namespaces (after merging) here? But we cannot assume C++ rules for types. Richard. > Honza > >> >> Richard. >> >> >Honza >> >> >> >> Richard. >> >> >> >> >Honza >> >> >>
Index: tree.c =================================================================== --- tree.c (revision 198796) +++ tree.c (working copy) @@ -11572,6 +11572,12 @@ get_binfo_at_offset (tree binfo, HOST_WI if (TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (expected_type)) return binfo; + else + if (types_compatible_p (type, expected_type)) + { + debug_tree (TYPE_MAIN_VARIANT (type)); + debug_tree (TYPE_MAIN_VARIANT (expected_type)); + } if (offset < 0) return NULL_TREE; @@ -11605,6 +11611,12 @@ get_binfo_at_offset (tree binfo, HOST_WI found_binfo = base_binfo; break; } + else + if (types_compatible_p (TREE_TYPE (base_binfo), TREE_TYPE (fld))) + { + debug_tree (TREE_TYPE (base_binfo)); + debug_tree (TREE_TYPE (fld)); + } if (!found_binfo) return NULL_TREE; binfo = found_binfo;