From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17683 invoked by alias); 27 May 2015 12:39:53 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 17582 invoked by uid 89); 27 May 2015 12:39:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_50,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 27 May 2015 12:39:51 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t4RCdmau011003 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 27 May 2015 08:39:48 -0400 Received: from [10.10.116.26] ([10.10.116.26]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t4RCdlYr028574; Wed, 27 May 2015 08:39:47 -0400 Message-ID: <5565BB13.6040205@redhat.com> Date: Wed, 27 May 2015 12:50:00 -0000 From: Jason Merrill User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Aldy Hernandez , Richard Biener , Jan Hubicka CC: gcc-patches Subject: Re: [patch 10/10] debug-early merge: compiler proper References: <554C060F.6000609@redhat.com> <555CAD35.5040304@redhat.com> In-Reply-To: <555CAD35.5040304@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2015-05/txt/msg02415.txt.bz2 On 05/20/2015 11:50 AM, Aldy Hernandez wrote: > + determine anscestry later. */ ancestry > +static bool early_dwarf_dumping; Sorry for the late bikeshedding, but "dumping" suddently strikes me as odd, since there is no output as with other dumping in the compiler. Can we change that to "generation" or "building"? > + /* Reuse DIE even with a differing context. > + > + This happens when called through > + dwarf2out_abstract_function for formal parameter > + packs. */ > + gcc_assert (parm_die->die_parent->die_tag > + == DW_TAG_GNU_formal_parameter_pack); Does this mean we're generating a new DW_TAG_GNU_formal_parameter_pack in late debug even though we already generated one in early debug? If so, why? > - /* It is possible to have both DECL_ABSTRACT_P and DECLARATION be true if we > - started to generate the abstract instance of an inline, decided to output > - its containing class, and proceeded to emit the declaration of the inline > - from the member list for the class. If so, DECLARATION takes priority; > - we'll get back to the abstract instance when done with the class. */ > - > - /* The class-scope declaration DIE must be the primary DIE. */ > - if (origin && declaration && class_or_namespace_scope_p (context_die)) > - { > - origin = NULL; > - gcc_assert (!old_die); > - } Can't this happen anymore? > + if ((is_cu_die (old_die->die_parent) > + /* FIXME: Jason doesn't like this condition, but it fixes > + the inconsistency/ICE with the following Fortran test: > + > + module some_m > + contains > + logical function funky (FLAG) > + funky = .true. > + end function > + end module > + > + Another alternative is !is_cu_die (context_die). > + */ > + || old_die->die_parent->die_tag == DW_TAG_module I like it now. :) You can leave the rest of the comment. > + /* For non DECL_EXTERNALs, if range information is available, fill > + the DIE with it. */ > else if (!DECL_EXTERNAL (decl)) > { > HOST_WIDE_INT cfa_fb_offset; > + > struct function *fun = DECL_STRUCT_FUNCTION (decl); > > - if (!old_die || !get_AT (old_die, DW_AT_inline)) > - equate_decl_number_to_die (decl, subr_die); > + /* If we have no fun->fde, we have no range information. > + Skip over and fill in range information in the second > + dwarf pass. */ > + if (!fun->fde) > + goto no_fde_continue; How about controlling this block with !early_dwarf so you don't need to deal with missing FDE? > if (generic_decl_parm > && lang_hooks.function_parameter_pack_p (generic_decl_parm)) > - gen_formal_parameter_pack_die (generic_decl_parm, > - parm, subr_die, > - &parm); > + { > + if (early_dwarf_dumping) > + gen_formal_parameter_pack_die (generic_decl_parm, > + parm, subr_die, > + &parm); > + else if (parm) > + parm = DECL_CHAIN (parm); > + } Let's try only setting generic_decl when early_dwarf. > + /* Unless we have an existing non-declaration DIE, equate the new > + DIE. */ > + if (!old_die || is_declaration_die (old_die)) > + equate_decl_number_to_die (decl, subr_die); ... > + if (decl && (DECL_ABSTRACT_P (decl) || declaration || old_die == NULL > + /* If we make it to a specialization, we have already > + handled the declaration by virtue of early dwarf. > + If so, make a new assocation if available, so late > + dwarf can find it. */ > + || (specialization_p && early_dwarf_dumping))) > equate_decl_number_to_die (decl, var_die); Why are the conditions so different? Can we use the function condition for variables, too? > + /* Do nothing. This must have been early dumped and it > + won't even need location information since it's a > + DW_AT_inline function. */ > + for (dw_die_ref c = context_die; c; c = c->die_parent) > + if (c->die_tag == DW_TAG_inlined_subroutine > + || c->die_tag == DW_TAG_subprogram) > + { > + gcc_assert (get_AT (c, DW_AT_inline)); > + break; > + } Maybe wrap this in #ifdef ENABLE_CHECKING. > + /* Do the new DIE dance. */ > + stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt); > + BLOCK_DIE (stmt) = stmt_die; > + } > + } > + else if (BLOCK_ABSTRACT_ORIGIN (stmt)) > + { > + /* If this is an inlined instance, create a new lexical die for > + anything below to attach DW_AT_abstract_origin to. */ > + stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt); > + } > + else > + { > + if (!stmt_die) > + { > + /* This is the first time we are creating something for this > + block. */ > + stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt); > + BLOCK_DIE (stmt) = stmt_die; > + } Surely we don't need to repeat the new_die call three times; the first and last are both controlled by !stmt_die. And don't we want to set BLOCK_DIE for the inlined case as well, so that we can find the DIE again in late debug? > + /* Fill in the size of variable-length fields in late dwarf. */ > + if (TREE_ASM_WRITTEN (type) > + && !early_dwarf_dumping) > + { > + tree member; > + for (member = TYPE_FIELDS (type); member; member = DECL_CHAIN (member)) > + fill_variable_array_bounds (TREE_TYPE (member)); > + return; > + } Why is this happening in late dwarf? I'm concerned that front-end information that is necessary to do this might be lost by that point. > + /* Variable-length types may be incomplete even if > + TREE_ASM_WRITTEN. For such types, fall through to > + gen_array_type_die() and possibly fill in > + DW_AT_{upper,lower}_bound attributes. */ > + if ((TREE_CODE (type) != ARRAY_TYPE > + && TREE_CODE (type) != RECORD_TYPE > + && TREE_CODE (type) != UNION_TYPE > + && TREE_CODE (type) != QUAL_UNION_TYPE) > + || (TYPE_SIZE (type) > + && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST)) Similarly, why check for INTEGER_CST here? > + bool t = early_dwarf_dumping; > + early_dwarf_dumping = true; > + dwarf2out_decl (decl); > + early_dwarf_dumping = t; Let's use a RAII (resource acquisition is initialization) pattern for this and dwarf2out_imported_module_or_decl and dwarf2out_early_global_decl: struct set_early_dwarf { bool saved; set_early_dwarf(): saved(early_dwarf) { early_dwarf = true; } ~set_early_dwarf() { early_dwarf = saved; } }; set_early_dwarf s; dwarf2out_decl (decl); > + /* When generating LTO bytecode we can not generate new assembler > + names at this point and all important decls got theirs via > + free-lang-data. */ > + if (((!flag_generate_lto && !flag_generate_offload) > + || DECL_ASSEMBLER_NAME_SET_P (decl)) > + && DECL_ASSEMBLER_NAME (decl) != DECL_NAME (decl) Doesn't early_finish happen before free_lang_data, so we should be fine? Jason