Bug 19722 - BSD Dense functions are wrong
Summary: BSD Dense functions are wrong
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/General (show other bugs)
Version: 7.3 (2007.09)
Hardware: Alpha NetBSD
: medium normal
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-01-24 09:10 UTC by Manuel Bouyer
Modified: 2009-02-01 09:26 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
use the BWX instructions for 8 and 16 bit access on Dense mapping (2.39 KB, patch)
2009-01-24 09:10 UTC, Manuel Bouyer
no flags Details | Splinter Review

Description Manuel Bouyer 2009-01-24 09:10:19 UTC
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.
Comment 1 Matt Turner 2009-01-31 10:14:22 UTC
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 2 Julien Cristau 2009-01-31 10:47:52 UTC
> --- 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.
Comment 3 Manuel Bouyer 2009-01-31 11:14:07 UTC
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

Comment 4 Eric Anholt 2009-02-01 09:26:42 UTC
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.