> On 7 May 2025, at 6:18 PM, David Malcolm <dmalc...@redhat.com> wrote:
> 
> External email: Use caution opening links or attachments
> 
> 
> On Tue, 2025-05-06 at 14:00 +0530, soum...@nvidia.com wrote:
>> From: Soumya AR <soum...@nvidia.com>
>> 
>> Hi,
>> 
>> This RFC and subsequent patch series introduces support for printing
>> and parsing
>> of aarch64 tuning parameters in the form of JSON.
>> 
>> It is important to note that this mechanism is specifically intended
>> for power
>> users to experiment with tuning parameters. This proposal does not
>> suggest the
>> use of JSON tuning files in production. Additionally, the JSON format
>> should not
>> be considered stable and may change as GCC evolves.
>> 
>> [1] Introduction
>> 
>> Currently, the aarch64 backend in GCC (15) stores the tuning
>> parameteres of all
>> backends under gcc/config/aarch64/tuning_models/. Since these
>> parameters are
>> hardcoded for each CPU, this RFC proposes a technique to support the
>> adjustment
>> of these parameters at runtime. This allows easier experimentation
>> with more
>> aggressive parameters to find optimal numbers.
>> 
>> The tuning data is fed to the compiler in JSON format, which was
>> primarily
>> chosen for the following reasons:
>> 
>> * JSON can represent hierarchical data. This is useful for
>> incorporating the
>> nested nature of the tuning structures.
>> * JSON supports integers, strings, booleans, and arrays.
>> * GCC already has support for parsing and printing JSON, removing the
>> need for
>> writing APIs to read and write the JSON files.
>> 
>> Thus, if we take the following example of some tuning parameters:
>> 
>> static struct cpu_addrcost_table generic_armv9_a_addrcost_table =
>> {
>>    {
>>      1, /* hi  */
>>      0, /* si  */
>>      0, /* di  */
>>      1, /* ti  */
>>    },
>>  0, /* pre_modify  */
>>  0, /* post_modify  */
>>  2, /* post_modify_ld3_st3  */
>>  2, /* post_modify_ld4_st4  */
>> };
>> 
>> static cpu_prefetch_tune generic_armv9a_prefetch_tune =
>> {
>>  0,                  /* num_slots  */
>>  -1,                 /* l1_cache_size  */
>>  64,                 /* l1_cache_line_size  */
>>  -1,                 /* l2_cache_size  */
>>  true,                       /* prefetch_dynamic_strides */
>> };
>> 
>> static struct tune_params neoversev3_tunings =
>> {
>>  &generic_armv9_a_addrcost_table,
>>  10, /* issue_rate  */
>>  AARCH64_FUSE_NEOVERSE_BASE, /* fusible_ops  */
>>  "32:16",    /* function_align.  */
>>  &generic_armv9a_prefetch_tune,
>>  AARCH64_LDP_STP_POLICY_ALWAYS,   /* ldp_policy_model.  */
>> };
>> 
>> We can represent them in JSON as:
>> 
>> {
>>  "tune_params": {
>>    "addr_cost": {
>>      "addr_scale_costs": { "hi": 1, "si": 0, "di": 0, "ti": 1 },
>>      "pre_modify": 0,
>>      "post_modify": 0,
>>      "post_modify_ld3_st3": 2,
>>      "post_modify_ld4_st4": 2
>>    },
>>    "issue_rate": 10,
>>    "fusible_ops": 1584,
>>    "function_align": "32:16",
>>    "prefetch": {
>>      "num_slots": 0,
>>      "l1_cache_size": -1,
>>      "l1_cache_line_size": 64,
>>      "l2_cache_size": -1,
>>      "prefetch_dynamic_strides": true
>>    },
>>    "ldp_policy_model": "AARCH64_LDP_STP_POLICY_ALWAYS"
>>  }
>> }
>> 
>> ---
>> 
>> [2] Methodology
>> 
>> Before the internal tuning parameters are overridden with user
>> provided ones, we
>> must ensure the validity of the provided data.
>> 
>> This is done using a "base" JSON schema, which contains information
>> about the
>> tune_params data structure used by the aarch64 backend.
>> 
>> Example:
>> 
>> {
>>  "tune_params": {
>>    "addr_cost": {
>>      "addr_scale_costs": {
>>        "hi": "int",
>>        "si": "int",
>>        "di": "int",
>>        "ti": "int"
>>      },
>>      "pre_modify": "int",
>>      "post_modify": "int",
>>      "post_modify_ld3_st3": "int",
>>      "post_modify_ld4_st4": "int"
>>    },
>>    "issue_rate": "int",
>>    "fusible_ops": "uint",
>>    "function_align": "string",
>>    "prefetch": {
>>      "num_slots": "int",
>>      "l1_cache_size": "int",
>>      "l1_cache_line_size": "int",
>>      "l2_cache_size": "int",
>>      "prefetch_dynamic_strides": "boolean"
>>    },
>>    "ldp_policy_model": "string"
>>  }
>> }
>> 
>> Using this schema, we can:
>>      * Verify that the correct datatypes have been used.
>>      * Verify if the user provided "key" or tuning parameter
>> exists.
>>      * Allow user to only specify the required fields (in nested
>> fashion),
>>      eliminating the need to list down every single paramter if
>> they only
>>      wish to experiment with some.
>> 
>> The schema is currently stored as a raw JSON string in
>> config/aarch64/aarch64-json-schema.h.
> 

