From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa2.mentor.iphmx.com (esa2.mentor.iphmx.com [68.232.141.98]) by sourceware.org (Postfix) with ESMTPS id D66FF38515FF for ; Mon, 19 Jul 2021 16:13:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D66FF38515FF Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=mentor.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com IronPort-SDR: Pp7xUE3X86xgyG4Nny2TlggpLz56Ub16lkXISuddNTowGFhW4JFFV0EYmPuBDSpeN3ehAOqQij kS726vgy+1S/lSj7UmqFH0zGhr7pxlPZUNNvQCZkCTsykPJ4Y/OlIcuLWlrj4NP/poffeBN831 yBC+BAHc5n3WwvH+9Ph//aEEiwxhZLkAN1t9GYsIHfmodE+ZGRY7HYpGvIs5UcVedOH2h4uBim L4nY/gsvtiUJSYuRcbNhgDMABWvQykMC8Dl1I3X7IDYV15JAI1+6XH+wyEd3+u/5DvbQz8qvwK Lk1gGk37q+8tOB9VfsZp7yZv X-IronPort-AV: E=Sophos;i="5.84,252,1620720000"; d="scan'208";a="63682079" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa2.mentor.iphmx.com with ESMTP; 19 Jul 2021 08:13:45 -0800 IronPort-SDR: fzMGA1pnWLD+p7QRxr+VvcHQoIsO0rCDVkgtJWLtZ6rrdurXd8DrdH1JSsYTmtk2j4rrxXO55v eY8hZkvnCw+hJZ1MXF2ewGJ4NqbOf1Vd6oaZCZB4SFmSsBRyy6iDB3/a+gmZH38/eh5XzJ1u7C 5zhHYfPFz5NCy9SfqA7NBC/JFKoPTVwD09Z9Ey89iAOkBwNbJFh6c7DPV4a+jgmiUDz8Gu9iIX SirLy8GkqAi+Rtes2VZgiiR72wf2fStc1ZcPgULGwra2k+qcAHYLxrLoPq/SFXHhHVTt9brOK9 fa0= Subject: Re: [PATCH] [DWARF] Fix hierarchy of debug information for offload kernels. To: Richard Biener CC: Thomas Schwinge , Abid Qadeer , Jakub Jelinek , GCC Patches References: <20210701151657.935006-1-abidh@codesourcery.com> <8735sfamu7.fsf@dem-tschwing-1.ger.mentorg.com> <50d1657e-c5de-9cb9-1aaa-233f27dfe9b9@mentor.com> From: Hafiz Abid Qadeer Message-ID: <210da16b-5992-7d7f-7223-4a4f08998d49@mentor.com> Date: Mon, 19 Jul 2021 17:13:40 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-02.mgc.mentorg.com (139.181.222.2) To SVR-IES-MBX-03.mgc.mentorg.com (139.181.222.3) X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Jul 2021 16:13:48 -0000 On 19/07/2021 11:45, Richard Biener wrote: > On Fri, Jul 16, 2021 at 10:23 PM Hafiz Abid Qadeer > wrote: >> >> On 15/07/2021 13:09, Richard Biener wrote: >>> On Thu, Jul 15, 2021 at 12:35 PM Hafiz Abid Qadeer >>> wrote: >>>> >>>> On 15/07/2021 11:33, Thomas Schwinge wrote: >>>>> >>>>>> Note that the "parent" should be abstract but I don't think dwarf has a >>>>>> way to express a fully abstract parent of a concrete instance child - or >>>>>> at least how GCC expresses this causes consumers to "misinterpret" >>>>>> that. I wonder if adding a DW_AT_declaration to the late DWARF >>>>>> emitted "parent" would fix things as well here? >>>>> >>>>> (I suppose not, Abid?) >>>>> >>>> >>>> Yes, adding DW_AT_declaration does not fix the problem. >>> >>> Does emitting >>> >>> DW_TAG_compile_unit >>> DW_AT_name ("") >>> >>> DW_TAG_subprogram // notional parent function (foo) with no code range >>> DW_AT_declaration 1 >>> a: DW_TAG_subprogram // offload function foo._omp_fn.0 >>> DW_AT_declaration 1 >>> >>> DW_TAG_subprogram // offload function >>> DW_AT_abstract_origin a >>> ... >>> >>> do the trick? The following would do this, flattening function definitions >>> for the concrete copies: >>> >>> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c >>> index 82783c4968b..a9c8bc43e88 100644 >>> --- a/gcc/dwarf2out.c >>> +++ b/gcc/dwarf2out.c >>> @@ -6076,6 +6076,11 @@ maybe_create_die_with_external_ref (tree decl) >>> /* Peel types in the context stack. */ >>> while (ctx && TYPE_P (ctx)) >>> ctx = TYPE_CONTEXT (ctx); >>> + /* For functions peel the context up to namespace/TU scope. The abstract >>> + copies reveal the true nesting. */ >>> + if (TREE_CODE (decl) == FUNCTION_DECL) >>> + while (ctx && TREE_CODE (ctx) == FUNCTION_DECL) >>> + ctx = DECL_CONTEXT (ctx); >>> /* Likewise namespaces in case we do not want to emit DIEs for them. */ >>> if (debug_info_level <= DINFO_LEVEL_TERSE) >>> while (ctx && TREE_CODE (ctx) == NAMESPACE_DECL) >>> @@ -6099,8 +6104,7 @@ maybe_create_die_with_external_ref (tree decl) >>> /* Leave function local entities parent determination to when >>> we process scope vars. */ >>> ; >>> - else >>> - parent = lookup_decl_die (ctx); >>> + parent = lookup_decl_die (ctx); >>> } >>> else >>> /* In some cases the FEs fail to set DECL_CONTEXT properly. >>> >> >> Thanks. This solves the problem. Only the first hunk was required. Second hunk >> actually causes an ICE when TREE_CODE (ctx) == BLOCK. >> OK to commit the attached patch? > > I think we need to merge the TYPE_P peeling and FUNCTION_DECL peeling into > one loop since I suppose we can have a nested function in class scope. > So sth like > > diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c > index 82783c4968b..61228410b51 100644 > --- a/gcc/dwarf2out.c > +++ b/gcc/dwarf2out.c > @@ -6073,8 +6073,12 @@ maybe_create_die_with_external_ref (tree decl) > } > else > ctx = DECL_CONTEXT (decl); > - /* Peel types in the context stack. */ > - while (ctx && TYPE_P (ctx)) > + /* Peel types in the context stack. For functions peel the context up > + to namespace/TU scope. The abstract copies reveal the true nesting. */ > + while (ctx > + && (TYPE_P (ctx) > + || (TREE_CODE (decl) == FUNCTION_DECL > + && TREE_CODE (ctx) == FUNCTION_DECL))) > ctx = TYPE_CONTEXT (ctx); > /* Likewise namespaces in case we do not want to emit DIEs for them. */ > if (debug_info_level <= DINFO_LEVEL_TERSE) > This causes an ICE, internal compiler error: tree check: expected class 'type', have 'declaration' (function_decl) Did you intend something like this: diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 561f8b23517..c61f0041fba 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -6121,3 +6121,8 @@ maybe_create_die_with_external_ref (tree decl) - /* Peel types in the context stack. */ - while (ctx && TYPE_P (ctx)) - ctx = TYPE_CONTEXT (ctx); + /* Peel types in the context stack. For functions peel the context up + to namespace/TU scope. The abstract copies reveal the true nesting. */ + while (ctx + && (TYPE_P (ctx) + || (TREE_CODE (decl) == FUNCTION_DECL + && TREE_CODE (ctx) == FUNCTION_DECL))) + ctx = TYPE_P (ctx) ? TYPE_CONTEXT (ctx) : DECL_CONTEXT (ctx); + > if that works it's OK. Can you run it on the gdb testsuite with -flto added > as well please (you need to do before/after comparison since IIRC adding > -flto will add a few fails). > > Thanks, > Richard. > -- Hafiz Abid Qadeer Mentor, a Siemens Business