Created attachment 22204 [details] [review] use the BWX instructions for 8 and 16 bit access on Dense mapping The {read,write}Dense{8,16} functions, as implemented, always to 32bits access and truncate the result to 8/16 bits, or do read/modify/write cycles. This is bad if the read or write cycle to adjacent registers change the adapter's state (e.g. if an adjacent register is a fifo, we'll discard values, or add additionnal, unwanted values in the fifo). Fix: use the BWX instructions for 8 and 16 bit access. Note: this use macros from <machine/bwx.h>. This will work on NetBSD and OpenBSD, but I don't know about FreeBSD.
The code in bsd_ev56.c and lnx_ev56.c seems to be identical. I checked the gcc output of lnx_ev56.c and it _does_ use BWX instructions where appropriate. Can you post some assembly output of the original code and your patched version to show the differences?
> --- Comment #1 from Matt Turner <mattst88@gmail.com> 2009-01-31 10:14:22 PST --- > The code in bsd_ev56.c and lnx_ev56.c seems to be identical. > > I checked the gcc output of lnx_ev56.c and it _does_ use BWX instructions where > appropriate. lnx_ev56.c is built with -mcpu=ev56 though, which doesn't seem to be the case for the bsd version. See bug #9598.
On Sat, Jan 31, 2009 at 10:14:22AM -0800, bugzilla-daemon@freedesktop.org wrote: > http://bugs.freedesktop.org/show_bug.cgi?id=19722 > > > > > > --- Comment #1 from Matt Turner <mattst88@gmail.com> 2009-01-31 10:14:22 PST --- > The code in bsd_ev56.c and lnx_ev56.c seems to be identical. > > I checked the gcc output of lnx_ev56.c and it _does_ use BWX instructions where > appropriate. > > Can you post some assembly output of the original code and your patched version > to show the differences? I guess gcc may optimise it when this file is built with -mcpu=ev56; but AFAIK bsd_ev56.c is not built with any special CPU flags and so will default to ev4 instruction set. Here's the result for e.g. *Dense8: (gdb) disas readDense8 Dump of assembler code for function readDense8: 0x0 <readDense8>: mb 0x4 <readDense8+4>: addq a0,a1,a0 0x8 <readDense8+8>: ldq_u v0,0(a0) 0xc <readDense8+12>: extbl v0,a0,v0 0x10 <readDense8+16>: ret 0x14 <readDense8+20>: unop 0x18 <readDense8+24>: nop 0x1c <readDense8+28>: unop End of assembler dump. (gdb) disas writeDense8 Dump of assembler code for function writeDense8: 0xb0 <writeDense8>: wmb 0xb4 <writeDense8+4>: addq a1,a2,a1 0xb8 <writeDense8+8>: insbl a0,a1,a0 0xbc <writeDense8+12>: ldq_u t0,0(a1) 0xc0 <writeDense8+16>: mskbl t0,a1,t0 0xc4 <writeDense8+20>: or a0,t0,a0 0xc8 <writeDense8+24>: stq_u a0,0(a1) 0xcc <writeDense8+28>: ret
commit 0055fe66d5f73742cafab868ccdb7a6f36ea1dd5 Author: Manuel Bouyer <bouyer@netbsd.org> Date: Sun Feb 1 09:14:19 2009 -0800 netbsd: Force the use of ev56 instructions for register access on ev56. This avoids 32-bit access which might affect other registers. The linux code uses gcc flags to get this to happen, but this seems like more of a sure thing.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.