Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

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

> On Thu, Apr 25, 2024 at 5:32 AM Jose E. Marchesi
> <jose.marchesi@oracle.com> wrote:
>>
>>
>> > On Wed, Apr 24, 2024 at 2:30 PM Jose E. Marchesi
>> > <jose.marchesi@oracle.com> wrote:
>> >>
>> >>
>> >> Hi Yonghong.
>> >>
>> >> > On 4/24/24 1:41 AM, Jose E. Marchesi wrote:
>> >> >> This little patch modifies selftests/bpf/Makefile so it passes the
>> >> >> following extra options when invoking gcc-bpf:
>> >> >>
>> >> >>   -gbtf
>> >> >>     This makes GCC to emit BTF debug info in .BTF and .BTF.ext.
>> >> >
>> >> > Could we do if '-g' is specified, for bpf program,
>> >> > btf will be automatically generated?
>> >>
>> >> Hmm, in principle I wouldn't oppose for -g to mean -gbtf instead of
>> >> -gdwarf.  DWARF can always be generated by using -gdwarf.
>> >>
>> >> Faust, Indu, WDYT?
>> >>
>> >> >>
>> >> >>   -mco-re
>> >> >>     This tells GCC to generate CO-RE relocations in .BTF.ext.
>> >> >
>> >> > Can we make this default? That is, remove -mco-re option. I
>> >> > can imagine for any serious bpf program, co-re is a must.
>> >>
>> >> CO-RE depends on BTF.  So I understand the above as making -mco-re the
>> >> default if BTF is generated, i.e. if -gbtf (or -g with the modification
>> >> above) are specified.  Isn't that what clang does?  Am I interpreting
>> >> correctly?
>> >>
>> >> >>
>> >> >>   -masm=pseudoc
>> >> >>     This tells GCC to emit BPF assembler using the pseudo-c syntax.
>> >> >
>> >> > Can we make it the other way round such that -masm=pseudoc is
>> >> > the default? You can have an option e.g., -masm=non-pseudoc,
>> >> > for the other format?
>> >>
>> >> We could add a configure-time build option:
>> >>
>> >>   --with-bpf-default-asm-syntax={pseudoc,normal}
>> >>
>> >> so that GCC can be built to use whatever selected syntax as default.
>> >> Distros and people can then decide what to do.
>> >
>> > distros just ship stuff.
>> > It's our job to pick good defaults.
>>
>> Yeah it was a rather dumb idea that would only complicate things for no
>> good reason.
>>
>> The unfortunate fact is that at this point the kernel headers that
>> almost all BPF programs use contain pseudo-C inline assembly and having
>> the toolchain using the conventional assembly syntax by default would
>> force users to specify the command-line option explicitly, which is a
>> great PITA.  So I guess this is one of these situations where the worse
>> option is indeed the best default, in practical terms.
>>
>> So ok, as much as it sucks we will make -masm=pseudoc the default in GCC
>> for the sake of practicality.
>>
>> > I agree with Yonghong that -g should imply -gbtf for bpf target
>> > and -mco-re doesn't need to be a flag at all.
>>
>> We like the idea of -g implying -gbtf rather than -gdwarf for the BPF
>> target.  It makes sense.  Faust is already working on it.
>>
>> As for -mco-re, it is already the default with -gbtf, and now it will be
>> the default for -g.
>>
>> > Compiler should do it when it sees those special attributes in C code.
>> > -masm=pseudoc is a good default as well, since that's what
>> > everyone in bpf community is used to.
>>
>> We will try to get all the changes above upstream before GCC 14 gets
>> branched, which shall happen any day now.  Once they are in GCC the only
>> extra option to be added to GCC_BPF_BUILD_RULE will be -g.  Will send an
>> updated patch then.
>
> Awesome. This is all great to hear.

The GCC 14 release branch was created today, but we managed to get the
changes for -g and default to pseudo-C just in time.