Date: Wed, 23 Jul 2003 02:35:31 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Bosko Milekic <bmilekic@technokratis.com> Cc: cvs-all@FreeBSD.org Subject: Re: 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 Message-ID: <20030723022239.F8681@gamplex.bde.org> In-Reply-To: <20030722112901.GA59012@technokratis.com> References: <200307221024.h6MAOggG066724@repoman.freebsd.org> <20030723003823.R8380@gamplex.bde.org> <20030722112901.GA59012@technokratis.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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@zeta.org.au>
% X-X-Sender: bde@gamplex.bde.org
% To: "Jacques A. Vidrine" <nectar@freebsd.org>
% cc: Nate Lawson <nate@root.org>, kan@freebsd.org, current@freebsd.org
% Subject: Re: warning: inlining failed
% In-Reply-To: <20030718194029.GC70721@madman.celabo.org>
% Message-ID: <20030719121303.O25023@gamplex.bde.org>
% References: <20030718121511.I26395@root.org> <20030718194029.GC70721@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030723022239.F8681>