> Does the schema get used to do validation anywhere?  FWIW (and as
> posted in another followup), for SARIF the DejaGnu tests validate the
> generated json against a schema; see gcc/testsuite/lib/scansarif.exp;
> there's also run-sarif-pytest which allows DejaGnu to run Python
> scripts to verify properties of the generated json.  The latter is
> probably overkill for the aarch64 tuning use-case, but is very helpful
> for SARIF, which has deeply nested json, cross-references, duplication
> and de-duplication, etc (so regexps aren't expressive enough for
> testing it)
> 

> [...snip...]
> 
>> ---
>> 
>> [3] Testing
>> 
>> To test out the functionality for this change, we have to ensure the
>> following
>> things are happening correctly:
>> 
>> 1. The JSON tunings printer is able to print back the correct values,
>> especially
>> when it comes to trickier datatypes like enums.
>> 2. The error handling works as expected, espcially in the case of
>> incorrect JSON
>> syntax, incorrect datatypes, and incorrect tuning data structure.
>> 3. During GCC invokation, the values from JSON are correctly loaded
>> in
>> aarch64_tune_params.
> 

Hi David,

Thanks for taking the time for this! It helped me quite a bit in reassessing my
approaches. I’m sorry it took so long to get back to you.

I have put up updated patches at:
https://gcc.gnu.org/pipermail/gcc-patches/2025-July/689893.html


> The error-handling seems to use plain "error (msg);" which uses
> input_location, which isn't going to tell users where in the JSON the
> problem is.  Do the error messages contain enough context for people to
> debug this?  Or are we assuming that this is an advanced feature that
> doesn't need a lot of UX polish?

Currently, I have a way of building up the error string as I parse through the
user provided JSON.

Let’s say there's a type mismatch in some nested member called 'param_B',
the parser will print something like:

error: key 'tune_params.param_A.param_B' expected to be an integer

Which I suppose is similar to how you mentioned that
" In JSON property '/runs/0/results/0/level': "
is emitted during sarif parsing.

Like you mentioned earlier, I think adding location tracking would probably be
overkill though. Not only is the target demographic more advanced users, the
schema itself is fairly small and not that relatively nested. 

That being said, I would be happy to explore the location map more if it's
worth the implementation.  

