RE: [PATCH] perf: use __builtin_preserve_field_info for GCC compatibility

[Andrew Pinski]

> -----Original Message-----
> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Sent: Thursday, October 2, 2025 10:32 AM
> To: Yonghong Song <yonghong.song@xxxxxxxxx>
> Cc: Andrew Pinski (QUIC) <quic_apinski@xxxxxxxxxxx>;
> Namhyung Kim <namhyung@xxxxxxxxxx>; Sam James
> <sam@xxxxxxxxxx>; Peter Zijlstra <peterz@xxxxxxxxxxxxx>;
> Ingo Molnar <mingo@xxxxxxxxxx>; Mark Rutland
> <mark.rutland@xxxxxxx>; Alexander Shishkin
> <alexander.shishkin@xxxxxxxxxxxxxxx>; Jiri Olsa
> <jolsa@xxxxxxxxxx>; Ian Rogers <irogers@xxxxxxxxxx>; Adrian
> Hunter <adrian.hunter@xxxxxxxxx>; Liang, Kan
> <kan.liang@xxxxxxxxxxxxxxx>; linux-perf-
> users@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> bpf@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] perf: use __builtin_preserve_field_info
> for GCC compatibility
> 
> On Wed, Aug 06, 2025 at 05:27:02PM -0700, Yonghong Song
> wrote:
> > On 8/6/25 4:57 PM, Andrew Pinski wrote:
> > > > -----Original Message-----
> > > > From: Namhyung Kim <namhyung@xxxxxxxxxx>
> > > > Sent: Wednesday, August 6, 2025 4:34 PM
> > > > To: Sam James <sam@xxxxxxxxxx>
> > > > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>; Ingo Molnar
> > > > <mingo@xxxxxxxxxx>; Arnaldo Carvalho de Melo
> <acme@xxxxxxxxxx>;
> > > > Mark Rutland <mark.rutland@xxxxxxx>; Alexander
> Shishkin
> > > > <alexander.shishkin@xxxxxxxxxxxxxxx>; Jiri Olsa
> > > > <jolsa@xxxxxxxxxx>; Ian Rogers <irogers@xxxxxxxxxx>;
> Adrian Hunter
> > > > <adrian.hunter@xxxxxxxxx>; Liang, Kan
> <kan.liang@xxxxxxxxxxxxxxx>;
> > > > Andrew Pinski <quic_apinski@xxxxxxxxxxx>; linux-perf-
> > > > users@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > > > bpf@xxxxxxxxxxxxxxx
> > > > Subject: Re: [PATCH] perf: use
> __builtin_preserve_field_info for
> > > > GCC compatibility
> > > >
> > > > Hello,
> > > >
> > > > On Wed, Aug 06, 2025 at 01:03:01AM +0100, Sam James
> > > > wrote:
> > > > > When exploring building bpf_skel with GCC's BPF
> support,
> > > > there was a
> > > > > buid failure because of bpf_core_field_exists vs the
> > > > mem_hops bitfield:
> > > > > ```
> > > > >   In file included from
> util/bpf_skel/sample_filter.bpf.c:6:
> > > > > util/bpf_skel/sample_filter.bpf.c: In function
> > > > 'perf_get_sample':
> > > > > tools/perf/libbpf/include/bpf/bpf_core_read.h:169:42:
> > > > error: cannot take address of bit-field 'mem_hops'
> > > > >    169 | #define ___bpf_field_ref1(field)        (&(field))
> > > > >        |                                          ^
> > > > > tools/perf/libbpf/include/bpf/bpf_helpers.h:222:29:
> note: in
> > > > expansion of macro '___bpf_field_ref1'
> > > > >    222 | #define ___bpf_concat(a, b) a ## b
> > > > >        |                             ^
> > > > > tools/perf/libbpf/include/bpf/bpf_helpers.h:225:29:
> note: in
> > > > expansion of macro '___bpf_concat'
> > > > >    225 | #define ___bpf_apply(fn, n) ___bpf_concat(fn,
> n)
> > > > >        |                             ^~~~~~~~~~~~~
> > > > > tools/perf/libbpf/include/bpf/bpf_core_read.h:173:9:
> note:
> > > > in expansion of macro '___bpf_apply'
> > > > >    173 |         ___bpf_apply(___bpf_field_ref,
> > > > ___bpf_narg(args))(args)
> > > > >        |         ^~~~~~~~~~~~
> > > > > tools/perf/libbpf/include/bpf/bpf_core_read.h:188:39:
> note:
> > > > in expansion of macro '___bpf_field_ref'
> > > > >    188 |
> > > > __builtin_preserve_field_info(___bpf_field_ref(field),
> > > > BPF_FIELD_EXISTS)
> > > > >        |                                       ^~~~~~~~~~~~~~~~
> > > > > util/bpf_skel/sample_filter.bpf.c:167:29: note: in
> expansion
> > > > of macro 'bpf_core_field_exists'
> > > > >    167 |                         if (bpf_core_field_exists(data-
> > > > > mem_hops))
> > > > >        |                             ^~~~~~~~~~~~~~~~~~~~~
> > > > > cc1: error: argument is not a field access ```
> > > > >
> > > > > ___bpf_field_ref1 was adapted for GCC in
> > > > > 12bbcf8e840f40b82b02981e96e0a5fbb0703ea9
> > > > > but the trick added for compatibility in
> > > > > 3a8b8fc3174891c4c12f5766d82184a82d4b2e3e
> > > > > isn't compatible with that as an address is used as an
> > > > argument.
> > > > > Workaround this by calling
> __builtin_preserve_field_info
> > > > directly as
> > > > > the bpf_core_field_exists macro does, but without the
> > > > ___bpf_field_ref use.
> > > >
> > > > IIUC GCC doesn't support bpf_core_fields_exists() for
> bitfield
> > > > members, right?  Is it gonna change in the future?
> > > Let's discuss how __builtin_preserve_field_info is handled
> in the first place for BPF. Right now it seems it is passed some
> expression as the first argument is never evaluated.
> > > The problem is GCC's implementation of
> __builtin_preserve_field_info is all in the backend and the
> front end does not understand of the special rules here.
> > >
> > > GCC implements some "special" builtins in the front-end
> but not by the normal function call rules but parsing them
> separately; this is how __builtin_offsetof and a few others are
> implemented in both the C and C++ front-ends (and
> implemented separately). Now we could have add a hook to
> allow a backend to something similar and maybe that is the
> best way forward here.
> > > But it won't be __builtin_preserve_field_info but rather
> `__builtin_preserve_field_type_info(type,field,kind)` instead.
> > >
> > > __builtin_preserve_enum_type_value would also be
> added with the following:
> > > __builtin_preserve_enum_type_value(enum_type,
> enum_value, kind)
> > >
> > > And change all of the rest of the builtins to accept a true
> type argument rather than having to cast an null pointer to
> that type.
> > >
> > > Will clang implement a similar builtin?
> >
> > The clang only has one builtin for some related relocations:
> >    __builtin_preserve_field_info(..., BPF_FIELD_EXISTS)
> >    __builtin_preserve_field_info(...,
> BPF_FIELD_BYTE_OFFSET)
> >    ...
> > They are all used in bpf_core_read.h.
> >
> > >
> > > Note this won't be done until at least GCC 16; maybe not
> until GCC 17 depending on if I or someone else gets time to
> implement the front-end parts which is acceptable to both the
> C and C++ front-ends.
> 
> So I'm taking the patch as-is, ok?
> 
> But first we need the Signed-off-by tag from Andrew Pinski as
> he is listed in a Co-authored-by, that I replaced with Co-
> developed-by as its the term used for this purpose in:
> 
> Yonghong, can I add an Acked-by: you since you participated in
> this discussion agreeing with the original patch (If I'm not
> mistaken)?

Signed-off-by: Andrew Pinski <andrew.pinski@xxxxxxxxxxxxxxxx>

Note my email address for doing Linux and GCC development is Andrew.pinski@xxxxxxxxxxxxxxxx . It was apinski_quic@xxxxxxxxxxx but that changed last month.

Thanks,
Andrew Pinski


>