* [PATCH,V2 0/3] Allow means for late BTF generation for BPF CO-RE
@ 2021-08-05 0:50 Indu Bhagat
2021-08-05 0:50 ` [PATCH,V2 1/3] bpf: Add new -mcore option " Indu Bhagat
` (2 more replies)
0 siblings, 3 replies; 23+ messages in thread
From: Indu Bhagat @ 2021-08-05 0:50 UTC (permalink / raw)
To: gcc-patches
[Changes from V1]
- [1/3] bpf: Add new -mcore option for BPF CO-RE
Moved the testcase from gcc.dg/debug/btf/ to gcc.target/bpf/. Adjusted the
testcase a bit.
- targhooks: New target hook for CTF/BTF debug info emission
(Same as V1)
- dwarf2out: Emit BTF in dwarf2out_finish for BPF CO-RE usecase
Moved the call to ctf_debug_finish (in dwarf2out_finish) before the point of
early exit taken when dwarf_debuginfo_p () is false.
[End of Changes from V1]
Hello,
This patch series puts the framework in place for late BTF generation (in
dwarf2out_finish). This is needed for the landing of BPF CO-RE support in GCC,
patches for which were posted recently
https://gcc.gnu.org/pipermail/gcc-patches/2021-August/576719.html.
BPF's Compile Once - Run Everywhere (CO-RE) feature is used to make a compiled
BPF program portable across kernel versions, all this without the need to
recompile the BPF program. A key part of BPF CO-RE capability is the BTF debug
info generated for them.
A traditional BPF program (non CO-RE) will have a .BTF section which contains
the type information in the BTF debug format. In case of CO-RE, however, an
additional section .BTF.ext section is generated. The .BTF.ext section contains
the CO-RE relocations. A BPF loader will use the .BTF.ext section along with the
associated .BTF.ext section to adjust some references in the instructions of
program to ensure it is compatible with the required kernel version / headers.
Roughly, each CO-RE relocation record will contain the following info
- offset of BPF instruction to be patched
- the BTF ID of the data structure being accessed by the instruction, and
- an offset to the BTF string which encodes a series of field accesses to
retrieve the field of interest in the instruction.
High-level design
-----------------
- The CTF container is populated with the compiler-internal representation for
the "type information" at dwarf2out_early_finish time.
- In case of CO-RE compilation, the information needed to generate .BTF.ext
section is added by the BPF backend to the CTF container (CTFC) at XXX time.
This introduces challenges in having LTO support for CO-RE - CO-RE relocations
can only be generated late, much like late DWARF.
- Combining late and early BTF is not being done as the patch set disables LTO
to be used together with CO-RE for the BPF target.
- A new target hook is added for the CTFC (CTF Container) to know whether early
emission of CTF/BTF is allowed for the target.
Testing Notes
------------
- Bootstrapped and reg tested on x86_64
- make all-gcc for --target=bpf-unknown-none; tested ctf.exp, btf.exp and bpf.exp
Thanks,
Indu Bhagat (3):
bpf: Add new -mcore option for BPF CO-RE
targhooks: New target hook for CTF/BTF debug info emission
dwarf2out: Emit BTF in dwarf2out_finish for BPF CO-RE usecase
gcc/config/bpf/bpf.c | 29 ++++++++++++++++
gcc/config/bpf/bpf.opt | 4 +++
gcc/doc/tm.texi | 6 ++++
gcc/doc/tm.texi.in | 2 ++
gcc/dwarf2ctf.c | 55 ++++++++++++++++++++++++-------
gcc/dwarf2ctf.h | 4 ++-
gcc/dwarf2out.c | 9 +++--
gcc/target.def | 10 ++++++
gcc/targhooks.c | 6 ++++
gcc/targhooks.h | 2 ++
gcc/testsuite/gcc.target/bpf/core-lto-1.c | 9 +++++
11 files changed, 121 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/bpf/core-lto-1.c
--
1.8.3.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH,V2 1/3] bpf: Add new -mcore option for BPF CO-RE
2021-08-05 0:50 [PATCH,V2 0/3] Allow means for late BTF generation for BPF CO-RE Indu Bhagat
@ 2021-08-05 0:50 ` Indu Bhagat
2021-08-10 11:51 ` Richard Biener
2021-08-05 0:50 ` [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission Indu Bhagat
2021-08-05 0:50 ` [PATCH, V2 3/3] dwarf2out: Emit BTF in dwarf2out_finish for BPF CO-RE usecase Indu Bhagat
2 siblings, 1 reply; 23+ messages in thread
From: Indu Bhagat @ 2021-08-05 0:50 UTC (permalink / raw)
To: gcc-patches
-mcore in the BPF backend enables code generation for the CO-RE usecase. LTO is
disabled for CO-RE compilations.
gcc/ChangeLog:
* config/bpf/bpf.c (bpf_option_override): For BPF backend, disable LTO
support when compiling for CO-RE.
* config/bpf/bpf.opt: Add new command line option -mcore.
gcc/testsuite/ChangeLog:
* gcc.target/bpf/core-lto-1.c: New test.
---
gcc/config/bpf/bpf.c | 15 +++++++++++++++
gcc/config/bpf/bpf.opt | 4 ++++
gcc/testsuite/gcc.target/bpf/core-lto-1.c | 9 +++++++++
3 files changed, 28 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/bpf/core-lto-1.c
diff --git a/gcc/config/bpf/bpf.c b/gcc/config/bpf/bpf.c
index e635f9e..028013e 100644
--- a/gcc/config/bpf/bpf.c
+++ b/gcc/config/bpf/bpf.c
@@ -158,6 +158,21 @@ bpf_option_override (void)
{
/* Set the initializer for the per-function status structure. */
init_machine_status = bpf_init_machine_status;
+
+ /* To support the portability needs of BPF CO-RE approach, BTF debug
+ information includes the BPF CO-RE relocations. The information
+ necessary for these relocations is added to the CTF container by the
+ BPF backend. Enabling LTO poses challenges in the generation of the BPF
+ CO-RE relocations because if LTO is in effect, they need to be
+ generated late in the LTO link phase. This in turn means the compiler
+ needs to provide means to combine the early and late BTF debug info,
+ similar to DWARF debug info.
+
+ In any case, in absence of linker support for BTF sections at this time,
+ it is acceptable to simply disallow LTO for BPF CO-RE compilations. */
+
+ if (flag_lto && TARGET_BPF_CORE)
+ error ("BPF CO-RE does not support LTO");
}
#undef TARGET_OPTION_OVERRIDE
diff --git a/gcc/config/bpf/bpf.opt b/gcc/config/bpf/bpf.opt
index 916b53c..e8926f5 100644
--- a/gcc/config/bpf/bpf.opt
+++ b/gcc/config/bpf/bpf.opt
@@ -127,3 +127,7 @@ Generate little-endian eBPF.
mframe-limit=
Target Joined RejectNegative UInteger IntegerRange(0, 32767) Var(bpf_frame_limit) Init(512)
Set a hard limit for the size of each stack frame, in bytes.
+
+mcore
+Target Mask(BPF_CORE)
+Generate all necessary information for BPF Compile Once - Run Everywhere.
diff --git a/gcc/testsuite/gcc.target/bpf/core-lto-1.c b/gcc/testsuite/gcc.target/bpf/core-lto-1.c
new file mode 100644
index 0000000..a90dc5b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bpf/core-lto-1.c
@@ -0,0 +1,9 @@
+/* Test -mcore with -flto.
+
+ -mcore is used to generate information for BPF CO-RE usecase. To support
+ the generataion of the .BTF and .BTF.ext sections in GCC, -flto is disabled
+ with -mcore. */
+
+/* { dg-do compile } */
+/* { dg-error "BPF CO-RE does not support LTO" "" { target bpf-*-* } 0 } */
+/* { dg-options "-gbtf -mcore -flto" } */
--
1.8.3.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-05 0:50 [PATCH,V2 0/3] Allow means for late BTF generation for BPF CO-RE Indu Bhagat
2021-08-05 0:50 ` [PATCH,V2 1/3] bpf: Add new -mcore option " Indu Bhagat
@ 2021-08-05 0:50 ` Indu Bhagat
2021-08-10 11:54 ` Richard Biener
2021-08-05 0:50 ` [PATCH, V2 3/3] dwarf2out: Emit BTF in dwarf2out_finish for BPF CO-RE usecase Indu Bhagat
2 siblings, 1 reply; 23+ messages in thread
From: Indu Bhagat @ 2021-08-05 0:50 UTC (permalink / raw)
To: gcc-patches
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).
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH, V2 3/3] dwarf2out: Emit BTF in dwarf2out_finish for BPF CO-RE usecase
2021-08-05 0:50 [PATCH,V2 0/3] Allow means for late BTF generation for BPF CO-RE Indu Bhagat
2021-08-05 0:50 ` [PATCH,V2 1/3] bpf: Add new -mcore option " Indu Bhagat
2021-08-05 0:50 ` [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission Indu Bhagat
@ 2021-08-05 0:50 ` Indu Bhagat
2021-08-10 12:00 ` Richard Biener
2 siblings, 1 reply; 23+ messages in thread
From: Indu Bhagat @ 2021-08-05 0:50 UTC (permalink / raw)
To: gcc-patches
DWARF generation is split between early and late phases when LTO is in effect.
This poses challenges for CTF/BTF generation especially if late debug info
generation is desirable, as turns out to be the case for BPF CO-RE.
In case of BPF CO-RE, the BPF backend adds information about CO-RE relocations
to the CTF container. This information is what needs to be emitted as a
separate .BTF.ext section when -more is in effect. Further, each CO-RE
relocation record holds an offset to a string specifying the access to the
structure's field. This means that .BTF string table needs to be modified
"late" in the compilation process. In other words, this implies that the BTF
sections cannot be finalized in dwarf2out_early_finish when -mcore for the BPF
backend is in effect.
Now, the emission of CTF/BTF debug info cannot be moved unconditionally to
dwarf2out_finish because dwarf2out_finish is not invoked at all for the LTO
compile phase for slim LTO objects, thus breaking CTF/BTF generation for other
targets when used with LTO.
The approach taken here in this patch is that -
1. LTO is disabled for BPF CO-RE
The reason to disable LTO for BPF CO-RE is that if LTO is in effect, BPF CO-RE
relocations need to be generated in the LTO link phase _after_ the optimizations
are done. This means we need to devise way to combine early and late BTF. At
this time, in absence of linker support for BTF sections, it makes sense to
steer clear of LTO for BPF CO-RE and bypass the issue.
2. Use a target hook to allow BPF backend to cleanly convey the case when late
finalization of the CTF container is desirable.
So, in other words,
dwarf2out_early_finish
- Always emit CTF here.
- if (BTF && ctfc_debuginfo_early_finish_p), emit BTF now.
dwarf2out_finish
- if (BTF && !ctfc_debuginfo_early_finish_p && !in_lto_p) emit BTF now.
- Use of in_lto_p to make sure LTO link phase does not affect BTF sections
for other targets.
gcc/ChangeLog:
* dwarf2ctf.c (ctf_debug_finalize): Make it static.
(ctf_debug_early_finish): New definition.
(ctf_debug_finish): Likewise.
* dwarf2ctf.h (ctf_debug_finalize): Remove declaration.
(ctf_debug_early_finish): New declaration.
(ctf_debug_finish): Likewise.
* dwarf2out.c (dwarf2out_finish): Invoke ctf_debug_finish.
(dwarf2out_early_finish): Invoke ctf_debug_early_finish.
---
gcc/dwarf2ctf.c | 55 +++++++++++++++++++++++++++++++++++++++++++------------
gcc/dwarf2ctf.h | 4 +++-
gcc/dwarf2out.c | 9 +++++++--
3 files changed, 53 insertions(+), 15 deletions(-)
diff --git a/gcc/dwarf2ctf.c b/gcc/dwarf2ctf.c
index 5e8a725..0fa429c 100644
--- a/gcc/dwarf2ctf.c
+++ b/gcc/dwarf2ctf.c
@@ -917,6 +917,27 @@ gen_ctf_type (ctf_container_ref ctfc, dw_die_ref die)
return type_id;
}
+/* Prepare for output and write out the CTF debug information. */
+
+static void
+ctf_debug_finalize (const char *filename, bool btf)
+{
+ if (btf)
+ {
+ btf_output (filename);
+ btf_finalize ();
+ }
+
+ else
+ {
+ /* Emit the collected CTF information. */
+ ctf_output (filename);
+
+ /* Reset the CTF state. */
+ ctf_finalize ();
+ }
+}
+
bool
ctf_do_die (dw_die_ref die)
{
@@ -966,25 +987,35 @@ ctf_debug_init_postprocess (bool btf)
btf_init_postprocess ();
}
-/* Prepare for output and write out the CTF debug information. */
+/* Early finish CTF/BTF debug info. */
void
-ctf_debug_finalize (const char *filename, bool btf)
+ctf_debug_early_finish (const char * filename)
{
- if (btf)
+ /* Emit CTF debug info early always. */
+ if (ctf_debug_info_level > CTFINFO_LEVEL_NONE
+ /* Emit BTF debug info early if the target does not require late
+ emission. */
+ || (btf_debuginfo_p ()
+ && targetm.ctfc_debuginfo_early_finish_p ()))
{
- btf_output (filename);
- btf_finalize ();
+ /* Emit CTF/BTF debug info. */
+ ctf_debug_finalize (filename, btf_debuginfo_p ());
}
+}
- else
- {
- /* Emit the collected CTF information. */
- ctf_output (filename);
+/* Finish CTF/BTF debug info emission. */
- /* Reset the CTF state. */
- ctf_finalize ();
- }
+void
+ctf_debug_finish (const char * filename)
+{
+ /* Emit BTF debug info here when the target needs to update the CTF container
+ (ctfc) in the backend. An example of this, at this time is the BPF CO-RE
+ usecase. */
+ if (btf_debuginfo_p ()
+ && (!in_lto_p && !targetm.ctfc_debuginfo_early_finish_p ()))
+ /* Emit BTF debug info. */
+ ctf_debug_finalize (filename, btf_debuginfo_p ());
}
#include "gt-dwarf2ctf.h"
diff --git a/gcc/dwarf2ctf.h b/gcc/dwarf2ctf.h
index a3cf567..9edbde0 100644
--- a/gcc/dwarf2ctf.h
+++ b/gcc/dwarf2ctf.h
@@ -24,13 +24,15 @@ along with GCC; see the file COPYING3. If not see
#define GCC_DWARF2CTF_H 1
#include "dwarf2out.h"
+#include "flags.h"
/* Debug Format Interface. Used in dwarf2out.c. */
extern void ctf_debug_init (void);
extern void ctf_debug_init_postprocess (bool);
extern bool ctf_do_die (dw_die_ref);
-extern void ctf_debug_finalize (const char *, bool);
+extern void ctf_debug_early_finish (const char *);
+extern void ctf_debug_finish (const char *);
/* Wrappers for CTF/BTF to fetch information from GCC DWARF DIE. Used in
ctfc.c.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index b91a9b5..708cd1f 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -31895,6 +31895,11 @@ dwarf2out_finish (const char *filename)
unsigned char checksum[16];
char dl_section_ref[MAX_ARTIFICIAL_LABEL_BYTES];
+ /* Generate CTF/BTF debug info. */
+ if ((ctf_debug_info_level > CTFINFO_LEVEL_NONE
+ || btf_debuginfo_p ()) && lang_GNU_C ())
+ ctf_debug_finish (filename);
+
/* Skip emitting DWARF if not required. */
if (!dwarf_debuginfo_p ())
return;
@@ -32799,8 +32804,8 @@ dwarf2out_early_finish (const char *filename)
ctf_debug_do_cu (node->die);
/* Post process the debug data in the CTF container if necessary. */
ctf_debug_init_postprocess (btf_debuginfo_p ());
- /* Emit CTF/BTF debug info. */
- ctf_debug_finalize (filename, btf_debuginfo_p ());
+
+ ctf_debug_early_finish (filename);
}
/* Do not generate DWARF assembler now when not producing LTO bytecode. */
--
1.8.3.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH,V2 1/3] bpf: Add new -mcore option for BPF CO-RE
2021-08-05 0:50 ` [PATCH,V2 1/3] bpf: Add new -mcore option " Indu Bhagat
@ 2021-08-10 11:51 ` Richard Biener
2021-08-10 15:45 ` Jose E. Marchesi
0 siblings, 1 reply; 23+ messages in thread
From: Richard Biener @ 2021-08-10 11:51 UTC (permalink / raw)
To: Indu Bhagat; +Cc: GCC Patches
On Thu, Aug 5, 2021 at 2:54 AM Indu Bhagat via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> -mcore in the BPF backend enables code generation for the CO-RE usecase. LTO is
> disabled for CO-RE compilations.
-mcore reads like "core", why not -mco-re? Anyway, ...
> gcc/ChangeLog:
>
> * config/bpf/bpf.c (bpf_option_override): For BPF backend, disable LTO
> support when compiling for CO-RE.
> * config/bpf/bpf.opt: Add new command line option -mcore.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/bpf/core-lto-1.c: New test.
> ---
> gcc/config/bpf/bpf.c | 15 +++++++++++++++
> gcc/config/bpf/bpf.opt | 4 ++++
> gcc/testsuite/gcc.target/bpf/core-lto-1.c | 9 +++++++++
> 3 files changed, 28 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/bpf/core-lto-1.c
>
> diff --git a/gcc/config/bpf/bpf.c b/gcc/config/bpf/bpf.c
> index e635f9e..028013e 100644
> --- a/gcc/config/bpf/bpf.c
> +++ b/gcc/config/bpf/bpf.c
> @@ -158,6 +158,21 @@ bpf_option_override (void)
> {
> /* Set the initializer for the per-function status structure. */
> init_machine_status = bpf_init_machine_status;
> +
> + /* To support the portability needs of BPF CO-RE approach, BTF debug
> + information includes the BPF CO-RE relocations. The information
> + necessary for these relocations is added to the CTF container by the
> + BPF backend. Enabling LTO poses challenges in the generation of the BPF
> + CO-RE relocations because if LTO is in effect, they need to be
> + generated late in the LTO link phase. This in turn means the compiler
> + needs to provide means to combine the early and late BTF debug info,
> + similar to DWARF debug info.
> +
> + In any case, in absence of linker support for BTF sections at this time,
> + it is acceptable to simply disallow LTO for BPF CO-RE compilations. */
> +
> + if (flag_lto && TARGET_BPF_CORE)
> + error ("BPF CO-RE does not support LTO");
... these "errors" should use sorry ("...") which annotate places where the
compiler has to give up because sth is not implemented.
otherwise this patch needs BPF maintainer review of course.
Richard.
> }
>
> #undef TARGET_OPTION_OVERRIDE
> diff --git a/gcc/config/bpf/bpf.opt b/gcc/config/bpf/bpf.opt
> index 916b53c..e8926f5 100644
> --- a/gcc/config/bpf/bpf.opt
> +++ b/gcc/config/bpf/bpf.opt
> @@ -127,3 +127,7 @@ Generate little-endian eBPF.
> mframe-limit=
> Target Joined RejectNegative UInteger IntegerRange(0, 32767) Var(bpf_frame_limit) Init(512)
> Set a hard limit for the size of each stack frame, in bytes.
> +
> +mcore
> +Target Mask(BPF_CORE)
> +Generate all necessary information for BPF Compile Once - Run Everywhere.
> diff --git a/gcc/testsuite/gcc.target/bpf/core-lto-1.c b/gcc/testsuite/gcc.target/bpf/core-lto-1.c
> new file mode 100644
> index 0000000..a90dc5b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/bpf/core-lto-1.c
> @@ -0,0 +1,9 @@
> +/* Test -mcore with -flto.
> +
> + -mcore is used to generate information for BPF CO-RE usecase. To support
> + the generataion of the .BTF and .BTF.ext sections in GCC, -flto is disabled
> + with -mcore. */
> +
> +/* { dg-do compile } */
> +/* { dg-error "BPF CO-RE does not support LTO" "" { target bpf-*-* } 0 } */
> +/* { dg-options "-gbtf -mcore -flto" } */
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-05 0:50 ` [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission Indu Bhagat
@ 2021-08-10 11:54 ` Richard Biener
2021-08-16 17:39 ` Indu Bhagat
0 siblings, 1 reply; 23+ messages in thread
From: Richard Biener @ 2021-08-10 11:54 UTC (permalink / raw)
To: Indu Bhagat; +Cc: GCC Patches
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
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 3/3] dwarf2out: Emit BTF in dwarf2out_finish for BPF CO-RE usecase
2021-08-05 0:50 ` [PATCH, V2 3/3] dwarf2out: Emit BTF in dwarf2out_finish for BPF CO-RE usecase Indu Bhagat
@ 2021-08-10 12:00 ` Richard Biener
2021-08-10 16:39 ` David Faust
0 siblings, 1 reply; 23+ messages in thread
From: Richard Biener @ 2021-08-10 12:00 UTC (permalink / raw)
To: Indu Bhagat; +Cc: GCC Patches
On Thu, Aug 5, 2021 at 2:53 AM Indu Bhagat via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> DWARF generation is split between early and late phases when LTO is in effect.
> This poses challenges for CTF/BTF generation especially if late debug info
> generation is desirable, as turns out to be the case for BPF CO-RE.
>
> In case of BPF CO-RE, the BPF backend adds information about CO-RE relocations
> to the CTF container. This information is what needs to be emitted as a
> separate .BTF.ext section when -more is in effect. Further, each CO-RE
> relocation record holds an offset to a string specifying the access to the
> structure's field. This means that .BTF string table needs to be modified
> "late" in the compilation process. In other words, this implies that the BTF
> sections cannot be finalized in dwarf2out_early_finish when -mcore for the BPF
> backend is in effect.
OK, it didn't really get any clearer as to why the late annotation
cannot be done
after the early info was output. Something to do with the BTF string table,
but all structure field names must be already present there so I must be missing
something.
ISTR the CO-RE info is fully contained in a new section .BTF.ext and the
"early" .BTF section is not altered?
> Now, the emission of CTF/BTF debug info cannot be moved unconditionally to
> dwarf2out_finish because dwarf2out_finish is not invoked at all for the LTO
> compile phase for slim LTO objects, thus breaking CTF/BTF generation for other
> targets when used with LTO.
>
> The approach taken here in this patch is that -
>
> 1. LTO is disabled for BPF CO-RE
> The reason to disable LTO for BPF CO-RE is that if LTO is in effect, BPF CO-RE
> relocations need to be generated in the LTO link phase _after_ the optimizations
> are done. This means we need to devise way to combine early and late BTF. At
> this time, in absence of linker support for BTF sections, it makes sense to
> steer clear of LTO for BPF CO-RE and bypass the issue.
>
> 2. Use a target hook to allow BPF backend to cleanly convey the case when late
> finalization of the CTF container is desirable.
>
> So, in other words,
>
> dwarf2out_early_finish
> - Always emit CTF here.
> - if (BTF && ctfc_debuginfo_early_finish_p), emit BTF now.
>
> dwarf2out_finish
> - if (BTF && !ctfc_debuginfo_early_finish_p && !in_lto_p) emit BTF now.
> - Use of in_lto_p to make sure LTO link phase does not affect BTF sections
> for other targets.
>
> gcc/ChangeLog:
>
> * dwarf2ctf.c (ctf_debug_finalize): Make it static.
> (ctf_debug_early_finish): New definition.
> (ctf_debug_finish): Likewise.
> * dwarf2ctf.h (ctf_debug_finalize): Remove declaration.
> (ctf_debug_early_finish): New declaration.
> (ctf_debug_finish): Likewise.
> * dwarf2out.c (dwarf2out_finish): Invoke ctf_debug_finish.
> (dwarf2out_early_finish): Invoke ctf_debug_early_finish.
> ---
> gcc/dwarf2ctf.c | 55 +++++++++++++++++++++++++++++++++++++++++++------------
> gcc/dwarf2ctf.h | 4 +++-
> gcc/dwarf2out.c | 9 +++++++--
> 3 files changed, 53 insertions(+), 15 deletions(-)
>
> diff --git a/gcc/dwarf2ctf.c b/gcc/dwarf2ctf.c
> index 5e8a725..0fa429c 100644
> --- a/gcc/dwarf2ctf.c
> +++ b/gcc/dwarf2ctf.c
> @@ -917,6 +917,27 @@ gen_ctf_type (ctf_container_ref ctfc, dw_die_ref die)
> return type_id;
> }
>
> +/* Prepare for output and write out the CTF debug information. */
> +
> +static void
> +ctf_debug_finalize (const char *filename, bool btf)
> +{
> + if (btf)
> + {
> + btf_output (filename);
> + btf_finalize ();
> + }
> +
> + else
> + {
> + /* Emit the collected CTF information. */
> + ctf_output (filename);
> +
> + /* Reset the CTF state. */
> + ctf_finalize ();
> + }
> +}
> +
> bool
> ctf_do_die (dw_die_ref die)
> {
> @@ -966,25 +987,35 @@ ctf_debug_init_postprocess (bool btf)
> btf_init_postprocess ();
> }
>
> -/* Prepare for output and write out the CTF debug information. */
> +/* Early finish CTF/BTF debug info. */
>
> void
> -ctf_debug_finalize (const char *filename, bool btf)
> +ctf_debug_early_finish (const char * filename)
> {
> - if (btf)
> + /* Emit CTF debug info early always. */
> + if (ctf_debug_info_level > CTFINFO_LEVEL_NONE
> + /* Emit BTF debug info early if the target does not require late
> + emission. */
> + || (btf_debuginfo_p ()
> + && targetm.ctfc_debuginfo_early_finish_p ()))
> {
> - btf_output (filename);
> - btf_finalize ();
> + /* Emit CTF/BTF debug info. */
> + ctf_debug_finalize (filename, btf_debuginfo_p ());
> }
> +}
>
> - else
> - {
> - /* Emit the collected CTF information. */
> - ctf_output (filename);
> +/* Finish CTF/BTF debug info emission. */
>
> - /* Reset the CTF state. */
> - ctf_finalize ();
> - }
> +void
> +ctf_debug_finish (const char * filename)
> +{
> + /* Emit BTF debug info here when the target needs to update the CTF container
> + (ctfc) in the backend. An example of this, at this time is the BPF CO-RE
> + usecase. */
> + if (btf_debuginfo_p ()
> + && (!in_lto_p && !targetm.ctfc_debuginfo_early_finish_p ()))
> + /* Emit BTF debug info. */
> + ctf_debug_finalize (filename, btf_debuginfo_p ());
> }
>
> #include "gt-dwarf2ctf.h"
> diff --git a/gcc/dwarf2ctf.h b/gcc/dwarf2ctf.h
> index a3cf567..9edbde0 100644
> --- a/gcc/dwarf2ctf.h
> +++ b/gcc/dwarf2ctf.h
> @@ -24,13 +24,15 @@ along with GCC; see the file COPYING3. If not see
> #define GCC_DWARF2CTF_H 1
>
> #include "dwarf2out.h"
> +#include "flags.h"
>
> /* Debug Format Interface. Used in dwarf2out.c. */
>
> extern void ctf_debug_init (void);
> extern void ctf_debug_init_postprocess (bool);
> extern bool ctf_do_die (dw_die_ref);
> -extern void ctf_debug_finalize (const char *, bool);
> +extern void ctf_debug_early_finish (const char *);
> +extern void ctf_debug_finish (const char *);
>
> /* Wrappers for CTF/BTF to fetch information from GCC DWARF DIE. Used in
> ctfc.c.
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index b91a9b5..708cd1f 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -31895,6 +31895,11 @@ dwarf2out_finish (const char *filename)
> unsigned char checksum[16];
> char dl_section_ref[MAX_ARTIFICIAL_LABEL_BYTES];
>
> + /* Generate CTF/BTF debug info. */
> + if ((ctf_debug_info_level > CTFINFO_LEVEL_NONE
> + || btf_debuginfo_p ()) && lang_GNU_C ())
> + ctf_debug_finish (filename);
> +
> /* Skip emitting DWARF if not required. */
> if (!dwarf_debuginfo_p ())
> return;
> @@ -32799,8 +32804,8 @@ dwarf2out_early_finish (const char *filename)
> ctf_debug_do_cu (node->die);
> /* Post process the debug data in the CTF container if necessary. */
> ctf_debug_init_postprocess (btf_debuginfo_p ());
> - /* Emit CTF/BTF debug info. */
> - ctf_debug_finalize (filename, btf_debuginfo_p ());
> +
> + ctf_debug_early_finish (filename);
> }
>
> /* Do not generate DWARF assembler now when not producing LTO bytecode. */
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH,V2 1/3] bpf: Add new -mcore option for BPF CO-RE
2021-08-10 11:51 ` Richard Biener
@ 2021-08-10 15:45 ` Jose E. Marchesi
2021-08-26 10:05 ` Richard Biener
0 siblings, 1 reply; 23+ messages in thread
From: Jose E. Marchesi @ 2021-08-10 15:45 UTC (permalink / raw)
To: Richard Biener via Gcc-patches; +Cc: Indu Bhagat, Richard Biener
> On Thu, Aug 5, 2021 at 2:54 AM Indu Bhagat via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> -mcore in the BPF backend enables code generation for the CO-RE usecase. LTO is
>> disabled for CO-RE compilations.
>
> -mcore reads like "core", why not -mco-re? Anyway, ...
>
>> gcc/ChangeLog:
>>
>> * config/bpf/bpf.c (bpf_option_override): For BPF backend, disable LTO
>> support when compiling for CO-RE.
>> * config/bpf/bpf.opt: Add new command line option -mcore.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/bpf/core-lto-1.c: New test.
>> ---
>> gcc/config/bpf/bpf.c | 15 +++++++++++++++
>> gcc/config/bpf/bpf.opt | 4 ++++
>> gcc/testsuite/gcc.target/bpf/core-lto-1.c | 9 +++++++++
>> 3 files changed, 28 insertions(+)
>> create mode 100644 gcc/testsuite/gcc.target/bpf/core-lto-1.c
>>
>> diff --git a/gcc/config/bpf/bpf.c b/gcc/config/bpf/bpf.c
>> index e635f9e..028013e 100644
>> --- a/gcc/config/bpf/bpf.c
>> +++ b/gcc/config/bpf/bpf.c
>> @@ -158,6 +158,21 @@ bpf_option_override (void)
>> {
>> /* Set the initializer for the per-function status structure. */
>> init_machine_status = bpf_init_machine_status;
>> +
>> + /* To support the portability needs of BPF CO-RE approach, BTF debug
>> + information includes the BPF CO-RE relocations. The information
>> + necessary for these relocations is added to the CTF container by the
>> + BPF backend. Enabling LTO poses challenges in the generation of the BPF
>> + CO-RE relocations because if LTO is in effect, they need to be
>> + generated late in the LTO link phase. This in turn means the compiler
>> + needs to provide means to combine the early and late BTF debug info,
>> + similar to DWARF debug info.
>> +
>> + In any case, in absence of linker support for BTF sections at this time,
>> + it is acceptable to simply disallow LTO for BPF CO-RE compilations. */
>> +
>> + if (flag_lto && TARGET_BPF_CORE)
>> + error ("BPF CO-RE does not support LTO");
>
> ... these "errors" should use sorry ("...") which annotate places where the
> compiler has to give up because sth is not implemented.
>
> otherwise this patch needs BPF maintainer review of course.
I agree -mco-re is more clear than -mcore.
Other than that this looks OK BPF-wise.
>> }
>>
>> #undef TARGET_OPTION_OVERRIDE
>> diff --git a/gcc/config/bpf/bpf.opt b/gcc/config/bpf/bpf.opt
>> index 916b53c..e8926f5 100644
>> --- a/gcc/config/bpf/bpf.opt
>> +++ b/gcc/config/bpf/bpf.opt
>> @@ -127,3 +127,7 @@ Generate little-endian eBPF.
>> mframe-limit=
>> Target Joined RejectNegative UInteger IntegerRange(0, 32767) Var(bpf_frame_limit) Init(512)
>> Set a hard limit for the size of each stack frame, in bytes.
>> +
>> +mcore
>> +Target Mask(BPF_CORE)
>> +Generate all necessary information for BPF Compile Once - Run Everywhere.
>> diff --git a/gcc/testsuite/gcc.target/bpf/core-lto-1.c
>> b/gcc/testsuite/gcc.target/bpf/core-lto-1.c
>> new file mode 100644
>> index 0000000..a90dc5b
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/bpf/core-lto-1.c
>> @@ -0,0 +1,9 @@
>> +/* Test -mcore with -flto.
>> +
>> + -mcore is used to generate information for BPF CO-RE usecase. To support
>> + the generataion of the .BTF and .BTF.ext sections in GCC, -flto is disabled
>> + with -mcore. */
>> +
>> +/* { dg-do compile } */
>> +/* { dg-error "BPF CO-RE does not support LTO" "" { target bpf-*-* } 0 } */
>> +/* { dg-options "-gbtf -mcore -flto" } */
>> --
>> 1.8.3.1
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 3/3] dwarf2out: Emit BTF in dwarf2out_finish for BPF CO-RE usecase
2021-08-10 12:00 ` Richard Biener
@ 2021-08-10 16:39 ` David Faust
0 siblings, 0 replies; 23+ messages in thread
From: David Faust @ 2021-08-10 16:39 UTC (permalink / raw)
To: Richard Biener, Indu Bhagat; +Cc: GCC Patches
On 8/10/21 5:00 AM, Richard Biener via Gcc-patches wrote:
> On Thu, Aug 5, 2021 at 2:53 AM Indu Bhagat via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> DWARF generation is split between early and late phases when LTO is in effect.
>> This poses challenges for CTF/BTF generation especially if late debug info
>> generation is desirable, as turns out to be the case for BPF CO-RE.
>>
>> In case of BPF CO-RE, the BPF backend adds information about CO-RE relocations
>> to the CTF container. This information is what needs to be emitted as a
>> separate .BTF.ext section when -more is in effect. Further, each CO-RE
>> relocation record holds an offset to a string specifying the access to the
>> structure's field. This means that .BTF string table needs to be modified
>> "late" in the compilation process. In other words, this implies that the BTF
>> sections cannot be finalized in dwarf2out_early_finish when -mcore for the BPF
>> backend is in effect.
>
> OK, it didn't really get any clearer as to why the late annotation
> cannot be done
> after the early info was output. Something to do with the BTF string table,
> but all structure field names must be already present there so I must be missing
> something.
>
> ISTR the CO-RE info is fully contained in a new section .BTF.ext and the
> "early" .BTF section is not altered?
This is unfortunately not quite the case. The CO-RE relocation records
themselves are placed in the .BTF.ext section. But a key component of
each record is an "access string," a string which encodes the accessed
field via a sequence of field or array indices.
The .BTF.ext section does not have a string table of its own, so these
"access strings" are placed in the .BTF section string table. The CO-RE
relocation records refer to them by offset into the .BTF string table.
As a result, the CO-RE information spills into the regular .BTF section,
and we need to delay writing out the .BTF section until after these
strings can be added.
Personally I don't think this way of splitting the CO-RE information is
ideal, but hopefully now the issue is more clear?
Thank you for your reviews!
Example of access string encoding:
struct S {
int a;
union {
int _unused;
int b;
char c;
} u[4];
};
struct S *foo = ...;
int x = foo->a; /* encoded as "0:0" */
int y = foo[4]->u[2].b /* encoded as "4:1:2:1" */
char z = foo->u[3].c /* encoded as "0:1:3:2" */
>
>> Now, the emission of CTF/BTF debug info cannot be moved unconditionally to
>> dwarf2out_finish because dwarf2out_finish is not invoked at all for the LTO
>> compile phase for slim LTO objects, thus breaking CTF/BTF generation for other
>> targets when used with LTO.
>>
>> The approach taken here in this patch is that -
>>
>> 1. LTO is disabled for BPF CO-RE
>> The reason to disable LTO for BPF CO-RE is that if LTO is in effect, BPF CO-RE
>> relocations need to be generated in the LTO link phase _after_ the optimizations
>> are done. This means we need to devise way to combine early and late BTF. At
>> this time, in absence of linker support for BTF sections, it makes sense to
>> steer clear of LTO for BPF CO-RE and bypass the issue.
>>
>> 2. Use a target hook to allow BPF backend to cleanly convey the case when late
>> finalization of the CTF container is desirable.
>>
>> So, in other words,
>>
>> dwarf2out_early_finish
>> - Always emit CTF here.
>> - if (BTF && ctfc_debuginfo_early_finish_p), emit BTF now.
>>
>> dwarf2out_finish
>> - if (BTF && !ctfc_debuginfo_early_finish_p && !in_lto_p) emit BTF now.
>> - Use of in_lto_p to make sure LTO link phase does not affect BTF sections
>> for other targets.
>>
>> gcc/ChangeLog:
>>
>> * dwarf2ctf.c (ctf_debug_finalize): Make it static.
>> (ctf_debug_early_finish): New definition.
>> (ctf_debug_finish): Likewise.
>> * dwarf2ctf.h (ctf_debug_finalize): Remove declaration.
>> (ctf_debug_early_finish): New declaration.
>> (ctf_debug_finish): Likewise.
>> * dwarf2out.c (dwarf2out_finish): Invoke ctf_debug_finish.
>> (dwarf2out_early_finish): Invoke ctf_debug_early_finish.
>> ---
>> gcc/dwarf2ctf.c | 55 +++++++++++++++++++++++++++++++++++++++++++------------
>> gcc/dwarf2ctf.h | 4 +++-
>> gcc/dwarf2out.c | 9 +++++++--
>> 3 files changed, 53 insertions(+), 15 deletions(-)
>>
>> diff --git a/gcc/dwarf2ctf.c b/gcc/dwarf2ctf.c
>> index 5e8a725..0fa429c 100644
>> --- a/gcc/dwarf2ctf.c
>> +++ b/gcc/dwarf2ctf.c
>> @@ -917,6 +917,27 @@ gen_ctf_type (ctf_container_ref ctfc, dw_die_ref die)
>> return type_id;
>> }
>>
>> +/* Prepare for output and write out the CTF debug information. */
>> +
>> +static void
>> +ctf_debug_finalize (const char *filename, bool btf)
>> +{
>> + if (btf)
>> + {
>> + btf_output (filename);
>> + btf_finalize ();
>> + }
>> +
>> + else
>> + {
>> + /* Emit the collected CTF information. */
>> + ctf_output (filename);
>> +
>> + /* Reset the CTF state. */
>> + ctf_finalize ();
>> + }
>> +}
>> +
>> bool
>> ctf_do_die (dw_die_ref die)
>> {
>> @@ -966,25 +987,35 @@ ctf_debug_init_postprocess (bool btf)
>> btf_init_postprocess ();
>> }
>>
>> -/* Prepare for output and write out the CTF debug information. */
>> +/* Early finish CTF/BTF debug info. */
>>
>> void
>> -ctf_debug_finalize (const char *filename, bool btf)
>> +ctf_debug_early_finish (const char * filename)
>> {
>> - if (btf)
>> + /* Emit CTF debug info early always. */
>> + if (ctf_debug_info_level > CTFINFO_LEVEL_NONE
>> + /* Emit BTF debug info early if the target does not require late
>> + emission. */
>> + || (btf_debuginfo_p ()
>> + && targetm.ctfc_debuginfo_early_finish_p ()))
>> {
>> - btf_output (filename);
>> - btf_finalize ();
>> + /* Emit CTF/BTF debug info. */
>> + ctf_debug_finalize (filename, btf_debuginfo_p ());
>> }
>> +}
>>
>> - else
>> - {
>> - /* Emit the collected CTF information. */
>> - ctf_output (filename);
>> +/* Finish CTF/BTF debug info emission. */
>>
>> - /* Reset the CTF state. */
>> - ctf_finalize ();
>> - }
>> +void
>> +ctf_debug_finish (const char * filename)
>> +{
>> + /* Emit BTF debug info here when the target needs to update the CTF container
>> + (ctfc) in the backend. An example of this, at this time is the BPF CO-RE
>> + usecase. */
>> + if (btf_debuginfo_p ()
>> + && (!in_lto_p && !targetm.ctfc_debuginfo_early_finish_p ()))
>> + /* Emit BTF debug info. */
>> + ctf_debug_finalize (filename, btf_debuginfo_p ());
>> }
>>
>> #include "gt-dwarf2ctf.h"
>> diff --git a/gcc/dwarf2ctf.h b/gcc/dwarf2ctf.h
>> index a3cf567..9edbde0 100644
>> --- a/gcc/dwarf2ctf.h
>> +++ b/gcc/dwarf2ctf.h
>> @@ -24,13 +24,15 @@ along with GCC; see the file COPYING3. If not see
>> #define GCC_DWARF2CTF_H 1
>>
>> #include "dwarf2out.h"
>> +#include "flags.h"
>>
>> /* Debug Format Interface. Used in dwarf2out.c. */
>>
>> extern void ctf_debug_init (void);
>> extern void ctf_debug_init_postprocess (bool);
>> extern bool ctf_do_die (dw_die_ref);
>> -extern void ctf_debug_finalize (const char *, bool);
>> +extern void ctf_debug_early_finish (const char *);
>> +extern void ctf_debug_finish (const char *);
>>
>> /* Wrappers for CTF/BTF to fetch information from GCC DWARF DIE. Used in
>> ctfc.c.
>> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
>> index b91a9b5..708cd1f 100644
>> --- a/gcc/dwarf2out.c
>> +++ b/gcc/dwarf2out.c
>> @@ -31895,6 +31895,11 @@ dwarf2out_finish (const char *filename)
>> unsigned char checksum[16];
>> char dl_section_ref[MAX_ARTIFICIAL_LABEL_BYTES];
>>
>> + /* Generate CTF/BTF debug info. */
>> + if ((ctf_debug_info_level > CTFINFO_LEVEL_NONE
>> + || btf_debuginfo_p ()) && lang_GNU_C ())
>> + ctf_debug_finish (filename);
>> +
>> /* Skip emitting DWARF if not required. */
>> if (!dwarf_debuginfo_p ())
>> return;
>> @@ -32799,8 +32804,8 @@ dwarf2out_early_finish (const char *filename)
>> ctf_debug_do_cu (node->die);
>> /* Post process the debug data in the CTF container if necessary. */
>> ctf_debug_init_postprocess (btf_debuginfo_p ());
>> - /* Emit CTF/BTF debug info. */
>> - ctf_debug_finalize (filename, btf_debuginfo_p ());
>> +
>> + ctf_debug_early_finish (filename);
>> }
>>
>> /* Do not generate DWARF assembler now when not producing LTO bytecode. */
>> --
>> 1.8.3.1
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-10 11:54 ` Richard Biener
@ 2021-08-16 17:39 ` Indu Bhagat
2021-08-17 8:04 ` Richard Biener
0 siblings, 1 reply; 23+ messages in thread
From: Indu Bhagat @ 2021-08-16 17:39 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
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
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-16 17:39 ` Indu Bhagat
@ 2021-08-17 8:04 ` Richard Biener
2021-08-17 17:26 ` Indu Bhagat
0 siblings, 1 reply; 23+ messages in thread
From: Richard Biener @ 2021-08-17 8:04 UTC (permalink / raw)
To: Indu Bhagat; +Cc: GCC Patches
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
> >>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-17 8:04 ` Richard Biener
@ 2021-08-17 17:26 ` Indu Bhagat
2021-08-18 7:00 ` Richard Biener
0 siblings, 1 reply; 23+ messages in thread
From: Indu Bhagat @ 2021-08-17 17:26 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, David Faust
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
>>>>
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-17 17:26 ` Indu Bhagat
@ 2021-08-18 7:00 ` Richard Biener
2021-08-18 14:20 ` Indu Bhagat
` (3 more replies)
0 siblings, 4 replies; 23+ messages in thread
From: Richard Biener @ 2021-08-18 7:00 UTC (permalink / raw)
To: Indu Bhagat; +Cc: GCC Patches, David Faust
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
> >>>>
> >>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-18 7:00 ` Richard Biener
@ 2021-08-18 14:20 ` Indu Bhagat
2021-08-19 14:41 ` Jose E. Marchesi
` (2 subsequent siblings)
3 siblings, 0 replies; 23+ messages in thread
From: Indu Bhagat @ 2021-08-18 14:20 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, David Faust
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
>>>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-18 7:00 ` Richard Biener
2021-08-18 14:20 ` Indu Bhagat
@ 2021-08-19 14:41 ` Jose E. Marchesi
2021-08-19 15:10 ` Jose E. Marchesi
2021-08-24 17:06 ` Indu Bhagat
3 siblings, 0 replies; 23+ messages in thread
From: Jose E. Marchesi @ 2021-08-19 14:41 UTC (permalink / raw)
To: Richard Biener via Gcc-patches; +Cc: Indu Bhagat, Richard Biener
> 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 ;)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-18 7:00 ` Richard Biener
2021-08-18 14:20 ` Indu Bhagat
2021-08-19 14:41 ` Jose E. Marchesi
@ 2021-08-19 15:10 ` Jose E. Marchesi
2021-08-24 17:06 ` Indu Bhagat
3 siblings, 0 replies; 23+ messages in thread
From: Jose E. Marchesi @ 2021-08-19 15:10 UTC (permalink / raw)
To: Richard Biener via Gcc-patches; +Cc: Indu Bhagat, Richard Biener
> 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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-18 7:00 ` Richard Biener
` (2 preceding siblings ...)
2021-08-19 15:10 ` Jose E. Marchesi
@ 2021-08-24 17:06 ` Indu Bhagat
2021-08-26 10:03 ` Richard Biener
3 siblings, 1 reply; 23+ messages in thread
From: Indu Bhagat @ 2021-08-24 17:06 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, David Faust
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-24 17:06 ` Indu Bhagat
@ 2021-08-26 10:03 ` Richard Biener
2021-08-26 18:55 ` Indu Bhagat
0 siblings, 1 reply; 23+ messages in thread
From: Richard Biener @ 2021-08-26 10:03 UTC (permalink / raw)
To: Indu Bhagat; +Cc: GCC Patches, David Faust
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH,V2 1/3] bpf: Add new -mcore option for BPF CO-RE
2021-08-10 15:45 ` Jose E. Marchesi
@ 2021-08-26 10:05 ` Richard Biener
2021-08-26 13:27 ` Jose E. Marchesi
0 siblings, 1 reply; 23+ messages in thread
From: Richard Biener @ 2021-08-26 10:05 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: Richard Biener via Gcc-patches, Indu Bhagat
On Tue, Aug 10, 2021 at 5:45 PM Jose E. Marchesi
<jose.marchesi@oracle.com> wrote:
>
>
> > On Thu, Aug 5, 2021 at 2:54 AM Indu Bhagat via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> -mcore in the BPF backend enables code generation for the CO-RE usecase. LTO is
> >> disabled for CO-RE compilations.
> >
> > -mcore reads like "core", why not -mco-re? Anyway, ...
> >
> >> gcc/ChangeLog:
> >>
> >> * config/bpf/bpf.c (bpf_option_override): For BPF backend, disable LTO
> >> support when compiling for CO-RE.
> >> * config/bpf/bpf.opt: Add new command line option -mcore.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> * gcc.target/bpf/core-lto-1.c: New test.
> >> ---
> >> gcc/config/bpf/bpf.c | 15 +++++++++++++++
> >> gcc/config/bpf/bpf.opt | 4 ++++
> >> gcc/testsuite/gcc.target/bpf/core-lto-1.c | 9 +++++++++
> >> 3 files changed, 28 insertions(+)
> >> create mode 100644 gcc/testsuite/gcc.target/bpf/core-lto-1.c
> >>
> >> diff --git a/gcc/config/bpf/bpf.c b/gcc/config/bpf/bpf.c
> >> index e635f9e..028013e 100644
> >> --- a/gcc/config/bpf/bpf.c
> >> +++ b/gcc/config/bpf/bpf.c
> >> @@ -158,6 +158,21 @@ bpf_option_override (void)
> >> {
> >> /* Set the initializer for the per-function status structure. */
> >> init_machine_status = bpf_init_machine_status;
> >> +
> >> + /* To support the portability needs of BPF CO-RE approach, BTF debug
> >> + information includes the BPF CO-RE relocations. The information
> >> + necessary for these relocations is added to the CTF container by the
> >> + BPF backend. Enabling LTO poses challenges in the generation of the BPF
> >> + CO-RE relocations because if LTO is in effect, they need to be
> >> + generated late in the LTO link phase. This in turn means the compiler
> >> + needs to provide means to combine the early and late BTF debug info,
> >> + similar to DWARF debug info.
> >> +
> >> + In any case, in absence of linker support for BTF sections at this time,
> >> + it is acceptable to simply disallow LTO for BPF CO-RE compilations. */
> >> +
> >> + if (flag_lto && TARGET_BPF_CORE)
> >> + error ("BPF CO-RE does not support LTO");
> >
> > ... these "errors" should use sorry ("...") which annotate places where the
> > compiler has to give up because sth is not implemented.
> >
> > otherwise this patch needs BPF maintainer review of course.
>
> I agree -mco-re is more clear than -mcore.
>
> Other than that this looks OK BPF-wise.
So in other context I wonder if CO-RE is really target specific, it's a flavor
of the BTF debug format, so shouldn't it be -gbtf-core or sth like that?
Richard.
> >> }
> >>
> >> #undef TARGET_OPTION_OVERRIDE
> >> diff --git a/gcc/config/bpf/bpf.opt b/gcc/config/bpf/bpf.opt
> >> index 916b53c..e8926f5 100644
> >> --- a/gcc/config/bpf/bpf.opt
> >> +++ b/gcc/config/bpf/bpf.opt
> >> @@ -127,3 +127,7 @@ Generate little-endian eBPF.
> >> mframe-limit=
> >> Target Joined RejectNegative UInteger IntegerRange(0, 32767) Var(bpf_frame_limit) Init(512)
> >> Set a hard limit for the size of each stack frame, in bytes.
> >> +
> >> +mcore
> >> +Target Mask(BPF_CORE)
> >> +Generate all necessary information for BPF Compile Once - Run Everywhere.
> >> diff --git a/gcc/testsuite/gcc.target/bpf/core-lto-1.c
> >> b/gcc/testsuite/gcc.target/bpf/core-lto-1.c
> >> new file mode 100644
> >> index 0000000..a90dc5b
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.target/bpf/core-lto-1.c
> >> @@ -0,0 +1,9 @@
> >> +/* Test -mcore with -flto.
> >> +
> >> + -mcore is used to generate information for BPF CO-RE usecase. To support
> >> + the generataion of the .BTF and .BTF.ext sections in GCC, -flto is disabled
> >> + with -mcore. */
> >> +
> >> +/* { dg-do compile } */
> >> +/* { dg-error "BPF CO-RE does not support LTO" "" { target bpf-*-* } 0 } */
> >> +/* { dg-options "-gbtf -mcore -flto" } */
> >> --
> >> 1.8.3.1
> >>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH,V2 1/3] bpf: Add new -mcore option for BPF CO-RE
2021-08-26 10:05 ` Richard Biener
@ 2021-08-26 13:27 ` Jose E. Marchesi
0 siblings, 0 replies; 23+ messages in thread
From: Jose E. Marchesi @ 2021-08-26 13:27 UTC (permalink / raw)
To: Richard Biener; +Cc: Richard Biener via Gcc-patches, Indu Bhagat
Hi Richard.
> On Tue, Aug 10, 2021 at 5:45 PM Jose E. Marchesi
> <jose.marchesi@oracle.com> wrote:
>>
>>
>> > On Thu, Aug 5, 2021 at 2:54 AM Indu Bhagat via Gcc-patches
>> > <gcc-patches@gcc.gnu.org> wrote:
>> >>
>> >> -mcore in the BPF backend enables code generation for the CO-RE usecase. LTO is
>> >> disabled for CO-RE compilations.
>> >
>> > -mcore reads like "core", why not -mco-re? Anyway, ...
>> >
>> >> gcc/ChangeLog:
>> >>
>> >> * config/bpf/bpf.c (bpf_option_override): For BPF backend, disable LTO
>> >> support when compiling for CO-RE.
>> >> * config/bpf/bpf.opt: Add new command line option -mcore.
>> >>
>> >> gcc/testsuite/ChangeLog:
>> >>
>> >> * gcc.target/bpf/core-lto-1.c: New test.
>> >> ---
>> >> gcc/config/bpf/bpf.c | 15 +++++++++++++++
>> >> gcc/config/bpf/bpf.opt | 4 ++++
>> >> gcc/testsuite/gcc.target/bpf/core-lto-1.c | 9 +++++++++
>> >> 3 files changed, 28 insertions(+)
>> >> create mode 100644 gcc/testsuite/gcc.target/bpf/core-lto-1.c
>> >>
>> >> diff --git a/gcc/config/bpf/bpf.c b/gcc/config/bpf/bpf.c
>> >> index e635f9e..028013e 100644
>> >> --- a/gcc/config/bpf/bpf.c
>> >> +++ b/gcc/config/bpf/bpf.c
>> >> @@ -158,6 +158,21 @@ bpf_option_override (void)
>> >> {
>> >> /* Set the initializer for the per-function status structure. */
>> >> init_machine_status = bpf_init_machine_status;
>> >> +
>> >> + /* To support the portability needs of BPF CO-RE approach, BTF debug
>> >> + information includes the BPF CO-RE relocations. The information
>> >> + necessary for these relocations is added to the CTF container by the
>> >> + BPF backend. Enabling LTO poses challenges in the generation of the BPF
>> >> + CO-RE relocations because if LTO is in effect, they need to be
>> >> + generated late in the LTO link phase. This in turn means the compiler
>> >> + needs to provide means to combine the early and late BTF debug info,
>> >> + similar to DWARF debug info.
>> >> +
>> >> + In any case, in absence of linker support for BTF sections at this time,
>> >> + it is acceptable to simply disallow LTO for BPF CO-RE compilations. */
>> >> +
>> >> + if (flag_lto && TARGET_BPF_CORE)
>> >> + error ("BPF CO-RE does not support LTO");
>> >
>> > ... these "errors" should use sorry ("...") which annotate places where the
>> > compiler has to give up because sth is not implemented.
>> >
>> > otherwise this patch needs BPF maintainer review of course.
>>
>> I agree -mco-re is more clear than -mcore.
>>
>> Other than that this looks OK BPF-wise.
>
> So in other context I wonder if CO-RE is really target specific, it's a flavor
> of the BTF debug format, so shouldn't it be -gbtf-core or sth like that?
I wouldn't say CO-RE is a flavor of BTF: it makes use of the BTF string
table (an unfortunate design decision) and the relocations are stored in
a section called .BTF.ext (which is confusing) but if anything it would
be... an extension?
I would say it is very target-specific anyway, since it requires the BPF
backend to generate particular sequences of instructions, and the BPF
"hardware" (kernel loader and verifier) to do the right thing with the
generated relocations. The whole thing is very tightly related to BPF
and the kernel data structures.
I would certainly not recommend anyone to implement CO-RE for other
targets/architectures...
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-26 10:03 ` Richard Biener
@ 2021-08-26 18:55 ` Indu Bhagat
2021-08-27 6:12 ` Richard Biener
0 siblings, 1 reply; 23+ messages in thread
From: Indu Bhagat @ 2021-08-26 18:55 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, David Faust
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-26 18:55 ` Indu Bhagat
@ 2021-08-27 6:12 ` Richard Biener
2021-09-02 19:21 ` Indu Bhagat
0 siblings, 1 reply; 23+ messages in thread
From: Richard Biener @ 2021-08-27 6:12 UTC (permalink / raw)
To: Indu Bhagat; +Cc: GCC Patches, David Faust
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission
2021-08-27 6:12 ` Richard Biener
@ 2021-09-02 19:21 ` Indu Bhagat
0 siblings, 0 replies; 23+ messages in thread
From: Indu Bhagat @ 2021-09-02 19:21 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, David Faust
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
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2021-09-02 19:21 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-05 0:50 [PATCH,V2 0/3] Allow means for late BTF generation for BPF CO-RE Indu Bhagat
2021-08-05 0:50 ` [PATCH,V2 1/3] bpf: Add new -mcore option " Indu Bhagat
2021-08-10 11:51 ` Richard Biener
2021-08-10 15:45 ` Jose E. Marchesi
2021-08-26 10:05 ` Richard Biener
2021-08-26 13:27 ` Jose E. Marchesi
2021-08-05 0:50 ` [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission Indu Bhagat
2021-08-10 11:54 ` Richard Biener
2021-08-16 17:39 ` Indu Bhagat
2021-08-17 8:04 ` Richard Biener
2021-08-17 17:26 ` Indu Bhagat
2021-08-18 7:00 ` Richard Biener
2021-08-18 14:20 ` Indu Bhagat
2021-08-19 14:41 ` Jose E. Marchesi
2021-08-19 15:10 ` Jose E. Marchesi
2021-08-24 17:06 ` Indu Bhagat
2021-08-26 10:03 ` Richard Biener
2021-08-26 18:55 ` Indu Bhagat
2021-08-27 6:12 ` Richard Biener
2021-09-02 19:21 ` Indu Bhagat
2021-08-05 0:50 ` [PATCH, V2 3/3] dwarf2out: Emit BTF in dwarf2out_finish for BPF CO-RE usecase Indu Bhagat
2021-08-10 12:00 ` Richard Biener
2021-08-10 16:39 ` David Faust
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).