> Jose E. Marchesi writes: > >>> This patch updates the support for the BPF CO-RE builtins >>> __builtin_preserve_access_index and __builtin_preserve_field_info, >>> and adds support for the CO-RE builtins __builtin_btf_type_id, >>> __builtin_preserve_type_info and __builtin_preserve_enum_value. >>> >>> These CO-RE relocations are now converted to __builtin_core_reloc which >>> abstracts all of the original builtins in a polymorphic relocation >>> specific builtin. >>> >>> The builtin processing is now split in 2 stages, the first (pack) is >>> executed right after the front-end and the second (process) right before >>> the asm output. >>> >>> In expand pass the __builtin_core_reloc is converted to a >>> unspec:UNSPEC_CORE_RELOC rtx entry. >>> >>> The data required to process the builtin is now collected in the packing >>> stage (after front-end), not allowing the compiler to optimize any of >>> the relevant information required to compose the relocation when >>> necessary. >>> At expansion, that information is recovered and CTF/BTF is queried to >>> construct the information that will be used in the relocation. >>> At this point the relocation is added to specific section and the >>> builtin is expanded to the expected default value for the builtin. >>> >>> In order to process __builtin_preserve_enum_value, it was necessary to >>> hook the front-end to collect the original enum value reference. >>> This is needed since the parser folds all the enum values to its >>> integer_cst representation. >>> >>> More details can be found within the core-builtins.cc. >>> >>> Regtested in host x86_64-linux-gnu and target bpf-unknown-none. >>> --- >>> gcc/config.gcc | 4 +- >>> gcc/config/bpf/bpf-passes.def | 20 - >>> gcc/config/bpf/bpf-protos.h | 4 +- >>> gcc/config/bpf/bpf.cc | 817 +--------- >>> gcc/config/bpf/bpf.md | 17 + >>> gcc/config/bpf/core-builtins.cc | 1397 +++++++++++++++++ >>> gcc/config/bpf/core-builtins.h | 36 + >>> gcc/config/bpf/coreout.cc | 50 +- >>> gcc/config/bpf/coreout.h | 13 +- >>> gcc/config/bpf/t-bpf | 6 +- >>> gcc/doc/extend.texi | 51 + >>> ...core-builtin-fieldinfo-const-elimination.c | 29 + >>> 12 files changed, 1639 insertions(+), 805 deletions(-) >>> delete mode 100644 gcc/config/bpf/bpf-passes.def >>> create mode 100644 gcc/config/bpf/core-builtins.cc >>> create mode 100644 gcc/config/bpf/core-builtins.h >>> create mode 100644 >>> gcc/testsuite/gcc.target/bpf/core-builtin-fieldinfo-const-elimination.c >>> >>> diff --git a/gcc/config.gcc b/gcc/config.gcc >>> index eba69a463be0..c521669e78b1 100644 >>> --- a/gcc/config.gcc >>> +++ b/gcc/config.gcc >>> @@ -1597,8 +1597,8 @@ bpf-*-*) >>> use_collect2=no >>> extra_headers="bpf-helpers.h" >>> use_gcc_stdint=provide >>> - extra_objs="coreout.o" >>> - target_gtfiles="$target_gtfiles \$(srcdir)/config/bpf/coreout.cc" >>> + extra_objs="coreout.o core-builtins.o" >>> + target_gtfiles="$target_gtfiles \$(srcdir)/config/bpf/coreout.cc >>> \$(srcdir)/config/bpf/core-builtins.cc" >>> ;; >>> cris-*-elf | cris-*-none) >>> tm_file="elfos.h newlib-stdint.h ${tm_file}" >>> diff --git a/gcc/config/bpf/bpf-passes.def b/gcc/config/bpf/bpf-passes.def >>> deleted file mode 100644 >>> index deeaee988a01..000000000000 >>> --- a/gcc/config/bpf/bpf-passes.def >>> +++ /dev/null >>> @@ -1,20 +0,0 @@ >>> -/* Declaration of target-specific passes for eBPF. >>> - Copyright (C) 2021-2023 Free Software Foundation, Inc. >>> - >>> - This file is part of GCC. >>> - >>> - GCC is free software; you can redistribute it and/or modify it >>> - under the terms of the GNU General Public License as published by >>> - the Free Software Foundation; either version 3, or (at your option) >>> - any later version. >>> - >>> - GCC is distributed in the hope that it will be useful, but >>> - WITHOUT ANY WARRANTY; without even the implied warranty of >>> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >>> - General Public License for more details. >>> - >>> - You should have received a copy of the GNU General Public License >>> - along with GCC; see the file COPYING3. If not see >>> - <http://www.gnu.org/licenses/>. */ >>> - >>> -INSERT_PASS_AFTER (pass_df_initialize_opt, 1, pass_bpf_core_attr); >>> diff --git a/gcc/config/bpf/bpf-protos.h b/gcc/config/bpf/bpf-protos.h >>> index b484310e8cbf..fbcf5111eb21 100644 >>> --- a/gcc/config/bpf/bpf-protos.h >>> +++ b/gcc/config/bpf/bpf-protos.h >>> @@ -30,7 +30,7 @@ extern void bpf_print_operand_address (FILE *, rtx); >>> extern void bpf_expand_prologue (void); >>> extern void bpf_expand_epilogue (void); >>> extern void bpf_expand_cbranch (machine_mode, rtx *); >>> - >>> -rtl_opt_pass * make_pass_bpf_core_attr (gcc::context *); >>> +const char *bpf_add_core_reloc (rtx *operands, const char *templ); >>> +void bpf_process_move_operands (rtx *operands); >>> >>> #endif /* ! GCC_BPF_PROTOS_H */ >>> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc >>> index b5b5674edbb5..101e994905d2 100644 >>> --- a/gcc/config/bpf/bpf.cc >>> +++ b/gcc/config/bpf/bpf.cc >>> @@ -69,10 +69,7 @@ along with GCC; see the file COPYING3. If not see >>> #include "gimplify.h" >>> #include "gimplify-me.h" >>> >>> -#include "ctfc.h" >>> -#include "btf.h" >>> - >>> -#include "coreout.h" >>> +#include "core-builtins.h" >>> >>> /* Per-function machine data. */ >>> struct GTY(()) machine_function >>> @@ -174,22 +171,7 @@ static const struct attribute_spec >>> bpf_attribute_table[] = >>> one. */ >>> #define BPF_BUILTIN_MAX_ARGS 5 >>> >>> -enum bpf_builtins >>> -{ >>> - BPF_BUILTIN_UNUSED = 0, >>> - /* Built-ins for non-generic loads and stores. */ >>> - BPF_BUILTIN_LOAD_BYTE, >>> - BPF_BUILTIN_LOAD_HALF, >>> - BPF_BUILTIN_LOAD_WORD, >>> - >>> - /* Compile Once - Run Everywhere (CO-RE) support. */ >>> - BPF_BUILTIN_PRESERVE_ACCESS_INDEX, >>> - BPF_BUILTIN_PRESERVE_FIELD_INFO, >>> - >>> - BPF_BUILTIN_MAX, >>> -}; >>> - >>> -static GTY (()) tree bpf_builtins[(int) BPF_BUILTIN_MAX]; >>> +GTY (()) tree bpf_builtins[(int) BPF_BUILTIN_MAX]; >>> >>> void bpf_register_coreattr_pass (void); >>> >>> @@ -546,6 +528,17 @@ bpf_expand_cbranch (machine_mode mode, rtx *operands) >>> } >>> } >>> >>> +/* This is used define_expand "mov<MM:mode>" to verofy if we need to >>> replace >> >> Typo; verofy >> >>> + any of the operands. Currently this is used to replace the >>> + __attribute__((preserve_access_index)) by the respective >>> __builtin_core_reloc >>> + which will at final create the relocation and respective label. */ >>> + >>> +void >>> +bpf_process_move_operands (rtx *operands) >>> +{ >>> + bpf_replace_core_move_operands (operands); >>> +} >> >> Is this intermediary function really necessary? Just call >> bpf_place_core_move_operands from the expand. > No I did it only as a placeholder for any future requirements unrelated > to the CO-RE builtins. Will remove. >> >>> +(define_insn "mov_reloc_core<MM:mode>" >>> + [(set (match_operand:MM 0 "nonimmediate_operand" "=r,q,r") >>> + (unspec:MM [ >>> + (match_operand:MM 1 "immediate_operand" " I,I,B") >>> + (match_operand:SI 2 "immediate_operand" " I,I,I") >>> + ] UNSPEC_CORE_RELOC) >>> + )] >>> + "" >>> + "@ >>> + *return bpf_add_core_reloc (operands, \"{mov\t%0,%1|%0 = %1}\"); >>> + *return bpf_add_core_reloc (operands, \"{st<mop>\t%0,%1|*(<smop> *) >>> (%0) = %1}\"); >>> + *return bpf_add_core_reloc (operands, \"{lddw\t%0,%1|%0 = %1 ll}\");" >>> + [(set_attr "type" "alu,st,alu")]) >>> + >> >> I am assuming that this insn only has to implement whatever situations >> supported by valid operands and usage of the builtins in any possible >> context, right? i.e. it doesn't have to handle all the situations that >> "*movMODE" handles. And this is checked by the builtin implementation? > Only the implementations of the mov that take an immediate at the second > operand can actually be used for the builtin. This unspec is only > expanded in case of a builtin. I presume that this limits it to these > scenarios.
Ok, thanks for the info. I just wanted to make sure that no matter the builtins are used in the C programs we will never get an ICE because of lack of insns. > >>> +struct cr_builtins { >> >> Brace in first column please. >> >>> + /* FIXED: This wat not Ok. >> >> Hm? If that is fixed, do we still need that comment? :) >> >>> + emit_insn ( \ >>> + gen_mov_reloc_coredi (reg, \ >>> + gen_rtx_CONST_INT (Pmode, 0), \ >>> + gen_rtx_CONST_INT (Pmode, index))); \ >> >> These backslahes... was that in a macro originally? >> >>> + return true; >>> + } >>> + } >>> + return false; >>> +} > > Thanks!