> 
> As noted in a footnote in another reply, for sarif parsing, the user
> gets physical and logical locations within the JSON (and metadata about
> violations of the spec):
> 
> In JSON property '/runs/0/results/0/level':
> bad-level.sarif:12:20: error: unrecognized value for 'level': 'mostly 
> harmless' [SARIF v2.1.0 §3.27.10]
>   12 |           "level": "mostly harmless",
>      |                    ^~~~~~~~~~~~~~~~~
> 
> but libsarifreplay.cc here is using libgdiagnostics.h to manage things,
> rather than diagnostic-core.h (and thus has a different way of managing
> locations.  I have posted patches in the past that wired up the json
> parser to location_t and diagnostic-core.h, if that would be useful).
> 
>> To test these, we make use of a combination of regression tests (in
>> gcc.target/aarch64/aarch64-json-tunings/) as well as self-tests to
>> check the
>> contents of aarch64_tune_params during the GCC build.
>> 
>> ---
>> 
>> [4] Limitations:
>> 
>> Lack of comments in JSON:
>>      * JSON does not have the ability to store comments, which
>> leads to the
>>      loss of useful information that is provided in the form of
>> comments in
>>      the header files. A workaround is to have a dummy "comment"
>> key and
>>      ignore it when parsing. (e.g., "comment": "parameter
>> description")
> 
> Note that gcc/json-parser.cc can support reading C/C++-style comments
> if you enable a flag when creating the parser object, but these
> comments are discarded (not round-tripped like e.g. XML can support).
> They've helpful for DejaGnu tests of sarif-replay fwiw for verifying
> that it emits good error messages on bogus json inputs.
> 
> FWIW I have experimental code for writing out json::value trees as
> JSON5, which does support comments (also, for that matter, for CBOR,
> though that doesn't support comments).  I don't yet have parsing for
> JSON5.
> 

Thanks for pointing this out! I've updated the code to allow this. It would
allow users to document their tuning configurations inline, even if the comments
aren't preserved. 

>> 
>> No enum support in JSON:
>>      * The current workaround for this is to use strings instead
>> of enums,
>>      but we lose out on the ability to pass enum as values, as
>> well as doing
>>      bitwise operations on the enums, something used quite
>> frequently for
>>      some parameters.
> 
> Not sure if this is helpful, but I suppose the json could contain
> something like:
> 
> "prop" : { "binop": "or", "values": ["ENUM1", "ENUM2", "ENUM3"]}
> 
> to do the equivalent of (ENUM1 | ENUM2 | ENUM 3).  Though that might
> not be very amenable to schema-validation.

Yeah, that’s a nice suggestion. I think the issue with implementing it this way
(or any way that gives the user the ability to write the enums directly) is
that the tunings parser would need a way of mapping “ENUM1", “ENUM2".. from
strings to the corresponding enum. We do this with other enums but this specific
case is for extra tuning features, which are much more in number, and also keep
growing. 

If a user wanted to introduce a new feature, they would also need to have
additional mapping for it in the JSON parser, and I suppose that overhead would
not be worth having this. 

>> 
>> No type distinction in JSON:
>>      * JSON uses the "number" type which allows signed and
>> unsigned integers
>>      as well as floats but provides no distinction between them.
> 
> Yes, this is a pain.
> 
>> 
>> Storing the JSON schema:
>>      * The JSON schema is currently stored as a raw JSON string
>> in
>>      aarch64-json-schema.h. This is helpful in exposing the file
>> to the
>>      testing framework, but is not the cleanest solution.
>> 
>>      * Theoretically, the schema could be stored in the
>> installation
>>      directory, but this interferes with the idea of having self-
>> tests for
>>      the JSON parser.
> 
> How are the self-tests using the schema?

The self-tests call the parsing infrastructure which uses the schema to validate
the provided JSON input. The JSON input is provided as a raw string in the
self tests.

Technically, a "reduced" schema for validating the individual tests could also
just be passed as a raw string but I suppose this issue would again present
itself when we check for warnings and errors in the test suite (against the
schema).

>> 
>> Maintaing the printer/parser routines and JSON schema:
>>      * Any change in the aarch64 tuning format will result in the
>> need for
>>      manual changes to be made to the routines for the JSON
>> tunings printer,
>>      parser, and schema.
>> 
> 
> Thanks; hope this is constructive
> Dave


Reply via email to