[PATCH] linux/kernel.h linux/device.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[PATCH] linux/kernel.h linux/device.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[Joe Perches]

When DEBUG is not defined, pr_debug and dev_dbg and some
other local debugging functions are specified as:

"inline __attribute__((format (printf, x, y)))"

This is done to validate printk arguments when not debugging.

Converting these functions to macros or statement expressions
"do { if (0) printk(fmt, ##arg); } while (0)"
or
"({ if (0) printk(fmt, ##arg); 0; })
makes at least gcc 4.2.2 produce smaller objects.

This has the additional benefit of allowing the optimizer to
avoid calling functions like print_mac that might have been
arguments to the printk.

defconfig x86 current:

$ size vmlinux
   text    data     bss     dec     hex filename
4716770  474560  618496 5809826  58a6a2 vmlinux

all converted: (More patches follow)

$ size vmlinux
   text    data     bss     dec     hex filename
4716642  474560  618496 5809698  58a622 vmlinux

Even kernel/sched.o, which doesn't even use these
functions, becomes smaller.

It appears that merely having an indirect include
of <linux/device.h> can cause bigger objects.

$ size sched.inline.o sched.if0.o
   text    data     bss     dec     hex filename
  31385    2854     328   34567    8707 sched.inline.o
  31366    2854     328   34548    86f4 sched.if0.o

The current preprocessed only kernel/sched.i file contains:

# 612 "include/linux/device.h"
static inline __attribute__((always_inline)) int __attribute__ ((format (printf, 2, 3)))
dev_dbg(struct device *dev, const char *fmt, ...)
{
 return 0;
}
# 628 "include/linux/device.h"
static inline __attribute__((always_inline)) int __attribute__ ((format (printf, 2, 3)))
dev_vdbg(struct device *dev, const char *fmt, ...)
{
 return 0;
}

Removing these unused inlines from sched.i shrinks sched.o

