cvs commit: src/sys/kern init_main.c kern_malloc.c md5c.c subr_autoconf.c subr_mbuf.c subr_prf.c tty_subr.c vfs_cluster.c vfs_subr.c

[Bruce Evans]

On Tue, 22 Jul 2003, Bosko Milekic wrote:

>
> On Wed, Jul 23, 2003 at 12:54:22AM +1000, Bruce Evans wrote:
> > Inlining was broken in gcc-3.1 (2002/05/09).  subr_mbuf.c worked as intended
> > for almost a year until then.  subr_mbuf.c now doesn't even attempt to work
> > like its comments say it is intended to:
> >
> > % /******************************************************************************
> > %  * Internal routines.
> > %  *
> > %  * Because mb_alloc() and mb_free() are inlines (to keep the common
> > %  * cases down to a maximum of one function call), below are a few
> > %  * routines used only internally for the sole purpose of making certain
> > %  * functions smaller.
> > %  */
> >
> > But mb_alloc() and mb_free() aren't inlines...
> >
> > >   I guess GCC's warnings just got fixed recently.
> >
> > I suspect the warning got broken in gcc-3.1 too.  It seemed to work
> > perfectly when I turned it back on in 1997 (bsd.kern.mk 1.6).  However,
> > it can't have been completely broken for gcc-3.1, since I needed to
> > turn it off for the high-resolution profiling case.
> >
> > Bruce
>
>   Yeah, hmmmm. :-(
>
>   Is there a way to force GCC to inline them, despite what it thinks?
>   As I said on some other list, in retrospect, I would re-evaluate the
>   way this is done but currently have diverted my attention to a
>   somewhat different approach.

My initial investigation of this problem:

% From bde at zeta.org.au Sat Jul 19 13:34:58 2003 +1000
% Date: Sat, 19 Jul 2003 13:34:54 +1000 (EST)
% From: Bruce Evans <bde at zeta.org.au>
% X-X-Sender: bde at gamplex.bde.org
% To: "Jacques A. Vidrine" <nectar at freebsd.org>
% cc: Nate Lawson <nate at root.org>, kan at freebsd.org, current at freebsd.org
% Subject: Re: warning: inlining failed
% In-Reply-To: <20030718194029.GC70721 at madman.celabo.org>
% Message-ID: <20030719121303.O25023 at gamplex.bde.org>
% References: <20030718121511.I26395 at root.org> <20030718194029.GC70721 at madman.celabo.org>
% MIME-Version: 1.0
% Content-Type: TEXT/PLAIN; charset=US-ASCII
% Status: O
% X-Status:
% X-Keywords:
% X-UID: 8213
%
% On Fri, 18 Jul 2003, Jacques A. Vidrine wrote:
%
% > On Fri, Jul 18, 2003 at 12:18:14PM -0700, Nate Lawson wrote:
% > > Warner mentioned this was due to the gcc import.  Nearly every part of the
% > > kernel that uses newbus or buf.h prints out lots of warnings.  Can someone
% > > see about fixing this, whether it's by fixing our headers or build flags
% > > or gcc itself?  I've already wasted a few reboot cycles because valid
% > > warnings were lost in the crowd.
% > >
% > > cc -O -pipe -mcpu=pentiumpro  -D_KERNEL -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes  -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual  -fformat-extensions -std=c99 -DKLD_MODULE -nostdinc -I-   -I. -I@ -I@/dev -I@/../include -I/usr/include -fno-common  -mno-align-long-strings -mpreferred-stack-boundary=2 -ffreestanding -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes  -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual  -fformat-extensions -std=c99 -c
% > > /home/src/sys/modules/ext2fs/../../gnu/ext2fs/ext2_vfsops.c
% > > /home/src/sys/gnu/ext2fs/ext2_vfsops.c: In function `compute_sb_data':
% > > @/sys/buf.h:281: warning: inlining failed in call to `BUF_LOCK'
% > > /home/src/sys/gnu/ext2fs/ext2_vfsops.c:496: warning: called from here
% > ...
% > Does `-finline-limit=1200' (or bigger) help?
%
% Maybe.  It allows larger declared-inline functions to actually be inlined
% of course.  This probably helps performance negatively in the case of
% large functions like BUF_LOCK.
%
% > I think GCC 3.3 added a warning for when inline functions generated `a
% > lot' of instructions.  In such a case, the function is not inlined.  I
% > believe this also happened with GCC 3.2, but it just didn't normally
% > tell you about it.
%
% A warning (-Winline) about gcc not inlining a function because the
% function involves "a lot" of instructions has existed for ages, and
% FreeBSD has used it since since I reenabled it in 1997 in rev.1.6 of
% bsd.kern.mk, but it was apparently broken in at least gcc-3.[1-2].
% The main differences between gcc-3.2 and gcc-3.3 in this area seem to
% be just that the warning actually works in gcc-3.3, and gcc-3.3 has
% more options for quantifying "a lot" than anyone would want to know
% about.
%
% Since gcc now warns when it should, and successful inlining of all
% inline functions in FreeBSD was apparently broken in gcc-3.1, gcc-3.3
% now emits hundreds or thousands of warnings about functions that it
% can't inline.  -Wunline was supposed to let us fix bogus inlining
% incrementatally, but this was defeated by it not working in gcc-3.[1-2].
%
% E.g., according to my kernel backups, non-inlining of BUF_LOCK started
% with gcc-3.1.  Some relevant history:
%
% 1996/06/26: BUF_LOCK implemented (as an inline) in buf.h rev.1.71
% 2002/05/09: kernel built on this date by gcc-2.95.4 (20020320) has no
%             static copies of BUF_LOCK
% 2002/06/29: kernel built on this date by gcc-3.1 (20020529) has 11
%             static copies of BUF_LOCK
%
% The new options for controlling inlining are:
%
% -finline-linit=<value>
% --param max-inline-insns=<value>
% --param max-inline-insns-single=<value>
% --param max-inline-insns-auto=<value>
% --param min-inline-insns=<value>
% --param max-inline-insns-rtl=<value>
%
% See gcc.info for details.
%
% I couldn't find a setting that I liked.  Most things compile with
% --param max-inline-insns-single=1600, which sort of corresponds to
% -finline-linit=3200 (more than 5 times larger than the default).
% A few files need amazingly larger values.  Compiling with values
% smaller than the default unconvered interesting bugs in the source
% code (invalid asm and an unitiialized variable).  What I want is
% for leaf functions declared as inline to always be inlined unless
% they are larger than some limit, but the gcc controls are mainly for
% for limiting the size of non-leaf functions.  Apparently-small
% functions can become amazingly large due to nested inllining.  This
% gives inlining failures which are not entirely the fault of bloat in
% the inline functions.  E.g., the following trick (which is used a lot
% in subr_mbuf.c and kern_descrip.c) doesn't actually give inline functions:
%
% %%%
% static inline int
% bigfunc(int foo, int flags)
% {
% 	/* Large code. */
% 	...
% 	return (resultoflargecode);
% }
%
% static int
% smallfunc1(int foo)
% {
% 	return (bigfunc(foo, 1));
% }
%
% static int
% smallfunc2(int foo)
% {
% 	return (bigfunc(foo, 2));
% }
%
% static int
% smallfunc3(int foo)
% {
% 	return (bigfunc(foo, 3));
% }
% ...
% %%%
%
% This trick is used mainly to avoid repeating the relevant parts of
% bigfunc() at the source level.  Repeating them at the object level is
% wanted and expected to do more than just avoid a function call since
% large sections of code can be optimized away when `flags' has a constant
% value.  But gcc-3 doesn't like this trick since it gives large code
% to inline.  The effectivness of the desired inlining (or lack thereof)
% is apparently null.  No one noticed when the inlining stopped with
% gcc-3.1 14 months ago.  (I checked that the interesting inlining of
% mb_alloc() was done on 2002/05/09 but not on 2002/06/29.)
%
% Bruce

>   What concerns me about GCC's idea of what is "inlinable" code and what
>   is not is that in some scenarios (admittedly this may not be the case
>   for mb_alloc), we will have an inlined version of some function for an
>   application such as the following:
>
>   inline-function-A() {
>   	blah blah;
>   }
>
>   /* Exported API */
>   real-out-of-line-function1() {
> 	inline-function-A();
> 	if (one_more_thing)
> 		try_this();
>   }
>
>   /* Some other Exported API */
>   real-out-of-line-function2() {
>   	inline-function-A();
>   }
>
>   In such a scenario, the inline-function-A(), only inlined in two
>   places, reduces code duplication.  Even if it's slightly bigger than
>   what GCC thinks "is OK," I could still see a legitimate reason for
>   inlining it.

I think gcc's inlining heuristics are tuned towards automatic inlining,
and it should try harder for explicit inline keywords (for C, maybe
not for C++).  Maybe we should set the inlining limits to infinity for
the kernel since we know what we are doing with the inline keywords
(just like for register keywords ;-) and we don't use automatic
inlining.

Bruce