| Message ID | 1628124628-28706-3-git-send-email-indu.bhagat@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | Allow means for late BTF generation for BPF CO-RE | expand |
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. > 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 >
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. > >> 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 >>
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 > >> >
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 >>>> >>
On Tue, Aug 17, 2021 at 7:26 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: > > 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. Hmm, well. How about emitting .BTF.ext.string from GCC and have the linker merge the .BTF.ext.string section with the CTF string section then? You can't really say "the ship has sailed" if I read the CTF webpage - there seems to be many format changes planned. Well. Guess that was it from my side on the topic of ranting about the not well thought out debug format ;) Richard. > 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 > >>>> > >> >
On 8/18/21 12:00 AM, Richard Biener wrote: > On Tue, Aug 17, 2021 at 7:26 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: >> >> 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. > > Hmm, well. How about emitting .BTF.ext.string from GCC and have the linker > merge the .BTF.ext.string section with the CTF string section then? You can't > really say "the ship has sailed" if I read the CTF webpage - there seems to be > many format changes planned. > BTF originated from CTF long ago. These are two distinct formats and have their own independent evolution trail with specific use-cases. It's true that CTF format has changes planned for V4 and it's open for folks to give feedback or get involved. But BTF will not be a direct benefactor of any of those changes, as BPF/BTF ecosystem is evolving elsewhere (hopefully for the better). > Well. Guess that was it from my side on the topic of ranting about the > not well thought out debug format ;) > > Richard. > >> 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 >>>>>> >>>> >>
> On Tue, Aug 17, 2021 at 7:26 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: >> >> 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. > > Hmm, well. How about emitting .BTF.ext.string from GCC and have the linker > merge the .BTF.ext.string section with the CTF string section then? You can't > really say "the ship has sailed" if I read the CTF webpage - there seems to be > many format changes planned. Unfortunately we have little (if any) influence in the design of BPF, BTF and CO-RE. All of which have been designed and is being evolved by the kernel people. CTF, on the other hand, is unrelated to CO-RE, and we are definitely keeping LTO in mind when designing the extra extensions (like the backtraces support) that need input from the compiler backends. > Well. Guess that was it from my side on the topic of ranting about the > not well thought out debug format ;) Trust me, we feel you ;)
> Hmm, well. How about emitting .BTF.ext.string from GCC and have the linker > merge the .BTF.ext.string section with the CTF string section then? You can't > really say "the ship has sailed" if I read the CTF webpage - there seems to be > many format changes planned. Forgot to mention that BPF programs are never linked in practice, even if we support it in the GNU toolchain. BPF programmers compile C code into an object file, and then that object file is loaded in the kernel via libbpf. So they don't ever use the linker. A pity, because this was a neat idea.
On 8/18/21 12:00 AM, Richard Biener wrote: > On Tue, Aug 17, 2021 at 7:26 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: >> >> 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. > > Hmm, well. How about emitting .BTF.ext.string from GCC and have the linker > merge the .BTF.ext.string section with the CTF string section then? You can't > really say "the ship has sailed" if I read the CTF webpage - there seems to be > many format changes planned. > > Well. Guess that was it from my side on the topic of ranting about the > not well thought out debug format ;) > > Richard. Hello Richard, As we clarified in this thread, BTF/CO-RE format cannot be changed. What are your thoughts on this patch set now ? Is this OK ? Thanks Indu >> Thanks for reviewing and voicing your concerns. >> Indu >> >> >>> Richard.
On Tue, Aug 24, 2021 at 7:07 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: > > On 8/18/21 12:00 AM, Richard Biener wrote: > > On Tue, Aug 17, 2021 at 7:26 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: > >> > >> 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. > > > > Hmm, well. How about emitting .BTF.ext.string from GCC and have the linker > > merge the .BTF.ext.string section with the CTF string section then? You can't > > really say "the ship has sailed" if I read the CTF webpage - there seems to be > > many format changes planned. > > > > Well. Guess that was it from my side on the topic of ranting about the > > not well thought out debug format ;) > > > > Richard. > > Hello Richard, > > As we clarified in this thread, BTF/CO-RE format cannot be changed. What > are your thoughts on this patch set now ? Is this OK ? Since the issue is intrinsic to BTF/CO-RE and not the actual target can we do w/o a target hook by just gating on BTF_WITH_CORE as debug format or so? Richard. > Thanks > Indu > > >> Thanks for reviewing and voicing your concerns. > >> Indu > >> > >> > >>> Richard.
On 8/26/21 3:03 AM, Richard Biener wrote: > On Tue, Aug 24, 2021 at 7:07 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: >> >> On 8/18/21 12:00 AM, Richard Biener wrote: >>> On Tue, Aug 17, 2021 at 7:26 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: >>>> >>>> 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. >>> >>> Hmm, well. How about emitting .BTF.ext.string from GCC and have the linker >>> merge the .BTF.ext.string section with the CTF string section then? You can't >>> really say "the ship has sailed" if I read the CTF webpage - there seems to be >>> many format changes planned. >>> >>> Well. Guess that was it from my side on the topic of ranting about the >>> not well thought out debug format ;) >>> >>> Richard. >> >> Hello Richard, >> >> As we clarified in this thread, BTF/CO-RE format cannot be changed. What >> are your thoughts on this patch set now ? Is this OK ? > > Since the issue is intrinsic to BTF/CO-RE and not the actual target can we > do w/o a target hook by just gating on BTF_WITH_CORE as debug format > or so? > > Richard. > The issue is intrinsic to BTF debug format *when* CO-RE is in effect, so it is not entirely target independent because the whole "Compile Once - Run Everywhere" scheme is BPF backend specific. The debug information generation routines need to know if CO-RE is in effect (to finalize BTF debug info generation late and not early). Now, because it is the user who selects it via the -mco-re option, we need to have a way to detect this at run-time. Guarding it with a definition like BTF_WITH_CORE (is this what you meant?) will not work. But, yes, we can do without a target hook. We can keep a global var in the BTF context in btfout.c / CTF container (CTFC) which can be updated by the backend when BPF CO-RE is in effect (the -mco-re option). This was also considered as an option but the target hook option was chosen because it appeared to be the GCC's preferred way of conveying information from the backend. Is keeping global var preferable in this specific case ? Indu
On Thu, Aug 26, 2021 at 8:55 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: > > On 8/26/21 3:03 AM, Richard Biener wrote: > > On Tue, Aug 24, 2021 at 7:07 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: > >> > >> On 8/18/21 12:00 AM, Richard Biener wrote: > >>> On Tue, Aug 17, 2021 at 7:26 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: > >>>> > >>>> 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. > >>> > >>> Hmm, well. How about emitting .BTF.ext.string from GCC and have the linker > >>> merge the .BTF.ext.string section with the CTF string section then? You can't > >>> really say "the ship has sailed" if I read the CTF webpage - there seems to be > >>> many format changes planned. > >>> > >>> Well. Guess that was it from my side on the topic of ranting about the > >>> not well thought out debug format ;) > >>> > >>> Richard. > >> > >> Hello Richard, > >> > >> As we clarified in this thread, BTF/CO-RE format cannot be changed. What > >> are your thoughts on this patch set now ? Is this OK ? > > > > Since the issue is intrinsic to BTF/CO-RE and not the actual target can we > > do w/o a target hook by just gating on BTF_WITH_CORE as debug format > > or so? > > > > Richard. > > > > The issue is intrinsic to BTF debug format *when* CO-RE is in effect, so > it is not entirely target independent because the whole "Compile Once - > Run Everywhere" scheme is BPF backend specific. I see. > The debug information generation routines need to know if CO-RE is in > effect (to finalize BTF debug info generation late and not early). Now, > because it is the user who selects it via the -mco-re option, we need to > have a way to detect this at run-time. Guarding it with a definition > like BTF_WITH_CORE (is this what you meant?) will not work. I was thinking about having BTF_CORE_DEBUG in addition to BTF_DEBUG and thus have this part of the debug info format. That would be straight-forward in case the option to enable it were not backend specific but I guess it might be valid for the backend to alter ops->x_write_symbols in the backend option processing code. > But, yes, we can do without a target hook. We can keep a global var in > the BTF context in btfout.c / CTF container (CTFC) which can be updated > by the backend when BPF CO-RE is in effect (the -mco-re option). This > was also considered as an option but the target hook option was chosen > because it appeared to be the GCC's preferred way of conveying > information from the backend. Is keeping global var preferable in this > specific case ? No, a global variable would be worse. Richard. > > Indu
On 8/26/21 11:12 PM, Richard Biener wrote: > On Thu, Aug 26, 2021 at 8:55 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: >> >> On 8/26/21 3:03 AM, Richard Biener wrote: >>> On Tue, Aug 24, 2021 at 7:07 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: >>>> >>>> On 8/18/21 12:00 AM, Richard Biener wrote: >>>>> On Tue, Aug 17, 2021 at 7:26 PM Indu Bhagat <indu.bhagat@oracle.com> wrote: >>>>>> >>>>>> 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. >>>>> >>>>> Hmm, well. How about emitting .BTF.ext.string from GCC and have the linker >>>>> merge the .BTF.ext.string section with the CTF string section then? You can't >>>>> really say "the ship has sailed" if I read the CTF webpage - there seems to be >>>>> many format changes planned. >>>>> >>>>> Well. Guess that was it from my side on the topic of ranting about the >>>>> not well thought out debug format ;) >>>>> >>>>> Richard. >>>> >>>> Hello Richard, >>>> >>>> As we clarified in this thread, BTF/CO-RE format cannot be changed. What >>>> are your thoughts on this patch set now ? Is this OK ? >>> >>> Since the issue is intrinsic to BTF/CO-RE and not the actual target can we >>> do w/o a target hook by just gating on BTF_WITH_CORE as debug format >>> or so? >>> >>> Richard. >>> >> >> The issue is intrinsic to BTF debug format *when* CO-RE is in effect, so >> it is not entirely target independent because the whole "Compile Once - >> Run Everywhere" scheme is BPF backend specific. > > I see. > >> The debug information generation routines need to know if CO-RE is in >> effect (to finalize BTF debug info generation late and not early). Now, >> because it is the user who selects it via the -mco-re option, we need to >> have a way to detect this at run-time. Guarding it with a definition >> like BTF_WITH_CORE (is this what you meant?) will not work. > > I was thinking about having BTF_CORE_DEBUG in addition to BTF_DEBUG > and thus have this part of the debug info format. That would be > straight-forward > in case the option to enable it were not backend specific but I guess it might > be valid for the backend to alter ops->x_write_symbols in the backend > option processing code. > This is doable. I updated the patch series and have posted V3. Thanks Indu >> But, yes, we can do without a target hook. We can keep a global var in >> the BTF context in btfout.c / CTF container (CTFC) which can be updated >> by the backend when BPF CO-RE is in effect (the -mco-re option). This >> was also considered as an option but the target hook option was chosen >> because it appeared to be the GCC's preferred way of conveying >> information from the backend. Is keeping global var preferable in this >> specific case ? > > No, a global variable would be worse. > > Richard. > >> >> Indu
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 ();