Re: [PATCH,V2 1/3] bpf: Add new -mcore option for BPF CO-RE

["Jose E. Marchesi" <jose.marchesi@oracle.com>]

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