Bug#318218: Thoughts :-)
On Fri, Jul 15, 2005 at 02:13:35AM -0400, Nathanael Nerode wrote:
> This can probably be fixed by changing the following stanza from
> xc/programs/Xserver/hw/xfree86/common/compiler.h:
>
> # else /* !__alpha__ && !__powerpc__ && !__sparc__ */
>
> # define MMIO_IN8(base, offset) \
> *(volatile CARD8 *)(((CARD8*)(base)) + (offset))
> # define MMIO_IN16(base, offset) \
> *(volatile CARD16 *)(void *)(((CARD8*)(base)) + (offset))
> # define MMIO_IN32(base, offset) \
> *(volatile CARD32 *)(void *)(((CARD8*)(base)) + (offset))
> # define MMIO_OUT8(base, offset, val) \
> *(volatile CARD8 *)(((CARD8*)(base)) + (offset)) = (val)
> # define MMIO_OUT16(base, offset, val) \
> *(volatile CARD16 *)(void *)(((CARD8*)(base)) + (offset)) = (val)
> # define MMIO_OUT32(base, offset, val) \
> *(volatile CARD32 *)(void *)(((CARD8*)(base)) + (offset)) = (val)
>
> OK. The problem with this is that the cast to volatile CARD8* (et al) doesn't
> create a volatile object from the point of view of GCC4. (GCC4 may be wrong
> here.) However, code like the following, if I'm not mistaken, *does* create
> a volatile object, and GCC4 notices:
>
> static inline unsigned char
> xorgReadMmio8(volatile void * base, const unsigned long offset)
> {
> volatile CARD8 * tmp;
> tmp = (CARD8*)base + offset;
> return *tmp;
> }
> #define MMIO_IN8(base, offset) xorgReadMmio8(base, offset)
>
> (Based on a little testing of simplified code, GCC is smart enough to optimize
> away nearly everything in this, leaving nicely optimized assembly which still
> does the memory read, even when the resulting data is never accessed.)
>
> So a consistent replacement along these lines should do the trick.
> Shall I whip up a patch?
To be honest, I'd rather not mess with this file unless we have to. The
simple fix in Redhat's bugzilla looks a lot less invasive, and it seems to
fix all the reported problems so far, so I'd rather go with that unless
upstream says otherwise.
I'm making a patch for us right now.
- David Nusinow
Reply to: