Hi Richard.
Thanks for looking at this :)

[Today I sent a V4 of the series containing a couple of fixes to the BTF
 code.  It doesn't contain big changes so the discussion below still
 applies.]
 
>> In turn, debug_format_do_cu traverses the tree of DIEs passed to it calling
>> ctf_do_die on them.
>>
>> This conforms the debug format API:
>>
>>    FOO_debug_init ()
>>      Initialize the debug format FOO.
>>
>>    FOO_debug_finalize (FILENAME)
>>      Possibly write out, cleanup and finalization for debug format FOO.
>>
>>    FOO_do_die (DIE)
>>      Process the given DIE.
>>
>> Note how the emission of DWARF is interrupted after that point, if no
>> DWARF was requested by the user.
>
> I suppose since we're now supposed to write C++ the above fits
> a RAII style
>
>  if (ctf_debug_info_level > ...)
>   {
>     ctf_debug ctf (filename);
>     ctf.do_cu (comp_unit_die ());
>     for (...)
>        ctf.do_cu (node->die);
>   }
>
> but then I have no strong feelings about this.

We could turn it into a class, sure.

>> dwarf2out - dwarf2ctf
>> =====================
>>
>> The functions ctf_debug_init, ctf_do_die and ctf_debug_finalize, that
>> implement the API described above, are all in gcc/dwarf2ctf.c.
>>
>> Obviously, these routines need access to the dwarf DIE data
>> structures, and several functions which are defined in dwarf2out.[ch],
>> many (most?) of which are private to that file: dw_die_ref, get_AT,
>> etc.
>>
>> Therefore, in this implementation we opted by writing dwarf2ctf.c in a
>> way it can just be #included in dwarf2ctf.c.
>>
>> A question remains: would it be better to abstract these types and
>> functions in an API in dwarf2out.h?
>
> I think that refactoring the big dwarf2out.c would be indeed interesting.
> Not only exposing more details from dwarf2out.h (though we could start
> there).  One of the reason it's all private to dwarf2out.c is likely
> optimization (somewhat moot when you LTO GCC nowadays).
>
> So one interesting question is what's the API details CTF/BTF need?

In dwarf2ctf we use the following stuff defined in dwarf2out.c, which
are functions providing access to the DIE attributes:

- get_AT
- get_AT_ref
- get_AT_string
- AT_class
- AT_unsigned

So what if we start by adding non-static version of these functions and
export them in dwarf2out.h:

- dw_die_get_AT
- dw_die_get_AT_ref
- dw_die_get_AT_string
- dw_die_AT_class
- dw_die_AT_unsigned

?

> We might move data structures and basic accessors to
> dwarf2impl.{c,h} (as opposed to the already existing dwarf2.h).
> Or some better name (or as said, simply keep it in dwarf2out.c for now).
>
> Including dwarf2ctf.c is a bit ugly.

It is definitely ugly yes, but we didn't want to jump factoring an API
for dwarf2out without discussing it here first :)

>> Command line options for debug formats
>> ======================================
>>
>> This implementation adds the following command-line options to select the
>> emission of CTF and BTF:
>>
>>      -gctf[123]
>>      -gbtf[123]
>>
>> These options mimic the -g[123...] options for DWARF.
>
> Do -gcft and -gdwarf mix?  That is, can I have both in the same ELF
> file?  Since that doesn't work for -gdwarf -gstabs for example I wonder
> if we need to make this somehow different?

The idea is for them to mix, yes.

>> This involved adding new entries for debug_info_type:
>>
>>      CTF_DEBUG            - Write CTF debug info.
>>      BTF_DEBUG            - Write BTF debug info.
>>      CTF_AND_DWARF2_DEBUG - Write both CTF and DWARF info.
>>
>> Doing this, we just followed the trend initiated by vmsdbgout.c, which
>> added VMS_DEBUG and VMS_AND_DWARF2_DEBUG.
>>
>> This approach is not very good, because debug_info_type was designed
>> to cover different debug hook implementations; debug formats, in
>> contrast, are a different thing.
>>
>> This translates to problems and fragile behavior:
>>
>> - Everywhere write_symbols is checked we have to expand the logic to
>>   take the CTF values into account.  You can see that is the case in
>>   this patch series.  This is very fragile and doesn't scale well: we
>>   are most probably missing some checks.
>
> Would be nice to abstract most of the checks.  Like if the check
> is for "do I need to generate DWARF DIEs" then use
> need_dwarf_dies (), if it is for "do I want to output DWARF2 debug" then
> check output_dwarf ().  I see 64 places that check write_symbols,
> most in vmsdbgout.c ...
>
> Inventing predicates for the existing uses with your new uses in mind
> could make future changes easier at least.

Makes sense, we will look into abstracting these checks in suitable
predicates.  That certainly will improve things :)

>> - Backends select what debug hooks to use by defining constants like
>>   DWARF2_DEBUGGING_INFO.  Since the new debug formats are based on the
>>   DWARF debug hooks, that is the constant to define by the backends
>>   wanting to support DWARF + debug infos.
>>
>>   However, some backends may want to use one of the debug formats by
>>   default, i.e. for -g.  This is the case of the BPF backend, that
>>   needs to generate BTF instead of DWARF.  Currently, there is no way
>>   to specify this.
>
> It probably should define BTF_DEBUGGING_INFO and that should
> enable parts guarded with DWARF2_DEBUGGING_INFO as well.

Ok, so from a backend perspective there won't be any difference between
formats based on the DWARF debug hooks and (to deprecate) formats using
their own set of debug hooks...

Reply via email to