Date: Sun, 12 Apr 2009 03:18:43 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Ed Schouten <ed@freebsd.org> 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 Message-ID: <20090412025829.M4579@besplex.bde.org> In-Reply-To: <200904111401.n3BE1108088009@svn.freebsd.org> References: <200904111401.n3BE1108088009@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <christoph mallon gmx de>
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 <machine/cpufunc.h>, 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090412025829.M4579>