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

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

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)?

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

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