On Wed, Aug 12, 2026 at 6:03 PM Jeffrey Law <[email protected]> wrote: > > > > On 7/29/2026 4:07 PM, Alfie Richards wrote: > > On 29/07/2026 15:01, Jovan Dmitrovic wrote: > >> Hello everyone, > >> > >> I have revised this patch series, after some careful consideration. > >> As 1/3 from v2 has merged (and not included in this series), > >> I have switched up the order of patches, so 1/3 in v3 is 3/3 from v2. > >> Other two patches are been a rework of 2/3 from v2, split into two > >> parts. > >> > >> I've decided upon removing target_reg_cost from estimate > >> of register pressure, as it doesn't seem to benefit the overall > >> calculation (most likely because we cannot predict at this stage > >> how the moving between registers will occur). > >> > >> The third patch is just scaling of the spill cost done to be closer > >> to what target_spill_cost actually is (cost of loading+storing from > >> stack). > >> > >> Kind reminder for Alfie to run his benchmarks on ARM, > >> so that we have info if there is any change. > > > > Hi Jovan, > > > > Thank you for the reminder, > > > > These numbers are for both spec v6 and spec v8 on an aarch64 > > neoverse-v2 core at -mcpu=native -Ofast -flto=auto averaged over 3 runs. > > > > The statistically significant changes in number of cycles are: > > > > Spec v6 > > - blender_r: +1.47% > > - fotonik3d_r: +1.73% > > - ompnetpp_r: +0.65% > > Overall: +0.20% cycles > > > > Spec v8 > > - flightdm_r: +0.57% > > - omnetpp_r: -1.38% > > - gem5_r: -0.42% > > Overall: -0.08% cycles > > > > (this is compared to current trunk, which already has the first patch) > > > > From a brief visual analysis the perf data and code gen from fotonik, > > the regression seems to come from a change in a hot loop. > > Before these changes there were no spills to the stack in the loop > > body. Afterwards there seem to be quite a few. > > Otherwise code gen looks largely similar. > This data would seem to indicate this patch series is a regression on > that design. Given Richi has ACK'd the patches that leaves us in a bit > of a bind. Do we go forward knowing about the likely performance > regression, do we further break down the performance testing to try and > show that one patch (ideally the last) is the source of the regression, etc. > > Looking for guidance from Richi, Jovan and Alfie here...
Given IVOPTs is essentially unmaintained we can either avoid touching it at all, or make sure to make it more maintainable when touching it, by a) reducing code, b) writing code that we can understand. I think the existing code is very much GIGO - GIGO that might "work", but that we shouldn't "tweak" to be different GIGO. IMO the proposed changes make sense. If there's regressions we have to analyze what now goes wrong. That said, I welcome comments on the end result of the changes - please point out parts that still do not make sense or point out where the original code made more sense. Adding explaining comments can also make code more maintainable. Richard. > > jeff
