On Tue, 2026-02-03 at 01:49 +0800, Yangyu Chen wrote:
> This patch adds support for target_clones table option. The
> target_clones table option allows users to specify multiple versions
> of a function and select the version at runtime based on the
> specified
> table.
>
> The target clone table is a JSON object where function names serve as
> keys, and their values are nested objects. Each nested object maps
> architecture names to lists of target clone attributes. This
> structure
> allows specifying function clones for different architectures. Below
> is
> an example:
>
> ```
> {
> "foo": {
> "x86_64": ["avx2", "avx512f"],
> "riscv64": ["arch=+v", "arch=+zba,+zbb", ...],
> ... // more architectures
> },
> // more functions
> }
> ```
>
> A example of the table is as follows on RISC-V:
>
> C source code "ror32.c":
>
> ```
> void ror32(unsigned int *a, unsigned int b, unsigned long size) {
> for (unsigned long i = 0; i < size; i++) {
> a[i] = a[i] >> b | (a[i] << (32 - b));
> }
> }
> ```
>
> Table "ror32.target_clones":
>
> ```
> {
> "ror32": {
> "riscv64": ["arch=+zvbb,+zbb", "arch=+zbb"]
> }
> }
> ```
>
> Then use: gcc -O3 -ftarget-clones-table=ror32.target_clones -S
> ror32.c
> to compile the source code. This will generate 3 versions and its
> IFUNC
> resolver for the ror32 function which is "arch=+zvbb,+zbb" and
> "arch=+zbb" and the default version.
>
> Signed-off-by: Yangyu Chen <[email protected]>
[...snip...]
> +
> + /* Parse the JSON string.
> + The JSON string format looks like this:
> + {
> + "symbol_name1": {
> + "target1": ["clone1", "clone2", ...],
> + "target2": ["clone1", "clone2", ...],
> + },
> + ...
> + }
> + where symbol_name is the ASM name of the function mangled by
> the
> + frontend. The target1 and target2 are the targets, which can
> be
> + "x86_64", "aarch64", "riscv64", etc. The clone1, clone2, etc
> are the
> + target clones attributes, which can be "avx2", "avx512" etc.
> Note that
> + there is no need to specify the "default" target clone, it is
> + automatically added by the pass. */
> + json::parser_result_t result = json::parse_utf8_string (
> + json_str.size (), json_str.c_str (), true, NULL);
Note that "true" for the 3rd param enables C/C++-style comments when
parsing the JSON. This is probably a good thing as I imagine users
might want to document their clones tables. Should we document that
this common extension of the JSON spec is supported?
Do you anticipate these files being human-written, auto-generated, or
both?
> + if (auto json_err = result.m_err.get ())
> + {
> + error ("error parsing target clone table file %s: %s",
> + target_clones_table, json_err->get_msg ());
> + return res;
> + }
> +
> + auto json_val = result.m_val.get ();
> + auto kind = json_val->get_kind ();
> + if (kind != json::JSON_OBJECT)
> + {
> + error ("target clone table file %s is not a JSON object",
> + target_clones_table);
> + return res;
> + }
I see that here you have a bunch of hand-written logic here for walking
the tree of JSON values and extracting the data of interest. FWIW I
have some code in libsarifreplay.cc that has various helper functions
for doing a similar thing (albeit using libgdiagnostics, rather then
the diagnostics layer directly), and I see similar helpers in
gcc/config/aarch64/aarch64-json-tunings-parser.cc. To my knowledge
these are the only places so far in the codebase where we currently
parse JSON (other than selftests).
We should probably have a shared set of helpers for this, and support
locations within the json file so that we can have good location-
reporting for diagnostics involving bad json inputs - it's a bad UX to
be told you have an error *somewhere* in a file you wrote without
showing line/column etc - but that's probably out-of-scope for this
patch series (especially given the simplicity of your format). I have
somewhere an old patch that integrated the json-parsing with location_t
so that we can have good location-reporting on malformed/invalid json
inputs (perhaps generating the location_t entries on demand if we're
emitting an error about them); do you want me to try to dig it up?
> + auto json_obj = static_cast<const json::object *> (json_val);
> + unsigned i;
> + const char *symbol_name;
> + FOR_EACH_VEC_ELT (*json_obj, i, symbol_name)
> + {
> + auto symbol_val = json_obj->get (symbol_name);
> + if (!symbol_val || symbol_val->get_kind () !=
> json::JSON_OBJECT)
> + continue;
> + auto symbol_obj = static_cast<const json::object *>
> (symbol_val);
> + auto cur_target_val = symbol_obj->get (target.c_str ());
> + if (!cur_target_val
> + || cur_target_val->get_kind () != json::JSON_ARRAY)
> + continue;
> + auto cur_target_array = static_cast<const json::array *>
> + (cur_target_val);
> + for (unsigned j = 0; j < cur_target_array->length (); j++)
> + {
> + auto target_str_val = cur_target_array->get (j);
> + if (target_str_val->get_kind () != json::JSON_STRING)
> + error ("target clones attribute is not a string");
> + const char *target_str
> + = static_cast<const json::string *> (target_str_val)-
> >get_string ();
> + if (strcmp (target_str, "default") == 0)
> + error ("No need to specify \"default\" in target
> clones table");
> + res[symbol_name].safe_push (string_slice (ggc_strdup
> (target_str)));
> + }
> + }
> + return res;
> +}
[...snip...]
> diff --git a/gcc/testsuite/gcc.target/i386/tct-0.json
> b/gcc/testsuite/gcc.target/i386/tct-0.json
> new file mode 100644
> index 000000000000..9dc712e66e2b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/tct-0.json
> @@ -0,0 +1,5 @@
> +{
> + "foo": {
> + "x86_64": ["arch=x86-64-v2", "arch=x86-64-v3", "arch=x86-64-
> v4"]
> + }
> +}
FWIW the testsuite doesn't have any coverage for invalid JSON input.
How gracefully do we handle e.g. a missing comma in the input?
Should we ship JSON schemata for the JSON input formats we support? (so
that users can validate their files, and potentially have IDE support
for typesafe editing of them)
Hope this is constructive
Dave
