Thread (8 messages) 8 messages, 2 authors, 2014-03-31

Re: [PATCH 1/4] brcm80211: deinline brcmf_chip_cr4_enterdl, save 440 bytes

From: Arend van Spriel <hidden>
Date: 2014-03-31 13:43:10
Also in: lkml

On 31/03/14 13:18, Denys Vlasenko wrote:
On 03/31/2014 09:38 AM, Arend van Spriel wrote:
quoted
On 30/03/14 23:31, Denys Vlasenko wrote:
quoted
Automated script discovered that without forced inlining,
gcc-4.7 generates smaller code for this function.

There is no need to declare static functions inline anyway:
nowadays gcc detects single-callsite static functions
which benefit from inlining.
These patches look awfully familiar. I tend to object, but I don't know the details of this automated script.
The script removes "static" keyword, recompiles the .c file,
compares the sizes, and if code size went down,
creates a patch
quoted
How about execution time or is this only compile tested?
The change adds one pair of call/return instructions -
probably around 5-10 CPU cycles.

The function in question is a part of firmware download logic,
which is nowhere near being hot path/.
True. My remarks are on all four patches and I just replied to the first 
patch. The other patches are in interrupt handling code, ie. interrupt 
or bottom-halve context.
quoted
The other thing is that you seem to rely on a specific gcc version.
What about pre-4.7? How about different architectures.
Was this determined on x86, arm, sparc, mips.
All these questions make me say 'nay'.
Not making functions inline unless there is a good reason
is a general good coding practice. It is not a compiler-
or architecture-specific optimization.
Agree, but you seem to assume that in this case there is no good reason.

Regards,
Arend

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help