From owner-svn-src-all@FreeBSD.ORG Sat Apr 11 17:18:46 2009 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C0989106564A; Sat, 11 Apr 2009 17:18:46 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail11.syd.optusnet.com.au (mail11.syd.optusnet.com.au [211.29.132.192]) by mx1.freebsd.org (Postfix) with ESMTP id 5AF5D8FC1A; Sat, 11 Apr 2009 17:18:46 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from besplex.bde.org (c122-107-120-227.carlnfd1.nsw.optusnet.com.au [122.107.120.227]) by mail11.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id n3BHIhsE009623 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 12 Apr 2009 03:18:44 +1000 Date: Sun, 12 Apr 2009 03:18:43 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Ed Schouten In-Reply-To: <200904111401.n3BE1108088009@svn.freebsd.org> Message-ID: <20090412025829.M4579@besplex.bde.org> References: <200904111401.n3BE1108088009@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r190919 - in head/sys: amd64/amd64 amd64/include i386/i386 i386/include X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 11 Apr 2009 17:18:47 -0000 On Sat, 11 Apr 2009, Ed Schouten wrote: > Log: > Simplify in/out functions (for i386 and AMD64). > > Remove a hack to generate more efficient code for port numbers below > 0x100, which has been obsolete for at least ten years, because GCC has > an asm constraint to specify that. > > Submitted by: Christoph Mallon This is not quite the submitted patch and is more like I want. It still has a micro-pessimizations and style bugs: > Modified: head/sys/amd64/amd64/machdep.c > ============================================================================== > --- head/sys/amd64/amd64/machdep.c Sat Apr 11 13:15:38 2009 (r190918) > +++ head/sys/amd64/amd64/machdep.c Sat Apr 11 14:01:01 2009 (r190919) > @@ -2178,45 +2178,24 @@ user_dbreg_trap(void) > #ifdef KDB > > /* > - * Provide inb() and outb() as functions. They are normally only > - * available as macros calling inlined functions, thus cannot be > - * called from the debugger. > - * > - * The actual code is stolen from , and de-inlined. > + * Provide inb() and outb() as functions. They are normally only available as > + * inline functions, thus cannot be called from the debugger. > */ This part should be done differently, but for a quick fix: > > -#undef inb > -#undef outb > - > /* silence compiler warnings */ > -u_char inb(u_int); > -void outb(u_int, u_char); > +u_char inb_(u_short); > +void outb_(u_short, u_char); This still changes the API from u_int port to u_short port. A better way to stop the warnings might be to declare the functions as static and do whatever is done for other ddb functions to prevent the compiler removing static functions that are never called. I think there are a few other static ddb-hack functions that were broken by gcc removing them. It used to take -O3 to remove them, but now -O1 does. > > u_char > -inb(u_int port) > +inb_(u_short port) > { API change. > - u_char data; > - /* > - * We use %%dx and not %1 here because i/o is done at %dx and not at > - * %edx, while gcc generates inferior code (movw instead of movl) > - * if we tell it to load (u_short) port. > - */ > - __asm __volatile("inb %%dx,%0" : "=a" (data) : "d" (port)); > - return (data); > + return inb(port); This still removes the KNF parentheses around the return value. > Modified: head/sys/amd64/include/cpufunc.h > ============================================================================== > --- head/sys/amd64/include/cpufunc.h Sat Apr 11 13:15:38 2009 (r190918) > +++ head/sys/amd64/include/cpufunc.h Sat Apr 11 14:01:01 2009 (r190919) > ... > -static __inline u_char > -inbv(u_int port) > -{ > ... > - __asm __volatile("inb %%dx,%0" : "=a" (data) : "d" (port)); > + __asm volatile("inb %w1, %0" : "=a" (data) : "Nd" (port)); > return (data); > } This still replaces __volatile by volatile. This might be a good style change, but should be done globally and separately. Similarly for replacing __inline by inline (which you didn't commit). Similarly for other __volatile's. Similarly for i386. It is easier to review and fix things before they are committed. Bruce