Hi Richard.

> On Tue, Aug 10, 2021 at 5:45 PM Jose E. Marchesi
> <jose.march...@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...

Reply via email to