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

[Yonghong Song <yonghong.song@linux.dev>]

On 8/6/25 4:57 PM, Andrew Pinski wrote:
>
>> -----Original Message-----
>> From: Namhyung Kim <namhyung@kernel.org>
>> Sent: Wednesday, August 6, 2025 4:34 PM
>> To: Sam James <sam@gentoo.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>; Ingo Molnar
>> <mingo@redhat.com>; Arnaldo Carvalho de Melo
>> <acme@kernel.org>; Mark Rutland
>> <mark.rutland@arm.com>; Alexander Shishkin
>> <alexander.shishkin@linux.intel.com>; Jiri Olsa
>> <jolsa@kernel.org>; Ian Rogers <irogers@google.com>; Adrian
>> Hunter <adrian.hunter@intel.com>; Liang, Kan
>> <kan.liang@linux.intel.com>; Andrew Pinski
>> <quic_apinski@quicinc.com>; linux-perf-
>> users@vger.kernel.org; linux-kernel@vger.kernel.org;
>> bpf@vger.kernel.org
>> 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.
>
> Thanks,
> Andrew Pinski
>
>>> Link: https://gcc.gnu.org/PR121420
>>> Co-authored-by: Andrew Pinski
>> <quic_apinski@quicinc.com>
>>> Signed-off-by: Sam James <sam@gentoo.org>
>>> ---
>>>   tools/perf/util/bpf_skel/sample_filter.bpf.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/util/bpf_skel/sample_filter.bpf.c
>>> b/tools/perf/util/bpf_skel/sample_filter.bpf.c
>>> index b195e6efeb8be..e5666d4c17228 100644
>>> --- a/tools/perf/util/bpf_skel/sample_filter.bpf.c
>>> +++ b/tools/perf/util/bpf_skel/sample_filter.bpf.c
>>> @@ -164,7 +164,7 @@ static inline __u64
>> perf_get_sample(struct bpf_perf_event_data_kern *kctx,
>>>                if (entry->part == 8) {
>>>                        union perf_mem_data_src___new *data = (void
>>> *)&kctx->data->data_src;
>>>
>>> -                     if (bpf_core_field_exists(data->mem_hops))
>>> +                     if
>>> + (__builtin_preserve_field_info(data->mem_hops,
>> BPF_FIELD_EXISTS))
>>
>> I believe those two are equivalent (maybe worth a
>> comment?).  But it'd be great if BPF/clang folks can review if
>> it's ok.
>>
>> Anyway, I can build it with clang.
>>
>> Tested-by: Namhyung Kim <namhyung@kernel.org>
>>
>> Thanks,
>> Namhyung
>>
>>
>>>                                return data->mem_hops;
>>>
>>>                        return 0;
>>> --
>>> 2.50.1
>>>