Hi Richard, Gentle ping :)
Thanks, Soumya > On 18 Jul 2025, at 10:58 AM, Soumya AR <soum...@nvidia.com> wrote: > > Hi Richard, > Thank you for these suggestions! I’m really sorry about the delay in getting > back to you. > > I now have updated patches for this here: > https://gcc.gnu.org/pipermail/gcc-patches/2025-July/689893.html > > >> On 6 May 2025, at 4:04 PM, Richard Sandiford <richard.sandif...@arm.com> >> wrote: >> >> External email: Use caution opening links or attachments >> >> >> <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? >> > I took some time in refactoring my approach so it would be better suited for > auto-generation. Turned out to be much more frustrating than I expected, but I > have it in the updated patch now. > > Since the schema does not provide any information about the structure types, > the > new approach uses templates for all the routines. > > Now, if anyone were to update tune_params, they would only need to update the > schema alongside, and regenerate the printing/parsing routines using the > script. > I had initially hooked up the script to automatically run when the schema is > updated but this would cause a build dependency on Python, so I’m sticking > to manual invocation for now. > > As for structure definitions, like it was discussed later in the thread, I > don't see a clean way yet to handle those... > >> 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. >> >> 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. >> > > I naively thought that this change would be easy enough to implement but ran > into one major issue — since most nested structures in tune_params are > pointers > and not embedded directly, if we generate temporaries and remap the pointers > to > point to these temps, when and where do we free up the memory? > > Possible solutions: > 1. Simply don't bother freeing up the memory and let the memory leak be :) > 2. Use the GCC garbage collector > 3. Remove the pointers and have all the nested structures be embedded since > I'm not too sure why this is inconsistent in tune_params. > 4. (The current approach) Have each temporary structure be a static local > variable. This works, but operates under the assumption that nested > structures > in tune_params will always have unique types, since the variable declaration > is > a template that is resolved by the type of the nested structures. There is an > extra check added to ensure we don't overwrite accidentally, in the case we > ever end up having structure members of the same type. > > Let me know what you think is the best way to go about this. > > Thanks, > Soumya > >> Thanks, >> Richard