Signed-off-by: Joe Perches <joe@perches.com>

 include/linux/kernel.h           |    6 ++----
 include/linux/device.h           |   15 +++++----------

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 2df44e7..cd6d02c 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -293,10 +293,8 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 #define pr_debug(fmt, arg...) \
 	printk(KERN_DEBUG fmt, ##arg)
 #else
-static inline int __attribute__ ((format (printf, 1, 2))) pr_debug(const char * fmt, ...)
-{
-	return 0;
-}
+#define pr_debug(fmt, arg...) \
+	({ if (0) printk(KERN_DEBUG fmt, ##arg); 0; })
 #endif
 
 /*
diff --git a/include/linux/device.h b/include/linux/device.h
index 2258d89..12186e5 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -608,21 +608,16 @@ extern const char *dev_driver_string(struct device *dev);
 #define dev_dbg(dev, format, arg...)		\
 	dev_printk(KERN_DEBUG , dev , format , ## arg)
 #else
-static inline int __attribute__ ((format (printf, 2, 3)))
-dev_dbg(struct device *dev, const char *fmt, ...)
-{
-	return 0;
-}
+#define dev_dbg(dev, format, arg...)		\
+	({ if (0) dev_printk(KERN_DEBUG, dev, format, ##arg); 0; })
 #endif
 
 #ifdef VERBOSE_DEBUG
 #define dev_vdbg	dev_dbg
 #else
-static inline int __attribute__ ((format (printf, 2, 3)))
-dev_vdbg(struct device *dev, const char *fmt, ...)
-{
-	return 0;
-}
+
+#define dev_vdbg(dev, format, arg...)		\
+	({ if (0) dev_printk(KERN_DEBUG, dev, format, ##arg); 0; })
 #endif
 
 /* Create alias, so I can be autoloaded. */

[PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[Joe Perches]

Converting inline __attribute__((format (printf,x,y) functions
to macros or statement expressions produces smaller objects

before:
$ size vmlinux
   text    data     bss     dec     hex filename
4716770  474560  618496 5809826  58a6a2 vmlinux
after:
$ size vmlinux
   text    data     bss     dec     hex filename
4716706  474560  618496 5809762  58a662 vmlinux

Signed-off-by: Joe Perches <joe@perches.com>

 include/linux/fs.h |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index b84b848..a0ba590 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2055,11 +2055,10 @@ static struct file_operations __fops = {				\
 	.write	 = simple_attr_write,					\
 };
 
-static inline void __attribute__((format(printf, 1, 2)))
-__simple_attr_check_format(const char *fmt, ...)
-{
-	/* don't do anything, just let the compiler check the arguments; */
-}
+/* don't do anything, just let the compiler check the arguments; */
+
+#define __simple_attr_check_format(fmt, args...) \
+	do { if (0) printk(fmt, ##args); } while (0)
 
 int simple_attr_open(struct inode *inode, struct file *file,
 		     int (*get)(void *, u64 *), int (*set)(void *, u64),

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[Matthew Wilcox]

On Tue, Feb 26, 2008 at 08:02:27PM -0800, Joe Perches wrote:
> Converting inline __attribute__((format (printf,x,y) functions
> to macros or statement expressions produces smaller objects
> 
> before:
> $ size vmlinux
>    text    data     bss     dec     hex filename
> 4716770  474560  618496 5809826  58a6a2 vmlinux
> after:
> $ size vmlinux
>    text    data     bss     dec     hex filename
> 4716706  474560  618496 5809762  58a662 vmlinux

> -static inline void __attribute__((format(printf, 1, 2)))
> -__simple_attr_check_format(const char *fmt, ...)
> -{
> -	/* don't do anything, just let the compiler check the arguments; */
> -}
> +/* don't do anything, just let the compiler check the arguments; */
> +
> +#define __simple_attr_check_format(fmt, args...) \
> +	do { if (0) printk(fmt, ##args); } while (0)

That's very interesting.  It's only 64 bytes, but still, it's not
supposed to have any different effect.  Could you distill a test case
for the GCC folks and file it in their bugzilla?

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[Joe Perches]

On Tue, 2008-02-26 at 21:13 -0700, Matthew Wilcox wrote:
> That's very interesting.  It's only 64 bytes, but still, it's not
> supposed to have any different effect.

Especially because __simple_attr_check_format is not even used
or called in an x86 defconfig.  It's powerpc/cell specific.

> Could you distill a test case for the GCC folks

I'll play around with it.

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[David Rientjes]

On Tue, 26 Feb 2008, Matthew Wilcox wrote:

> On Tue, Feb 26, 2008 at 08:02:27PM -0800, Joe Perches wrote:
> > Converting inline __attribute__((format (printf,x,y) functions
> > to macros or statement expressions produces smaller objects
> > 
> > before:
> > $ size vmlinux
> >    text    data     bss     dec     hex filename
> > 4716770  474560  618496 5809826  58a6a2 vmlinux
> > after:
> > $ size vmlinux
> >    text    data     bss     dec     hex filename
> > 4716706  474560  618496 5809762  58a662 vmlinux
> 
> > -static inline void __attribute__((format(printf, 1, 2)))
> > -__simple_attr_check_format(const char *fmt, ...)
> > -{
> > -	/* don't do anything, just let the compiler check the arguments; */
> > -}
> > +/* don't do anything, just let the compiler check the arguments; */
> > +
> > +#define __simple_attr_check_format(fmt, args...) \
> > +	do { if (0) printk(fmt, ##args); } while (0)
> 
> That's very interesting.  It's only 64 bytes, but still, it's not
> supposed to have any different effect.  Could you distill a test case
> for the GCC folks and file it in their bugzilla?
> 

I'm not seeing any change in text size with allyesconfig after applying 
this patch with latest git:

   text	   data	    bss	    dec	    hex	filename
32696210	5021759	6735572	44453541	2a64ea5	vmlinux.before
32696210	5021759	6735572	44453541	2a64ea5	vmlinux.after

Joe, what version of gcc are you using?

		David

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[Joe Perches]

On Tue, 2008-02-26 at 21:44 -0800, David Rientjes wrote:
> I'm not seeing any change in text size with allyesconfig after applying 
> this patch with latest git:

This is just x86 defconfig

> Joe, what version of gcc are you using?

$ gcc --version
gcc (GCC) 4.2.2 20071128 (prerelease) (4.2.2-3.1mdv2008.0)

It's definitely odd.
The .o size changes are inconsistent.
Some get bigger, some get smaller.

The versioning ones I understand but I have no idea why
changes in drivers/ or mm/ or net/ exist.

I think it's gcc optimization changes, but dunno...
Any good ideas?

$ git reset --hard
HEAD is now at 7704a8b... Merge branch 'for-linus' of git://oss.sgi.com:8090/xfs/xfs-2.6
$ make mrproper ; make defconfig ; make > /dev/null
$ size vmlinux
   text    data     bss     dec     hex filename
4716770  474560  618496 5809826  58a6a2 vmlinux
$ size $(find -type f -print | grep "\.o$" | grep -vP "(vmlinux|built-in|piggy|allsyms.)\.o$") > size.default
$ patch -p1 < inline/fs.h.d
$ make > /dev/null
$ size vmlinux
   text    data     bss     dec     hex filename
4716706  474560  618496 5809762  58a662 vmlinux
$ size $(find -type f -print | grep "\.o$" | grep -vP "(vmlinux|built-in|piggy|allsyms.)\.o$") > size.inline_fs
$ diff --unified=0 size.default size.inline_fs
--- size.default        2008-02-26 22:18:33.000000000 -0800
+++ size.inline_fs      2008-02-26 22:33:27.000000000 -0800
@@ -21 +21 @@
-     79              0       0      79      4f ./arch/x86/boot/version.o
+     85              0       0      85      55 ./arch/x86/boot/version.o
@@ -335 +335 @@
-   5206             72      12    5290    14aa ./drivers/base/core.o
+   5201             72      12    5285    14a5 ./drivers/base/core.o
@@ -374 +374 @@
-  18192            104    1648   19944    4de8 ./drivers/char/tty_io.o
+  18184            104    1648   19936    4de0 ./drivers/char/tty_io.o
@@ -390 +390 @@
-   4293            560      24    4877    130d ./drivers/char/hpet.o
+   4287            560      24    4871    1307 ./drivers/char/hpet.o
@@ -473 +473 @@
-  38914             32     341   39287    9977 ./drivers/message/fusion/mptbase.o
+  38922             32     341   39295    997f ./drivers/message/fusion/mptbase.o
@@ -492 +492 @@
-  81665           2613       4   84282   1493a ./drivers/net/tg3.o
+  81659           2613       4   84276   14934 ./drivers/net/tg3.o
@@ -544 +544 @@
-  17508            845     552   18905    49d9 ./drivers/scsi/aic7xxx/aic79xx_osm.o
+  17510            845     552   18907    49db ./drivers/scsi/aic7xxx/aic79xx_osm.o
@@ -581 +581 @@
-     74           4480       0    4554    11ca ./drivers/scsi/scsi_wait_scan.mod.o
+     80           4480       0    4560    11d0 ./drivers/scsi/scsi_wait_scan.mod.o
@@ -774 +774 @@
-   1924              4       4    1932     78c ./fs/proc/kcore.o
+   1922              4       4    1930     78a ./fs/proc/kcore.o
@@ -776 +776 @@
-  41462            652      80   42194    a4d2 ./fs/proc/proc.o
+  41458            652      80   42190    a4ce ./fs/proc/proc.o
@@ -828 +828 @@
-   9583             80       0    9663    25bf ./fs/locks.o
+   9571             80       0    9651    25b3 ./fs/locks.o
@@ -870 +870 @@
-    277            396       4     677     2a5 ./init/version.o
+    281            396       4     681     2a9 ./init/version.o
@@ -926 +926 @@
-   8379            460       8    8847    228f ./kernel/sys.o
+   8381            460       8    8849    2291 ./kernel/sys.o
@@ -954 +954 @@
-  13337            188      73   13598    351e ./kernel/module.o
+  13341            188      73   13602    3522 ./kernel/module.o
@@ -1044 +1044 @@
-   1845              0       0    1845     735 ./mm/mremap.o
+   1841              0       0    1841     731 ./mm/mremap.o
@@ -1052 +1052 @@
-   8781             44    2196   11021    2b0d ./mm/swapfile.o
+   8777             44    2196   11017    2b09 ./mm/swapfile.o
@@ -1065 +1065 @@
-   2630              0       0    2630     a46 ./net/core/datagram.o
+   2631              0       0    2631     a47 ./net/core/datagram.o
@@ -1101 +1101 @@
-  13190             24       0   13214    339e ./net/ipv4/tcp_output.o
+  13192             24       0   13216    33a0 ./net/ipv4/tcp_output.o
@@ -1109 +1109 @@
-   6244            468       0    6712    1a38 ./net/ipv4/arp.o
+   6239            468       0    6707    1a33 ./net/ipv4/arp.o
@@ -1138 +1138 @@
-   4660            132      44    4836    12e4 ./net/ipv6/ip6_fib.o
+   4644            132      44    4820    12d4 ./net/ipv6/ip6_fib.o
@@ -1146 +1146 @@
-  16397             24       4   16425    4029 ./net/ipv6/mcast.o
+  16399             24       4   16427    402b ./net/ipv6/mcast.o
@@ -1159 +1159 @@
- 143799           7424    3036  154259   25a93 ./net/ipv6/ipv6.o
+ 143787           7424    3036  154247   25a87 ./net/ipv6/ipv6.o
@@ -1202 +1202 @@
-   2109            600       0    2709     a95 ./net/xfrm/xfrm_algo.o
+   2111            600       0    2711     a97 ./net/xfrm/xfrm_algo.o

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[David Rientjes]

On Tue, 26 Feb 2008, Joe Perches wrote:

> > I'm not seeing any change in text size with allyesconfig after applying 
> > this patch with latest git:
> 
> This is just x86 defconfig
> 

allyesconfig should be able to capture any text savings that this patch 
offers.

> > Joe, what version of gcc are you using?
> 
> $ gcc --version
> gcc (GCC) 4.2.2 20071128 (prerelease) (4.2.2-3.1mdv2008.0)
> 

My x86_64 defconfig with gcc 4.0.3 had no difference in text size after 
applying your patch, yet the same config on gcc 4.1.2 did:

	   text	   data	    bss	    dec	    hex	filename
	5386112	 846328	 719560	6952000	 6a1440	vmlinux.before
	5386048	 846328	 719560	6951936	 6a1400	vmlinux.after

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[David Rientjes]

On Tue, 26 Feb 2008, Joe Perches wrote:

> > Joe, what version of gcc are you using?
> 
> $ gcc --version
> gcc (GCC) 4.2.2 20071128 (prerelease) (4.2.2-3.1mdv2008.0)
> 
> It's definitely odd.
> The .o size changes are inconsistent.
> Some get bigger, some get smaller.
> 
> The versioning ones I understand but I have no idea why
> changes in drivers/ or mm/ or net/ exist.
> 

When I did the same comparisons on my x86_64 defconfig with gcc 4.1.3, I 
only saw differences in drivers/ and fs/.

> I think it's gcc optimization changes, but dunno...
> Any good ideas?
> 

What's interesting about this is that it doesn't appear to be related to 
your change (static inline function to macro definition).  It appears to 
be simply removing the static inline function.

The only reference to __simple_attr_check_format() in either the x86 or 
x86_64 defconfig is via DEFINE_SIMPLE_ATTRIBUTE() in fs/debugfs/file.c.

If you remove the only reference to it:

diff --git a/include/linux/fs.h b/include/linux/fs.h
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2044,7 +2044,6 @@ static inline void simple_transaction_set(struct file *file, size_t n)
 #define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt)		\
 static int __fops ## _open(struct inode *inode, struct file *file)	\
 {									\
-	__simple_attr_check_format(__fmt, 0ull);			\
 	return simple_attr_open(inode, file, __get, __set, __fmt);	\
 }									\
 static struct file_operations __fops = {				\

The text size remains the same:

   text    data     bss     dec     hex filename
5386111  846328  719560 6951999  6a143f vmlinux.before
5386111  846328  719560 6951999  6a143f vmlinux.after

Yet if you remove the reference _and_ the static inline function itself, 
replacing it with nothing:

diff --git a/include/linux/fs.h b/include/linux/fs.h
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2044,7 +2044,6 @@ static inline void simple_transaction_set(struct file *file, size_t n)
 #define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt)		\
 static int __fops ## _open(struct inode *inode, struct file *file)	\
 {									\
-	__simple_attr_check_format(__fmt, 0ull);			\
 	return simple_attr_open(inode, file, __get, __set, __fmt);	\
 }									\
 static struct file_operations __fops = {				\
@@ -2055,12 +2054,6 @@ static struct file_operations __fops = {				\
 	.write	 = simple_attr_write,					\
 };
 
-static inline void __attribute__((format(printf, 1, 2)))
-__simple_attr_check_format(const char *fmt, ...)
-{
-	/* don't do anything, just let the compiler check the arguments; */
-}
-
 int simple_attr_open(struct inode *inode, struct file *file,
 		     int (*get)(void *, u64 *), int (*set)(void *, u64),
 		     const char *fmt);

The text size does become smaller:

   text    data     bss     dec     hex filename
5386111  846328  719560 6951999  6a143f vmlinux.before
5386047  846328  719560 6951935  6a13ff vmlinux.after

gcc 4.0.3 maintains the same text size for both cases, while it appears 
gcc 4.1.3 and your version, 4.2.2, have this different behavior.

		David

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[Jan Hubicka]

>  
> -static inline void __attribute__((format(printf, 1, 2)))
> -__simple_attr_check_format(const char *fmt, ...)

It would be nice to have a testcase, but I guess it is because GCC can't
inline variadic functions.  The function gets identified as const and
removed as unused by DCE, but this happens later (that is after early
inlining and before real inlining).  GCC 4.0.3 didn't have early inliner
so it is probably where the difference is comming from.

One possibility to handle this side case would be to mark const
functions early during early optimization and only refine it using
Kenny's existing IPA pass that should turn this issue into no-op.

We probably also can simply allow inlining variadic functions not
calling va_start.  I must say that this option appeared to me but I was
unable to think of any sane use case.  This probably is one ;)

Honza
> -{
> -	/* don't do anything, just let the compiler check the arguments; */
> -}
> -
>  int simple_attr_open(struct inode *inode, struct file *file,
>  		     int (*get)(void *, u64 *), int (*set)(void *, u64),
>  		     const char *fmt);
> 
> The text size does become smaller:
> 
>    text    data     bss     dec     hex filename
> 5386111  846328  719560 6951999  6a143f vmlinux.before
> 5386047  846328  719560 6951935  6a13ff vmlinux.after
> 
> gcc 4.0.3 maintains the same text size for both cases, while it appears 
> gcc 4.1.3 and your version, 4.2.2, have this different behavior.
> 
> 		David

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[David Rientjes]

On Thu, 28 Feb 2008, Jan Hubicka wrote:

> > -static inline void __attribute__((format(printf, 1, 2))) 
> > -__simple_attr_check_format(const char *fmt, ...)
> 
> It would be nice to have a testcase, but I guess it is because GCC can't
> inline variadic functions.  The function gets identified as const and
> removed as unused by DCE, but this happens later (that is after early
> inlining and before real inlining).  GCC 4.0.3 didn't have early inliner
> so it is probably where the difference is comming from.
> 
> One possibility to handle this side case would be to mark const
> functions early during early optimization and only refine it using
> Kenny's existing IPA pass that should turn this issue into no-op.
> 
> We probably also can simply allow inlining variadic functions not
> calling va_start.  I must say that this option appeared to me but I was
> unable to think of any sane use case.  This probably is one ;)
> 

The only testcase I can identify is the current version of Linux, because 
I've certainly never seen this type of behavior before.

What's interesting is that you can remove __simple_attr_check_format() 
completely and replace it with an empty function definition:

	static inline void completely_useless_function(void)
	{
	}

that is not referenced anywhere in the tree:

diff --git a/include/linux/fs.h b/include/linux/fs.h
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2044,7 +2044,6 @@ static inline void simple_transaction_set(struct file *file, size_t n)
 #define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt)           \
 static int __fops ## _open(struct inode *inode, struct file *file)     \
 {                                                                      \
-       __simple_attr_check_format(__fmt, 0ull);                        \
        return simple_attr_open(inode, file, __get, __set, __fmt);      \
 }                                                                      \
 static struct file_operations __fops = {                               \
@@ -2055,10 +2054,8 @@ static struct file_operations __fops = {                         \
        .write   = simple_attr_write,                                   \
 };
 
-static inline void __attribute__((format(printf, 1, 2)))
-__simple_attr_check_format(const char *fmt, ...)
+static inline void completely_useless_function(void)
 {
-       /* don't do anything, just let the compiler check the arguments; */
 }
 
 int simple_attr_open(struct inode *inode, struct file *file,


And the size remains identical after this patch has been applied:

   text    data     bss     dec     hex filename
5386111  846328  719560 6951999  6a143f vmlinux.before
5386111  846328  719560 6951999  6a143f vmlinux.after

Yet, if you subsequently remove completely_useless_function(), the image 
is smaller:


diff --git a/include/linux/fs.h b/include/linux/fs.h
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2044,7 +2044,6 @@ static inline void simple_transaction_set(struct file *file, size_t n)
 #define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt)           \
 static int __fops ## _open(struct inode *inode, struct file *file)     \
 {                                                                      \
-       __simple_attr_check_format(__fmt, 0ull);                        \
        return simple_attr_open(inode, file, __get, __set, __fmt);      \
 }                                                                      \
 static struct file_operations __fops = {                               \
@@ -2055,12 +2054,6 @@ static struct file_operations __fops = {                         \
        .write   = simple_attr_write,                                   \
 };
 
-static inline void __attribute__((format(printf, 1, 2)))
-__simple_attr_check_format(const char *fmt, ...)
-{
-       /* don't do anything, just let the compiler check the arguments; */
-}
-
 int simple_attr_open(struct inode *inode, struct file *file,
                     int (*get)(void *, u64 *), int (*set)(void *, u64),
                     const char *fmt);


Then:

   text    data     bss     dec     hex filename
5386111  846328  719560 6951999  6a143f vmlinux.before
5386047  846328  719560 6951935  6a13ff vmlinux.after

So the only thing I can imagine is that gcc 4.1.3 and gcc 4.2.2 emit a 
larger text size simply based on the number of functions being defined, 
regardless of whether they are referenced in the source or not.  gcc 4.0.3 
doesn't have this behavior.

It's easy to reproduce:

	$ wget http://www.kernel.org/pub/linux/kernel/v2.6/linux-2.6.24.3.tar.bz2
	$ tar xvf linux-2.6.24.3.tar.bz2
	$ cd linux-2.6.24.3
	$ make defconfig ARCH=x86_64
	$ make ARCH=x86_64
	$ size vmlinux
	$ patch -p1 < remove-simple-attr-check-format.patch
	$ make ARCH=x86_64
	$ size vmlinux

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[Jakub Jelinek]

On Thu, Feb 28, 2008 at 12:58:35AM +0100, Jan Hubicka wrote:
> We probably also can simply allow inlining variadic functions not
> calling va_start.  I must say that this option appeared to me but I was
> unable to think of any sane use case.  This probably is one ;)

We already allow inlining variadic functions not calling va_start, already
3.2.x did that and so do all following gccs.  In 4.3+
__builtin_va_arg_pack{,_len} support was added, so that you can even pass
the ... arguments to variable length functions, query their count etc.

	Jakub

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[Jan Hubicka]

> On Thu, Feb 28, 2008 at 12:58:35AM +0100, Jan Hubicka wrote:
> > We probably also can simply allow inlining variadic functions not
> > calling va_start.  I must say that this option appeared to me but I was
> > unable to think of any sane use case.  This probably is one ;)
> 
> We already allow inlining variadic functions not calling va_start, already
> 3.2.x did that and so do all following gccs.  In 4.3+
> __builtin_va_arg_pack{,_len} support was added, so that you can even pass
> the ... arguments to variable length functions, query their count etc.

Hmm, I now remember this change. Thanks for pointing it out ;) I guess
it is just more or less random difference then (ie different ordering of
hashtables). The difference is tiny anyway.  The call ought to be always
early inlined and not seen by any optimization pass.

Honza
> 
> 	Jakub

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[Joe Perches]

On Thu, 2008-02-28 at 11:23 +0100, Jan Hubicka wrote:
> The call ought to be always
> early inlined and not seen by any optimization pass.

The inlined functions don't actually appear in the generated code.

Look at the code generation differences for kernel/sched.c
function place_entity

$ size sched.inline.o sched.if0.o
   text    data     bss     dec     hex filename
  31385    2854     328   34567    8707 sched.inline.o
  31366    2854     328   34548    86f4 sched.if0.o

The current preprocessed only kernel/sched.i file contains:

# 612 "include/linux/device.h"
static inline __attribute__((always_inline)) int __attribute__ ((format (printf, 2, 3)))
dev_dbg(struct device *dev, const char *fmt, ...)
{
 return 0;
}
# 628 "include/linux/device.h"
static inline __attribute__((always_inline)) int __attribute__ ((format (printf, 2, 3)))
dev_vdbg(struct device *dev, const char *fmt, ...)
{
 return 0;
}

But the function place_entity doesn't use it directly or indirectly.
If the lines above are removed, the generated code for place_entity changes.

static void
place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int initial)
{
	u64 vruntime;

	vruntime = cfs_rq->min_vruntime;

	if (sched_feat(TREE_AVG)) {
		struct sched_entity *last = __pick_last_entity(cfs_rq);
		if (last) {
			vruntime += last->vruntime;
			vruntime >>= 1;
		}
	} else if (sched_feat(APPROX_AVG) && cfs_rq->nr_running)
		vruntime += sched_vslice(cfs_rq)/2;

	/*
	 * The 'current' period is already promised to the current tasks,
	 * however the extra weight of the new task will slow them down a
	 * little, place the new task so that it fits in the slot that
	 * stays open at the end.
	 */
	if (initial && sched_feat(START_DEBIT))
		vruntime += sched_vslice_add(cfs_rq, se);

	if (!initial) {
		/* sleeps upto a single latency don't count. */
		if (sched_feat(NEW_FAIR_SLEEPERS))
			vruntime -= sysctl_sched_latency;

		/* ensure we never gain time by being placed backwards. */
		vruntime = max_vruntime(se->vruntime, vruntime);
	}

	se->vruntime = vruntime;
}

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[Denys Vlasenko]

On Friday 29 February 2008 02:09, Joe Perches wrote:
> But the function place_entity doesn't use it directly or indirectly.
> If the lines above are removed, the generated code for place_entity changes.

I see it all the time. Whenever I add/remove/change something
to a header, some functions grow a tiny bit, some shrink a bit.
gcc 4.2.1. Report is here:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29950
--
vda

Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros

[Joe Perches]

On Sun, 2008-03-23 at 16:22 +0100, Denys Vlasenko wrote:
> On Friday 29 February 2008 02:09, Joe Perches wrote:
> > But the function place_entity doesn't use it directly or indirectly.
> > If the lines above are removed, the generated code for place_entity changes.
> I see it all the time. Whenever I add/remove/change something
> to a header, some functions grow a tiny bit, some shrink a bit.
> gcc 4.2.1. Report is here:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29950

This seems more like a machine descriptions or target pass defect
than an RTL problem.

Should this defect be classified in group "Target" rather than "RTL"?