On Thu, Feb 8, 2024 at 11:58 AM Cupertino Miranda
<cupertino.miranda@xxxxxxxxxx> wrote:
>
> Due to internal differences between LLVM and GCC the current
> implementation for the CO-RE macros does not fit GCC parser, as it will
> optimize those expressions even before those would be accessible by the
> BPF backend.
>
> As examples, the following would be optimized out with the original
> definitions:
> - As enums are converted to their integer representation during
> parsing, the IR would not know how to distinguish an integer
> constant from an actual enum value.
> - Types need to be kept as temporary variables, as the existing type
> casts of the 0 address (as expanded for LLVM), are optimized away by
> the GCC C parser, never really reaching GCCs IR.
>
> Although, the macros appear to add extra complexity, the expanded code
> is removed from the compilation flow very early in the compilation
> process, not really affecting the quality of the generated assembly.
>
> Signed-off-by: Cupertino Miranda <cupertino.miranda@xxxxxxxxxx>
> ---
> tools/lib/bpf/bpf_core_read.h | 46 ++++++++++++++++++++++++++++++-----
> 1 file changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
> index 0d3e88bd7d5f..074f1f4e4d2b 100644
> --- a/tools/lib/bpf/bpf_core_read.h
> +++ b/tools/lib/bpf/bpf_core_read.h
> @@ -81,6 +81,23 @@ enum bpf_enum_value_kind {
> val; \
> })
>
> +/* Differentiator between compilers builtin implementations. This is a
> + * requirement due to the compiler parsing differences where GCC optimizes
> + * early in parsing those constructs of type pointers to the builtin specific
> + * type, resulting in not being possible to collect the required type
> + * information in the builtin expansion.
> + */
> +#ifdef __clang__
> +#define bpf_type_for_compiler(type) ((typeof(type) *) 0)
let's call it something with triple underscore and shorter at the same
time. ___bpf_typeof()?
> +#else
> +#define COMPOSE_VAR(t, s) t##s
we already define this as ___concat() in this file, let's reuse that one
> +#define bpf_type_for_compiler1(type, NR) ({ \
> + extern typeof(type) *COMPOSE_VAR(bpf_type_tmp_, NR); \
nite: double space
please also align '\' at the end to match the rest of this file
> + COMPOSE_VAR(bpf_type_tmp_, NR); \
> +})
> +#define bpf_type_for_compiler(type) bpf_type_for_compiler1(type, __COUNTER__)
> +#endif
> +
> /*
> * Extract bitfield, identified by s->field, and return its value as u64.
> * This version of macro is using direct memory reads and should be used from
[...]
> * Convenience macro to check that provided enumerator value is defined in
> @@ -246,8 +268,14 @@ enum bpf_enum_value_kind {
> * kernel's BTF;
> * 0, if no matching enum and/or enum value within that enum is found.
> */
> +#ifdef __clang__
> #define bpf_core_enum_value_exists(enum_type, enum_value) \
> __builtin_preserve_enum_value(*(typeof(enum_type) *)enum_value, BPF_ENUMVAL_EXISTS)
> +#else
> +#define bpf_core_enum_value_exists(enum_type, enum_value) \
> + __builtin_preserve_enum_value(bpf_type_for_compiler(enum_type), \
> + enum_value, BPF_ENUMVAL_EXISTS)
with ___bpf_typeof() it should fit on one line
> +#endif
>
> /*
> * Convenience macro to get the integer value of an enumerator value in
> @@ -257,8 +285,14 @@ enum bpf_enum_value_kind {
> * present in target kernel's BTF;
> * 0, if no matching enum and/or enum value within that enum is found.
> */
> +#ifdef __clang__
> #define bpf_core_enum_value(enum_type, enum_value) \
> __builtin_preserve_enum_value(*(typeof(enum_type) *)enum_value, BPF_ENUMVAL_VALUE)
> +#else
> +#define bpf_core_enum_value(enum_type, enum_value) \
> + __builtin_preserve_enum_value(bpf_type_for_compiler(enum_type), \
> + enum_value, BPF_ENUMVAL_VALUE)
> +#endif
>
> /*
> * bpf_core_read() abstracts away bpf_probe_read_kernel() call and captures
> --
> 2.30.2
>
pw-bot: cr
