Messages in this thread | | | Date | Fri, 25 Jun 2021 11:30:58 +0100 | From | Ionela Voinescu <> | Subject | Re: [PATCH V3 2/4] cpufreq: cppc: Pass structure instance by reference |
| |
Hey,
On Thursday 24 Jun 2021 at 07:52:52 (+0530), Viresh Kumar wrote: > On 23-06-21, 14:45, Ionela Voinescu wrote: > > On Monday 21 Jun 2021 at 14:49:35 (+0530), Viresh Kumar wrote: > > > Don't pass structure instance by value, pass it by reference instead. > > > > > > > Might be best to justify the change a bit :) > > I had it and removed later as I thought it would be obvious :) > > > For me this is a judgement call, and I don't really see the reasons for > > changing it: we don't care if the structure is modified or not, as we're > > not reusing the data after the call to cppc_get_rate_from_fbctrs(). > > More so, in this scenario we might not even want for the called function > > to modify the counter values. Also there is no further call to a function > > in cppc_get_rate_from_fbctrs(), that might require references to the > > fb_ctrs. > > > > So what is the reason behind this change? > > How about this commit log then: > > Theoretically speaking, call by reference is cheaper/faster than call by value > for structures as the later requires the compiler to make a new copy of the > whole structure (which has four u64 values here), to be used by the called > function, while with call by reference we just need to pass a single pointer > (u64 on 64-bit architectures) to the existing structure. > > Yes, on modern architectures, the compilers will likely end up using the > processor registers for passing this structure as it isn't doesn't have lot of > fields and it shouldn't be bad eventually, but nevertheless the code should do > the right thing without assuming about the compiler's or architecture's > optimizations. >
Yes, that's why "judgement call", which I'll let you make. The code is sane and I like the longer commit message.
Reviewed-by: Ionela Voinescu <ionela.voinescu@arm.com>
> Don't pass structure instance by value, pass it by reference instead. > > -- > viresh
| |