From owner-cvs-src@FreeBSD.ORG Tue Jul 22 09:35:43 2003 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3B31A37B401; Tue, 22 Jul 2003 09:35:43 -0700 (PDT) Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by mx1.FreeBSD.org (Postfix) with ESMTP id 72FB943FA3; Tue, 22 Jul 2003 09:35:41 -0700 (PDT) (envelope-from bde@zeta.org.au) Received: from gamplex.bde.org (katana.zip.com.au [61.8.7.246]) by mailman.zeta.org.au (8.9.3p2/8.8.7) with ESMTP id CAA10340; Wed, 23 Jul 2003 02:35:32 +1000 Date: Wed, 23 Jul 2003 02:35:31 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Bosko Milekic In-Reply-To: <20030722112901.GA59012@technokratis.com> Message-ID: <20030723022239.F8681@gamplex.bde.org> References: <200307221024.h6MAOggG066724@repoman.freebsd.org> <20030723003823.R8380@gamplex.bde.org> <20030722112901.GA59012@technokratis.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: cvs-src@FreeBSD.org cc: phk@phk.freebsd.dk cc: src-committers@FreeBSD.org 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 X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Jul 2003 16:35:43 -0000 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 % X-X-Sender: bde@gamplex.bde.org % To: "Jacques A. Vidrine" % cc: Nate Lawson , 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= % --param max-inline-insns= % --param max-inline-insns-single= % --param max-inline-insns-auto= % --param min-inline-insns= % --param max-inline-insns-rtl= % % 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