On Wed, Aug 2, 2017 at 6:35 AM, Richard Biener <rguent...@suse.de> wrote: > On Wed, 2 Aug 2017, Jason Merrill wrote: > >> On 05/19/2017 06:42 AM, Richard Biener wrote: >> > + /* ??? In some cases the C++ FE (at least) fails to >> > + set DECL_CONTEXT properly. Simply globalize stuff >> > + in this case. For example >> > + __dso_handle created via iostream line 74 col 25. */ >> > + parent = comp_unit_die (); >> >> I've now fixed __dso_handle, so that can be removed from the comment, but it >> still makes sense to have this fall-back for the (permitted) case of null >> DECL_CONTEXT. > > Adjusted the comment. > >> > + /* ??? LANG issue - DW_TAG_module for fortran. Either look >> > + at the input language (if we have enough DECL_CONTEXT to follow) >> > + or use a bit in tree_decl_with_vis to record the distinction. */ >> >> Sure, you should be able to look at TRANSLATION_UNIT_LANGUAGE. > > Yeah, the comment says we might be able to walk DECL_CONTEXT up to > a TRANSLATION_UNIT_DECL. I've amended is_fortran similar to as I > amended is_cxx, providing an overload for a decl, factoring out common > code. So this is now if (is_fortran (decl)) ... = new_die > (DW_TAG_module,...). > >> > ! /* ??? We cannot unconditionally output die_offset if >> > ! non-zero - at least -feliminate-dwarf2-dups will >> > ! create references to those DIEs via symbols. And we >> > ! do not clear its DIE offset after outputting it >> > ! (and the label refers to the actual DIEs, not the >> > ! DWARF CU unit header which is when using label + offset >> > ! would be the correct thing to do). >> >> As in our previous discussion, I think -feliminate-dwarf2-dups can go away >> now. But this is a more general issue: die_offset has meant the offset from >> the beginning of the CU, but if with_offset is set it means an offset from >> die_symbol. Since with_offset changes the meaning of die_symbol and >> die_offset, having different code here depending on that flag makes sense. >> >> It seems likely that when -fEDD goes away, we will never again want to do >> direct symbolic references to DIEs, in which case we could drop the current >> meaning of die_symbol, and so we wouldn't need the with_offset flag. > > Yeah, I've been playing with a patch to remove -fEDD but it has conflicts > with the early LTO debug work and thus I wanted to postpone it until > after that goes in to avoid further churn. I hope that's fine, it's > sth I'll tackle soon after this patch lands on trunk.
Sure. >> > ! /* Don't output the symbol twice. For LTO we want the label >> > ! on the section beginning, not on the actual DIE. */ >> > ! && (!flag_generate_lto >> > ! || die->die_tag != DW_TAG_compile_unit)) >> >> I think this check should just be !with_offset; if that flag is set the DIE >> doesn't actually have its own symbol. > > with_offset is set only during LTRANS phase for the DIEs refering to > the early DIEs via the CU label. But the above is guarding the > early phase when we do not want to output that CU label twice. > > Can we revisit this check when -fEDD has gone away? Yes. >> > ! if (old_die >> > ! && (c = get_AT_ref (old_die, DW_AT_abstract_origin)) >> > ! /* ??? In LTO all origin DIEs still refer to the early >> > ! debug copy. Detect that. */ >> > ! && get_AT (c, DW_AT_inline)) >> ... >> > ! /* "Unwrap" the decls DIE which we put in the imported unit >> > context. >> > ! ??? If we finish dwarf2out_function_decl refactoring we can >> > ! do this in a better way from the start and only lazily emit >> > ! the early DIE references. */ >> >> It seems like in gen_subprogram_die you deliberately avoid reusing the DIE >> from dwarf2out_register_external_die (since it doesn't have DW_AT_inline), >> and >> then in add_abstract_origin_attribute you need to look through that redundant >> die. Why not reuse it? > > What we're doing here is dealing with the case of an inlined > instance which is adjusted to point back to the early debug copy > directly than to the wrapping DIE (supposed to eventually contain > the concrete instance). But we enter this block when we're emitting the concrete out-of-line instance, and the DW_AT_inline check prevents us from using the wrapping DIE for the out-of-line instance. The comment should really change "inlined instance" to "concrete instance"; inlined instances are handled in gen_inlined_subroutine_die. Jason
