In Hi Richard,
> On 6 May 2025, at 12:34, Richard Sandiford <richard.sandif...@arm.com> wrote:
>
> <soum...@nvidia.com> writes:
>> 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.
>
> Thanks for doing this. It looks really useful. My main question is:
> rather than write the parsing and printing routines by hand, could we
> generate the structure definitions, the parsing code, and the printing
> code from the schema?
>
> The schema would need to provide more information about the structures
> compared to the current one. The approach would also presumably need
> build/*.o versions of the json routines. But it seems like something
> that we might want to do elsewhere, so would be worth building a bit
> of infrastructure around. And it would reduce the maintenance burden
> associated with adding a new field or changing an existing one.
>
Thanks for your thoughts. I suspected that we may need something like that
eventually.
Hypothetically in the future we’d like to be able to batch up the various
generic —params in a JSON input file as well to help superoptimiser tools for
performance exploration purposes.
It looks like the parsing and printing code would be easy to autogenerate.
The structure definitions in aarch64-protos.h may be tricker. As long as they
are effectively containers of primitive data it should be okay, though
currently some extend others (like the issue info structs).
Haven’t thought it through completely yet.
Thanks,
Kyrill
> Much more minor, but: in patch 1, I'm all in favour of removing the
> "const"s from the field declarations, such as:
>
> struct scale_addr_mode_cost
> {
> - const int hi;
> - const int si;
> - const int di;
> - const int ti;
> + int hi;
> + int si;
> + int di;
> + int ti;
> };
>
> IMO those consts should never have been there. But can we keep the
> predefined tables themselves as const, without things like:
>
> -const struct cpu_cost_table tsv110_extra_costs =
> +struct cpu_cost_table tsv110_extra_costs =
>
> ? If we make any changes to the contents of these tables, it should
> IMO be done via temporaries instead.
>
> Thanks,
> Richard
