Re: [PATCH] [DWARF] Fix hierarchy of debug information for offload kernels.

[Richard Biener <richard.guenther@gmail.com>]

On July 19, 2021 6:13:40 PM GMT+02:00, Hafiz Abid Qadeer <abid_qadeer@mentor.com> wrote:
>On 19/07/2021 11:45, Richard Biener wrote:
>> On Fri, Jul 16, 2021 at 10:23 PM Hafiz Abid Qadeer
>> <abid_qadeer@mentor.com> wrote:
>>>
>>> On 15/07/2021 13:09, Richard Biener wrote:
>>>> On Thu, Jul 15, 2021 at 12:35 PM Hafiz Abid Qadeer
>>>> <abid_qadeer@mentor.com> 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    ("<artificial>")
>>>>
>>>>   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);
>+

Yes, of course. 

>
>> 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.
>>