On Wed, Aug 23, 2023 at 04:39:35PM +0000, Charles Keepax wrote:
> On Wed, Aug 23, 2023 at 05:19:31PM +0100, Mark Brown wrote:
> > On Wed, Aug 23, 2023 at 03:49:58PM +0000, Charles Keepax wrote:
> > > On Wed, Aug 23, 2023 at 10:21:13AM +0100, Charles Keepax wrote:
> > > > Commit ac950278b087 ("ASoC: add N cpus to M codecs dai link support")
> > > > added an additional local params in __soc_pmc_hw_params, for the
> > > > CPU side of the DAI. The snd_pcm_hw_params struct is pretty large (604
> > > > bytes) and keeping two local copies of it makes the stack frame for
> > > > __soc_pcm_hw_params really large. As the two copies are only used
> > > > sequentially combine these into a single local variable to shrink the
> > > > stack frame.
> >
> > > Hmm... this might need a little more thought its not clear why this
> > > should change the frame size and it only seems to change the frame
> > > size on the ARM cross compiler I am using, not x86.
> >
> > Isn't that just going to be a function of the compiler being smart
> > enough to work out that there aren't overlapping uses of the two
> > variables and they can share stack space? There's no reason not to help
> > it figure that out.
>
> Yeah I think my only concern here was I no longer was certain I
> understood what was happening. I don't think the patch can do any
> harm, well except for the names being slightly less clear in the
> code. It is starting to look like the mostly comes down to the
> compiler being smart enough, although both were GCC in my case
> so the difference is still a little surprising to me.
>
Ah ok I see what is going on here, it depends on if you have
-Os or -O2 set. -O2 will merge the two variables and give a
smaller stack frame, -Os does not.
I would be inclined to say merge the patch, since it does help
if some is trying to size optimise their kernel, but I don't
feel strongly. Also I could respin to put this in the commit
message if people prefer?
Thanks,
Charles
