optimizing out inline functions

optimizing out inline functions

[Steve French]

In trying to remove some macros, I ran across another kernel style
question.  I see two ways that people try to let the compiler optimize
out unused code and would like to know which is preferred.  The first
example uses an empty inline function and trusts the compiler will
optimize it out.

#ifdef CONFIG_DEBUG_SOMETHING
static inline void some_debug_function(var1)
{
    something = var1;
    printk(some debug text);
}
#else
static inline void some_debug_function(var1)
{
   /* empty function */
}
#endif


alternatively I have seen places where people put a #define of do while 0, e.g.



#ifdef CONFIG_DEBUG_SOMETHING
static inline void some_debug_function(var1)
{
    something = var1;
    printk(some debug text);
}
#else
#define some_debug_function(var) do {} while (0)
#endif


Is one or the other style (with or without #define of empty function)
preferred?  Does the compiler optimize both #else clauses out
properly?  sparse and checkpatch seem to take either

-- 
Thanks,

Steve

Re: optimizing out inline functions

[Pekka Enberg]

On Wed, May 28, 2008 at 10:51 PM, Steve French <smfrench@gmail.com> wrote:
> Is one or the other style (with or without #define of empty function)
> preferred?  Does the compiler optimize both #else clauses out
> properly?  sparse and checkpatch seem to take either

Both are optimized out but empty function is preferred for type checking.

Re: optimizing out inline functions

[Sam Ravnborg]

On Wed, May 28, 2008 at 02:51:02PM -0500, Steve French wrote:
> In trying to remove some macros, I ran across another kernel style
> question.  I see two ways that people try to let the compiler optimize
> out unused code and would like to know which is preferred.  The first
> example uses an empty inline function and trusts the compiler will
> optimize it out.
> 
> #ifdef CONFIG_DEBUG_SOMETHING
> static inline void some_debug_function(var1)
> {
>     something = var1;
>     printk(some debug text);
> }
> #else
> static inline void some_debug_function(var1)
> {
>    /* empty function */
> }
> #endif

With reference to a recent thread about kconfig
I would prefer:
static inline void some_debug_function(var1)
{
	if (KCONFIG_DEBUG_SOMETHING) {
		something = var1;
		printk(some debug text);
	}
}


But we do not have KCONFIG_DEBUG_SOMETHING available
so the second best is to use an empty function
to keep the typechecking in place.

IIRC gcc optimize both away.

	Sam

Re: optimizing out inline functions

[James Kosin]

Sam Ravnborg wrote:
> On Wed, May 28, 2008 at 02:51:02PM -0500, Steve French wrote:
>> In trying to remove some macros, I ran across another kernel style
<<--SNIP-->>
> With reference to a recent thread about kconfig
> I would prefer:
> static inline void some_debug_function(var1)
> {
> 	if (KCONFIG_DEBUG_SOMETHING) {
> 		something = var1;
> 		printk(some debug text);
> 	}
> }
> 
> 
> But we do not have KCONFIG_DEBUG_SOMETHING available
> so the second best is to use an empty function
> to keep the typechecking in place.
> 
> IIRC gcc optimize both away.

Another way would be to have:

static inline void some_debug_function(var1)
{
    #ifdef KCONFIG_DEBUG_SOMETHING
       something = var1;
       printk(some debug text);
    #endif
}

BUT, this probably violates some styling rules.

James
}

Re: optimizing out inline functions

[Joe Perches]

On Thu, 2008-05-29 at 05:27 +0200, Johannes Weiner wrote:
> James Kosin <jkosin@beta.intcomgrp.com> writes:
> >> But we do not have KCONFIG_DEBUG_SOMETHING available
> >> so the second best is to use an empty function
> >> to keep the typechecking in place.
> >> IIRC gcc optimize both away.
> > Another way would be to have:
> > static inline void some_debug_function(var1)
> > {
> >    #ifdef KCONFIG_DEBUG_SOMETHING
> >       something = var1;
> >       printk(some debug text);
> >    #endif
> > }

A potential issue is a possible unnecessary call of any
function used as an argument to some_debug_function.

int unnecessary_debug_test(void)
{
	int foo;
	[]
	return foo;
}

static inline void some_debug_function(int bar)
{
}

some_debug_function(unnecessary_debug_test())

unnecessary_debug_test will still get called

Macro wrappers/statement expressions can be used to avoid
the function call.  see kernel.h/pr_debug for an example.

#ifdef DEBUG
/* If you are writing a driver, please use dev_dbg instead */
#define pr_debug(fmt, arg...) \
	printk(KERN_DEBUG fmt, ##arg)
#else
#define pr_debug(fmt, arg...) \
	({ if (0) printk(KERN_DEBUG fmt, ##arg); 0; })
#endif

Re: optimizing out inline functions

[Johannes Weiner]

Hi,

James Kosin <jkosin@beta.intcomgrp.com> writes:

> Sam Ravnborg wrote:
>> On Wed, May 28, 2008 at 02:51:02PM -0500, Steve French wrote:
>>> In trying to remove some macros, I ran across another kernel style
> <<--SNIP-->>
>> With reference to a recent thread about kconfig
>> I would prefer:
>> static inline void some_debug_function(var1)
>> {
>> 	if (KCONFIG_DEBUG_SOMETHING) {
>> 		something = var1;
>> 		printk(some debug text);
>> 	}
>> }
>>
>>
>> But we do not have KCONFIG_DEBUG_SOMETHING available
>> so the second best is to use an empty function
>> to keep the typechecking in place.
>>
>> IIRC gcc optimize both away.
>
> Another way would be to have:
>
> static inline void some_debug_function(var1)
> {
>    #ifdef KCONFIG_DEBUG_SOMETHING
>       something = var1;
>       printk(some debug text);
>    #endif
> }
>
> BUT, this probably violates some styling rules.

Without indenting the ifdefs, I think this solution is the best.

It gives you the advantages of type checking but saves a superfluous
prototype.

	Hannes

Re: optimizing out inline functions

[Andrew Morton]

On Wed, 28 May 2008 22:54:47 +0300 "Pekka Enberg" <penberg@cs.helsinki.fi> wrote:

> On Wed, May 28, 2008 at 10:51 PM, Steve French <smfrench@gmail.com> wrote:
> > Is one or the other style (with or without #define of empty function)
> > preferred?  Does the compiler optimize both #else clauses out
> > properly?  sparse and checkpatch seem to take either
> 
> Both are optimized out but empty function is preferred for type checking.

Plus the inlined function can help suppress unused-var warnings because
it counts as a "use".

Sometimes this works the other way and the argument to the macro/inline
just doesn't exist, in which case we're forced to use a macro for the stub.

Re: optimizing out inline functions

[James Kosin]

[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]

Johannes Weiner wrote:
> Hi,
>
> James Kosin <jkosin@beta.intcomgrp.com> writes:
>
>   
>> Sam Ravnborg wrote:
>>     
>>> On Wed, May 28, 2008 at 02:51:02PM -0500, Steve French wrote:
>>>       
>>>> In trying to remove some macros, I ran across another kernel style
>>>>         
>> <<--SNIP-->>
>>     
>>> With reference to a recent thread about kconfig
>>> I would prefer:
>>> static inline void some_debug_function(var1)
>>> {
>>> 	if (KCONFIG_DEBUG_SOMETHING) {
>>> 		something = var1;
>>> 		printk(some debug text);
>>> 	}
>>> }
>>>
>>>
>>> But we do not have KCONFIG_DEBUG_SOMETHING available
>>> so the second best is to use an empty function
>>> to keep the typechecking in place.
>>>
>>> IIRC gcc optimize both away.
>>>       
>> Another way would be to have:
>>
>> static inline void some_debug_function(var1)
>> {
>>    #ifdef KCONFIG_DEBUG_SOMETHING
>>       something = var1;
>>       printk(some debug text);
>>    #endif
>> }
>>
>> BUT, this probably violates some styling rules.
>>     
>
> Without indenting the ifdefs, I think this solution is the best.
>
> It gives you the advantages of type checking but saves a superfluous
> prototype.
>
> 	Hannes
>
>   
Actually, Joe Perches, gave a good reason for using the MACRO #define 
method; so, this could really turn into an interesting discussion.
Pros and Cons are always interesting when there is more than one way to 
do something.

James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 258 bytes --]

Re: optimizing out inline functions

[James Kosin]

[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]

Johannes Weiner wrote:
> Hi,
>
> James Kosin <jkosin@beta.intcomgrp.com> writes:
>
>   
>> Sam Ravnborg wrote:
>>     
>>> On Wed, May 28, 2008 at 02:51:02PM -0500, Steve French wrote:
>>>       
>>>> In trying to remove some macros, I ran across another kernel style
>>>>         
>> <<--SNIP-->>
>>     
>>> With reference to a recent thread about kconfig
>>> I would prefer:
>>> static inline void some_debug_function(var1)
>>> {
>>> 	if (KCONFIG_DEBUG_SOMETHING) {
>>> 		something = var1;
>>> 		printk(some debug text);
>>> 	}
>>> }
>>>
>>>
>>> But we do not have KCONFIG_DEBUG_SOMETHING available
>>> so the second best is to use an empty function
>>> to keep the typechecking in place.
>>>
>>> IIRC gcc optimize both away.
>>>       
>> Another way would be to have:
>>
>> static inline void some_debug_function(var1)
>> {
>>    #ifdef KCONFIG_DEBUG_SOMETHING
>>       something = var1;
>>       printk(some debug text);
>>    #endif
>> }
>>
>> BUT, this probably violates some styling rules.
>>     
>
> Without indenting the ifdefs, I think this solution is the best.
>
> It gives you the advantages of type checking but saves a superfluous
> prototype.
>
> 	Hannes
>
>   
Actually, Joe Perches, gave a good reason for using the MACRO #define 
method; so, this could really turn into an interesting discussion.
Pros and Cons are always interesting when there is more than one way to 
do something.

James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 258 bytes --]

Re: optimizing out inline functions

[Steve French]

Ran into one loosely related question, printk takes a variable
argument list, so the calling function in this case would also need to
be able to handle thos variable arguments.  With macros, we are able
to do things like with variable arguments easily

#define function_to_print_some_warning(format, arg...)
printk(KERN_WARNING ": " format "\n" , ## arg)

Are there style rules (or nicely written examples) for doing this
(variable argument lists) with (inline) functions

On Wed, May 28, 2008 at 3:00 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> On Wed, May 28, 2008 at 02:51:02PM -0500, Steve French wrote:
>> In trying to remove some macros, I ran across another kernel style
>> question.  I see two ways that people try to let the compiler optimize
>> out unused code and would like to know which is preferred.  The first
>> example uses an empty inline function and trusts the compiler will
>> optimize it out.
>>
>> #ifdef CONFIG_DEBUG_SOMETHING
>> static inline void some_debug_function(var1)
>> {
>>     something = var1;
>>     printk(some debug text);
>> }
>> #else
>> static inline void some_debug_function(var1)
>> {
>>    /* empty function */
>> }
>> #endif
>
> With reference to a recent thread about kconfig
> I would prefer:
> static inline void some_debug_function(var1)
> {
>        if (KCONFIG_DEBUG_SOMETHING) {
>                something = var1;
>                printk(some debug text);
>        }
> }
>
>
> But we do not have KCONFIG_DEBUG_SOMETHING available
> so the second best is to use an empty function
> to keep the typechecking in place.
>
> IIRC gcc optimize both away.
>
>        Sam
>



-- 
Thanks,

Steve

Re: optimizing out inline functions

[Sam Ravnborg]

On Thu, May 29, 2008 at 11:39:32AM -0500, Steve French wrote:
> Ran into one loosely related question, printk takes a variable
> argument list, so the calling function in this case would also need to
> be able to handle thos variable arguments.  With macros, we are able
> to do things like with variable arguments easily
> 
> #define function_to_print_some_warning(format, arg...)
> printk(KERN_WARNING ": " format "\n" , ## arg)
> 
> Are there style rules (or nicely written examples) for doing this
> (variable argument lists) with (inline) functions

Searching the code base I'm sure you will find plenty.
But I cannot point one out from the top of my head.

	Sam

Re: optimizing out inline functions

[Vegard Nossum]

On Thu, May 29, 2008 at 6:39 PM, Steve French <smfrench@gmail.com> wrote:
> Ran into one loosely related question, printk takes a variable
> argument list, so the calling function in this case would also need to
> be able to handle thos variable arguments.  With macros, we are able
> to do things like with variable arguments easily
>
> #define function_to_print_some_warning(format, arg...)
> printk(KERN_WARNING ": " format "\n" , ## arg)
>
> Are there style rules (or nicely written examples) for doing this
> (variable argument lists) with (inline) functions
>

Hm, are you referring to making a proxy inline function that takes a
variable number of arguments and calls another function with the same
(also variable number of) variables?

I don't think this is possible unless you want to use va_lists. And
that might not be desirable in an inline function, I don't know how
well gcc can optimize this away.

The canonical example, although not an inline function, is printk()
itself from kernel/printk.c, which simply calls vprintk().


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036