Hi Cupertino,
On 2/20/24 02:24, Cupertino Miranda wrote:
> Dissociated .BTF.ext from the CO-RE relocations creation. Improvement of
> allocation/deallocation of BTF structures. Moving deallocation to final
> when needed.
>
> gcc/ChangeLog:
>
> * config/bpf/bpf.cc (bpf_option_override): Make BTF.ext enabled
> by default for BPF.
> (btf_asm_init_sections): Add btf deallocation.
> * dwarf2ctf.cc (ctf_debug_finalize): Fixed btf deallocation.
I find the commit message and ChangeLog here overly brief and a little
bit confusing.
You refer to moving the BTF deallocation to 'final', but IMO in this
context 'final' has the particular meaning of referring to pass_final,
which is not where the deallocation is moved. Rather, the patch moves it
to bpf_file_end, which implements the TARGET_ASM_FILE_END hook and is
called after pass_final. So I suggest to avoid calling it 'final', and
to explain a little in the commit message under what circumstances the
deallocation must be moved.
Also, I think the ChangeLog is missing an entry for bpf_file_end.
Please find a little note on a typo inline below.
> ---
> gcc/config/bpf/bpf.cc | 20 +++++++++-----------
> gcc/dwarf2ctf.cc | 5 ++++-
> 2 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
> index d6ca47eeecbe..4318b26b9cda 100644
> --- a/gcc/config/bpf/bpf.cc
> +++ b/gcc/config/bpf/bpf.cc
> @@ -195,10 +195,8 @@ bpf_option_override (void)
> if (TARGET_BPF_CORE && !btf_debuginfo_p ())
> error ("BPF CO-RE requires BTF debugging information, use %<-gbtf%>");
>
> - /* To support the portability needs of BPF CO-RE approach, BTF debug
> - information includes the BPF CO-RE relocations. */
> - if (TARGET_BPF_CORE)
> - write_symbols |= BTF_WITH_CORE_DEBUG;
> + /* BPF applications always generate .BTF.ext. */
> + write_symbols |= BTF_WITH_CORE_DEBUG;
>
> /* Unlike much of the other BTF debug information, the information
> necessary
> for CO-RE relocations is added to the CTF container by the BPF backend.
> @@ -218,10 +216,7 @@ bpf_option_override (void)
> /* -gbtf implies -mcore when using the BPF backend, unless -mno-co-re
> is specified. */
> if (btf_debuginfo_p () && !(target_flags_explicit & MASK_BPF_CORE))
> - {
> - target_flags |= MASK_BPF_CORE;
> - write_symbols |= BTF_WITH_CORE_DEBUG;
> - }
> + target_flags |= MASK_BPF_CORE;
>
> /* Determine available features from ISA setting (-mcpu=). */
> if (bpf_has_jmpext == -1)
> @@ -267,7 +262,7 @@ bpf_option_override (void)
> static void
> bpf_asm_init_sections (void)
> {
> - if (TARGET_BPF_CORE)
> + if (btf_debuginfo_p () && btf_with_core_debuginfo_p ())
> btf_ext_init ();
> }
>
> @@ -279,8 +274,11 @@ bpf_asm_init_sections (void)
> static void
> bpf_file_end (void)
> {
> - if (TARGET_BPF_CORE)
> - btf_ext_output ();
> + if (btf_debuginfo_p () && btf_with_core_debuginfo_p ())
> + {
> + btf_ext_output ();
> + btf_finalize ();
> + }
> }
>
> #undef TARGET_ASM_FILE_END
> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
> index 93e5619933fa..b9dfecf2c1c4 100644
> --- a/gcc/dwarf2ctf.cc
> +++ b/gcc/dwarf2ctf.cc
> @@ -944,7 +944,10 @@ ctf_debug_finalize (const char *filename, bool btf)
> if (btf)
> {
> btf_output (filename);
> - btf_finalize ();
> + /* btf_finalize when compiling BPF applciations gets deallocated by the
> + BPF target in bpf_file_end. */
typo: applications
> + if (btf_debuginfo_p () && !btf_with_core_debuginfo_p ())
> + btf_finalize ();
> }
>
> else
