Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission

[Indu Bhagat <indu.bhagat@oracle.com>]

On 8/17/21 1:04 AM, Richard Biener wrote:
> On Mon, Aug 16, 2021 at 7:39 PM Indu Bhagat <indu.bhagat@oracle.com> wrote:
>>
>> On 8/10/21 4:54 AM, Richard Biener wrote:
>>> On Thu, Aug 5, 2021 at 2:52 AM Indu Bhagat via Gcc-patches
>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>
>>>> This patch adds a new target hook to detect if the CTF container can allow the
>>>> emission of CTF/BTF debug info at DWARF debug info early finish time. Some
>>>> backends, e.g., BPF when generating code for CO-RE usecase, may need to emit
>>>> the CTF/BTF debug info sections around the time when late DWARF debug is
>>>> finalized (dwarf2out_finish).
>>>
>>> Without looking at the dwarf2out.c usage in the next patch - I think
>>> the CTF part
>>> should be always emitted from dwarf2out_early_finish, the "hooks" should somehow
>>> arrange for the alternate output specific data to be preserved until
>>> dwarf2out_finish
>>> time so the late BTF data can be emitted from there.
>>>
>>> Lumping everything together now just makes it harder to see what info
>>> is required
>>> to persist and thus make LTO support more intrusive than necessary.
>>
>> In principle, I agree the approach to split generate/emit CTF/BTF like
>> you mention is ideal.  But, the BTF CO-RE relocations format is such
>> that the .BTF section cannot be finalized until .BTF.ext contents are
>> all fully known (David Faust summarizes this issue in the other thread
>> "[PATCH, V2 3/3] dwarf2out: Emit BTF in dwarf2out_finish for BPF CO-RE
>> usecase".)
>>
>> In summary, the .BTF.ext section refers to strings in the .BTF section.
>> These strings are added at the time the CO-RE relocations are added.
>> Recall that the .BTF section's header has information about the .BTF
>> string table start offset and length. So, this means the "CTF part" (or
>> the .BTF section) cannot simply be emitted in the dwarf2out_early_finish
>> because it's not ready yet. If it is still unclear, please let me know.
>>
>> My judgement here is that the BTF format itself is not amenable to split
>> early/late emission like DWARF. BTF has no linker support yet either.
> 
> But are the strings used for the CO-RE relocations not all present already?
> Or does the "CTF part" have only "foo", "bar" and "baz" while the CO-RE
> part wants to output sth like "foo->bar.baz" (which IMHO would be quite
> stupid also for size purposes)?
> 

Yes, the latter ("foo->bar.baz") is closer to what the format does for 
CO-RE relocations!

> That said, fix the format.
> 
> Alternatively hand the CO-RE part its own string table (what's the fuss
> with re-using the CTF string table if there's nothing to share ...)
> 

BTF and .BTF.ext formats are specified already by implementations in the 
kernel, libbpf, and LLVM. For that matter, I should add BPF CO-RE to the 
mix and say that BPF CO-RE capability _and_ .BTF/.BTF.ext debug formats 
have been defined already by the BPF kernel developers/associated 
entities. At this time, we as GCC developers simply extending the BPF 
backend/BTF generation support in GCC, cannot fix the format. That ship 
has sailed.

Thanks for reviewing and voicing your concerns.
Indu


> Richard.
> 
>>>
>>>> gcc/ChangeLog:
>>>>
>>>>           * config/bpf/bpf.c (ctfc_debuginfo_early_finish_p): New definition.
>>>>           (TARGET_CTFC_DEBUGINFO_EARLY_FINISH_P): Undefine and override.
>>>>           * doc/tm.texi: Regenerated.
>>>>           * doc/tm.texi.in: Document the new hook.
>>>>           * target.def: Add a new hook.
>>>>           * targhooks.c (default_ctfc_debuginfo_early_finish_p): Likewise.
>>>>           * targhooks.h (default_ctfc_debuginfo_early_finish_p): Likewise.
>>>> ---
>>>>    gcc/config/bpf/bpf.c | 14 ++++++++++++++
>>>>    gcc/doc/tm.texi      |  6 ++++++
>>>>    gcc/doc/tm.texi.in   |  2 ++
>>>>    gcc/target.def       | 10 ++++++++++
>>>>    gcc/targhooks.c      |  6 ++++++
>>>>    gcc/targhooks.h      |  2 ++
>>>>    6 files changed, 40 insertions(+)
>>>>
>>>> diff --git a/gcc/config/bpf/bpf.c b/gcc/config/bpf/bpf.c
>>>> index 028013e..85f6b76 100644
>>>> --- a/gcc/config/bpf/bpf.c
>>>> +++ b/gcc/config/bpf/bpf.c
>>>> @@ -178,6 +178,20 @@ bpf_option_override (void)
>>>>    #undef TARGET_OPTION_OVERRIDE
>>>>    #define TARGET_OPTION_OVERRIDE bpf_option_override
>>>>
>>>> +/* Return FALSE iff -mcore has been specified.  */
>>>> +
>>>> +static bool
>>>> +ctfc_debuginfo_early_finish_p (void)
>>>> +{
>>>> +  if (TARGET_BPF_CORE)
>>>> +    return false;
>>>> +  else
>>>> +    return true;
>>>> +}
>>>> +
>>>> +#undef TARGET_CTFC_DEBUGINFO_EARLY_FINISH_P
>>>> +#define TARGET_CTFC_DEBUGINFO_EARLY_FINISH_P ctfc_debuginfo_early_finish_p
>>>> +
>>>>    /* Define target-specific CPP macros.  This function in used in the
>>>>       definition of TARGET_CPU_CPP_BUILTINS in bpf.h */
>>>>
>>>> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
>>>> index cb01528..2d5ff05 100644
>>>> --- a/gcc/doc/tm.texi
>>>> +++ b/gcc/doc/tm.texi
>>>> @@ -10400,6 +10400,12 @@ Define this macro if GCC should produce debugging output in BTF debug
>>>>    format in response to the @option{-gbtf} option.
>>>>    @end defmac
>>>>
>>>> +@deftypefn {Target Hook} bool TARGET_CTFC_DEBUGINFO_EARLY_FINISH_P (void)
>>>> +This target hook returns nonzero if the CTF Container can allow the
>>>> + emission of the CTF/BTF debug info at the DWARF debuginfo early finish
>>>> + time.
>>>> +@end deftypefn
>>>> +
>>>>    @node Floating Point
>>>>    @section Cross Compilation and Floating Point
>>>>    @cindex cross compilation and floating point
>>>> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
>>>> index 4a522ae..05b3c2c 100644
>>>> --- a/gcc/doc/tm.texi.in
>>>> +++ b/gcc/doc/tm.texi.in
>>>> @@ -7020,6 +7020,8 @@ Define this macro if GCC should produce debugging output in BTF debug
>>>>    format in response to the @option{-gbtf} option.
>>>>    @end defmac
>>>>
>>>> +@hook TARGET_CTFC_DEBUGINFO_EARLY_FINISH_P
>>>> +
>>>>    @node Floating Point
>>>>    @section Cross Compilation and Floating Point
>>>>    @cindex cross compilation and floating point
>>>> diff --git a/gcc/target.def b/gcc/target.def
>>>> index 68a46aa..44e2251 100644
>>>> --- a/gcc/target.def
>>>> +++ b/gcc/target.def
>>>> @@ -4016,6 +4016,16 @@ clobbered parts of a register altering the frame register size",
>>>>     machine_mode, (int regno),
>>>>     default_dwarf_frame_reg_mode)
>>>>
>>>> +/* Return nonzero if CTF Container can finalize the CTF/BTF emission
>>>> +   at DWARF debuginfo early finish time.  */
>>>> +DEFHOOK
>>>> +(ctfc_debuginfo_early_finish_p,
>>>> + "This target hook returns nonzero if the CTF Container can allow the\n\
>>>> + emission of the CTF/BTF debug info at the DWARF debuginfo early finish\n\
>>>> + time.",
>>>> + bool, (void),
>>>> + default_ctfc_debuginfo_early_finish_p)
>>>> +
>>>>    /* If expand_builtin_init_dwarf_reg_sizes needs to fill in table
>>>>       entries not corresponding directly to registers below
>>>>       FIRST_PSEUDO_REGISTER, this hook should generate the necessary
>>>> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
>>>> index eb51909..e38566c 100644
>>>> --- a/gcc/targhooks.c
>>>> +++ b/gcc/targhooks.c
>>>> @@ -2112,6 +2112,12 @@ default_dwarf_frame_reg_mode (int regno)
>>>>      return save_mode;
>>>>    }
>>>>
>>>> +bool
>>>> +default_ctfc_debuginfo_early_finish_p (void)
>>>> +{
>>>> +  return true;
>>>> +}
>>>> +
>>>>    /* To be used by targets where reg_raw_mode doesn't return the right
>>>>       mode for registers used in apply_builtin_return and apply_builtin_arg.  */
>>>>
>>>> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
>>>> index f92e102..55dc443 100644
>>>> --- a/gcc/targhooks.h
>>>> +++ b/gcc/targhooks.h
>>>> @@ -255,6 +255,8 @@ extern unsigned int default_dwarf_poly_indeterminate_value (unsigned int,
>>>>                                                               unsigned int *,
>>>>                                                               int *);
>>>>    extern machine_mode default_dwarf_frame_reg_mode (int);
>>>> +extern bool default_ctfc_debuginfo_early_finish_p (void);
>>>> +
>>>>    extern fixed_size_mode default_get_reg_raw_mode (int);
>>>>    extern bool default_keep_leaf_when_profiled ();
>>>>
>>>> --
>>>> 1.8.3.1
>>>>
>>