> On Oct 7, 2024, at 20:37, Kito Cheng <kito.ch...@gmail.com> wrote:
>
> I suggest not handling the extension implication rules. This way, it
> can simplify
> the logic and reduce the cost of run-time checks.
>
> Also, you need to consider situations where that extension can't be detected.
For this, I would also like to update the __init_riscv_features_bits_linux
function in another patch [1] to set implied extensions such as V
implies Zve32x.
[1]
https://patchwork.sourceware.org/project/gcc/patch/20241003182256.1765569-1-chenyan...@isrc.iscas.ac.cn/
>
> And last, I would like to defer this until run-time resolver patch coming, but
> welcome to send another version of RFC patch :P
>
No worries. And I also provide a basic functional target_clones branch [2].
[2] https://github.com/cyyself/gcc/tree/rv_target_clones_20241007
>
> On Sun, Oct 6, 2024 at 2:21 AM Yangyu Chen <chenyan...@isrc.iscas.ac.cn>
> wrote:
>>
>> This patch implements the riscv_minimal_hwprobe_feature_bits feature
>> for the RISC-V target. The feature bits are defined in the previous
>> patch [1] to provide bitmasks of ISA extensions that defined in RISC-V
>> C-API. Thus, we need a function to generate the feature bits for IFUNC
>> resolver to dispatch between different functions based on the hardware
>> features. The final version of the target_clones support on RISC-V is
>> still under development, I am working on it.
>>
>> The minimal feature bits means to use the earliest extension appeard in
>> the Linux hwprobe to cover the given ISA string. To allow older kernels
>> without some implied extensions probe to run the FMV dispatcher
>> correctly.
>>
>> For example, V implies Zve32x, but Zve32x appears in the Linux kernel
>> since v6.11. If we use isa string directly to generate FMV dispatcher
>> with functions with "arch=+v" extension, since we have V implied the
>> Zve32x, FMV dispatcher will check if the Zve32x extension is supported
>> by the host. If the Linux kernel is older than v6.11, the FMV dispatcher
>> will fail to detect the Zve32x extension even it already implies by the
>> V extension, thus making the FMV dispatcher fail to dispatch the correct
>> function.
>>
>> Thus, we need to generate the minimal feature bits to cover the given
>> ISA string to allow the FMV dispatcher to work correctly on older
>> kernels.
>>
>> [1]
>> https://patchwork.sourceware.org/project/gcc/patch/20241003182256.1765569-1-chenyan...@isrc.iscas.ac.cn/
>>
>> gcc/ChangeLog:
>>
>> * common/config/riscv/riscv-common.cc
>> (struct riscv_ext_bitmask_table_t): New struct.
>> (riscv_minimal_hwprobe_feature_bits): New function.
>> * config/riscv/riscv-subset.h (GCC_RISCV_SUBSET_H):
>> (riscv_minimal_hwprobe_feature_bits): Declare the function.
>> * common/config/riscv/feature_bits.h: New file.
>> ---
>> gcc/common/config/riscv/feature_bits.h | 33 ++++++
>> gcc/common/config/riscv/riscv-common.cc | 144 ++++++++++++++++++++++++
>> gcc/config/riscv/riscv-subset.h | 4 +
>> 3 files changed, 181 insertions(+)
>> create mode 100644 gcc/common/config/riscv/feature_bits.h
>>
>> diff --git a/gcc/common/config/riscv/feature_bits.h
>> b/gcc/common/config/riscv/feature_bits.h
>> new file mode 100644
>> index 00000000000..c6c6d983edb
>> --- /dev/null
>> +++ b/gcc/common/config/riscv/feature_bits.h
>
> Rename to riscv_feature_bits.h to prevent potential file name conflict
> also move to gcc/config/riscv/ rather than gcc/config/riscv/
>
>> @@ -0,0 +1,33 @@
>> +/* Definition of RISC-V feature bits corresponding to
>> + libgcc/config/riscv/feature_bits.c
>> + Copyright (C) 2024 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/>. */
>> +
>
> Need header guard, something like #ifndef GCC_RISCV_FEATURE_BITS_H
>
>> +#define RISCV_FEATURE_BITS_LENGTH 2
>> +struct riscv_feature_bits {
>> + unsigned length;
>> + unsigned long long features[RISCV_FEATURE_BITS_LENGTH];
>> +};
>> +
>> +#define RISCV_VENDOR_FEATURE_BITS_LENGTH 1
>> +
>> +struct riscv_vendor_feature_bits {
>> + unsigned vendorID;
>> + unsigned length;
>> + unsigned long long features[RISCV_VENDOR_FEATURE_BITS_LENGTH];
>> +};
>> diff --git a/gcc/common/config/riscv/riscv-common.cc
>> b/gcc/common/config/riscv/riscv-common.cc
>> index bd42fd01532..9f343782ae6 100644
>> --- a/gcc/common/config/riscv/riscv-common.cc
>> +++ b/gcc/common/config/riscv/riscv-common.cc
>> @@ -19,6 +19,7 @@ along with GCC; see the file COPYING3. If not see
>>
>> #include <sstream>
>> #include <vector>
>> +#include <queue>
>>
>> #define INCLUDE_STRING
>> #define INCLUDE_SET
>> @@ -1754,6 +1755,75 @@ static const riscv_ext_flag_table_t
>> riscv_ext_flag_table[] =
>> {NULL, NULL, 0}
>> };
>>
>> +/* Types for recording extension to RISC-V C-API bitmask. */
>> +struct riscv_ext_bitmask_table_t {
>> + const char *ext;
>> + int groupid;
>> + int bit_position;
>> +};
>> +
>> +/* Mapping table between extension to RISC-V C-API extension bitmask.
>> + This table should sort the extension by Linux hwprobe order to get the
>> + minimal feature bits. */
>> +static const riscv_ext_bitmask_table_t riscv_ext_bitmask_table[] =
>> +{
>> + {"i", 0, 8},
>> + {"m", 0, 12},
>> + {"a", 0, 0},
>> + {"f", 0, 5},
>> + {"d", 0, 3},
>> + {"c", 0, 2},
>> + {"v", 0, 21},
>> + {"zba", 0, 27},
>> + {"zbb", 0, 28},
>> + {"zbs", 0, 33},
>> + {"zicboz", 0, 37},
>> + {"zbc", 0, 29},
>> + {"zbkb", 0, 30},
>> + {"zbkc", 0, 31},
>> + {"zbkx", 0, 32},
>> + {"zknd", 0, 41},
>> + {"zkne", 0, 42},
>> + {"zknh", 0, 43},
>> + {"zksed", 0, 44},
>> + {"zksh", 0, 45},
>> + {"zkt", 0, 46},
>> + {"zvbb", 0, 48},
>> + {"zvbc", 0, 49},
>> + {"zvkb", 0, 52},
>> + {"zvkg", 0, 53},
>> + {"zvkned", 0, 54},
>> + {"zvknha", 0, 55},
>> + {"zvknhb", 0, 56},
>> + {"zvksed", 0, 57},
>> + {"zvksh", 0, 58},
>> + {"zvkt", 0, 59},
>> + {"zfh", 0, 35},
>> + {"zfhmin", 0, 36},
>> + {"zihintntl", 0, 39},
>> + {"zvfh", 0, 50},
>> + {"zvfhmin", 0, 51},
>> + {"zfa", 0, 34},
>> + {"ztso", 0, 47},
>> + {"zacas", 0, 26},
>> + {"zicond", 0, 38},
>> + {"zihintpause", 0, 40},
>> + {"zve32x", 0, 60},
>> + {"zve32f", 0, 61},
>> + {"zve64x", 0, 62},
>> + {"zve64f", 0, 63},
>> + {"zve64d", 1, 0},
>> + {"zimop", 1, 1},
>> + {"zca", 1, 2},
>> + {"zcb", 1, 3},
>> + {"zcd", 1, 4},
>> + {"zcf", 1, 5},
>> + {"zcmop", 1, 6},
>> + {"zawrs", 1, 7},
>> +
>> + {NULL, -1, -1}
>> +};
>> +
>> /* Apply SUBSET_LIST to OPTS if OPTS is not null. */
>>
>> void
>> @@ -1783,6 +1853,80 @@ riscv_set_arch_by_subset_list (riscv_subset_list
>> *subset_list,
>> }
>> }
>>
>> +/* Get the minimal feature bits in Linux hwprobe of the given ISA string.
>> +
>> + Used for generating Function Multi-Versioning (FMV) dispatcher for
>> RISC-V.
>> +
>> + The minimal feature bits refer to using the earliest extension that
>> appeared
>> + in the Linux hwprobe to support the specified ISA string. This ensures
>> that
>> + older kernels, which may lack certain implied extensions, can still run
>> the
>> + FMV dispatcher correctly. */
>> +
>> +bool
>> +riscv_minimal_hwprobe_feature_bits (const char *isa,
>> + struct riscv_feature_bits *res)
>> +{
>> + riscv_subset_list *subset_list;
>> + subset_list = riscv_subset_list::parse (isa, UNKNOWN_LOCATION);
>> + if (!subset_list)
>> + return false;
>> +
>> + /* Initialize the result feature bits to zero. */
>> + res->length = RISCV_FEATURE_BITS_LENGTH;
>> + for (int i = 0; i < RISCV_FEATURE_BITS_LENGTH; ++i)
>> + res->features[i] = 0;
>> +
>> + /* Use a std::set to record all visted implied extensions. */
>
> Typo: visted -> visited
>
>> + std::set <std::string> implied_exts;
>> +
>> + /* Iterate through the extension bitmask table in Linux hwprobe order to
>> get
>> + the minimal covered feature bits. Avoding some sub-extensions which
>> will
>
> Typo: Avoding -> Avoiding
>
>> + be implied by the super-extensions like V implied Zve32x. */
>> + const riscv_ext_bitmask_table_t *ext_bitmask_tab;
>> + for (ext_bitmask_tab = &riscv_ext_bitmask_table[0];
>> + ext_bitmask_tab->ext;
>> + ++ext_bitmask_tab)
>> + {
>> + /* Skip the extension if it is not in the subset list or already
>> implied
>> + by previous extension. */
>> + if (subset_list->lookup (ext_bitmask_tab->ext) == NULL
>> + || implied_exts.count (ext_bitmask_tab->ext))
>> + continue;
>> +
>> + res->features[ext_bitmask_tab->groupid]
>> + |= 1ULL << ext_bitmask_tab->bit_position;
>> +
>> + /* Find the sub-extension using BFS and set the corresponding bit. */
>> + std::queue <const char *> search_q;
>> + search_q.push (ext_bitmask_tab->ext);
>> +
>> + while (!search_q.empty ())
>> + {
>> + const char * search_ext = search_q.front ();
>> + search_q.pop ();
>> +
>> + /* Iterate through the implied extension table. */
>> + const riscv_implied_info_t *implied_info;
>> + for (implied_info = &riscv_implied_info[0];
>> + implied_info->ext;
>> + ++implied_info)
>> + {
>> + /* When the search extension matches the implied extension and
>> + the implied extension has not been visited, mark the implied
>> + extension in the implied_exts set and push it into the
>> + queue. */
>> + if (implied_info->match (subset_list, search_ext)
>> + && implied_exts.count (implied_info->implied_ext) == 0)
>> + {
>> + implied_exts.insert (implied_info->implied_ext);
>> + search_q.push (implied_info->implied_ext);
>> + }
>> + }
>> + }
>> + }
>> + return true;
>> +}
>> +
>> /* Parse a RISC-V ISA string into an option mask. Must clear or set all arch
>> dependent mask bits, in case more than one -march string is passed. */
>>
>> diff --git a/gcc/config/riscv/riscv-subset.h
>> b/gcc/config/riscv/riscv-subset.h
>> index dace4de6575..5a1489c00ea 100644
>> --- a/gcc/config/riscv/riscv-subset.h
>> +++ b/gcc/config/riscv/riscv-subset.h
>> @@ -22,6 +22,8 @@ along with GCC; see the file COPYING3. If not see
>> #ifndef GCC_RISCV_SUBSET_H
>> #define GCC_RISCV_SUBSET_H
>>
>> +#include "common/config/riscv/feature_bits.h"
>> +
>> #define RISCV_DONT_CARE_VERSION -1
>>
>> /* Subset info. */
>> @@ -120,5 +122,7 @@ public:
>> extern const riscv_subset_list *riscv_cmdline_subset_list (void);
>> extern void
>> riscv_set_arch_by_subset_list (riscv_subset_list *, struct gcc_options *);
>> +extern bool riscv_minimal_hwprobe_feature_bits (const char *,
>> + struct riscv_feature_bits *);
>
> Declare this in riscv_feature_bits.h is fine, then you don't need to
> change anything within this file.
>
OK. I will fixed these in the next revision.
Thanks,
Yangyu Chen
>>
>> #endif /* ! GCC_RISCV_SUBSET_H */
>>
>> base-commit: a8e6360765336969e3f45ac16e4340e5e5468768
>> prerequisite-patch-id: 2a8fa0993d052b742835fa76402e4f763feb3475
>> --
>> 2.45.2
