[PATCH] Revert "Make sys.c use unaligned access functions provided in compiler."
Matt Turner
mattst88 at gmail.com
Mon Dec 28 11:02:02 PST 2009
On Mon, Dec 28, 2009 at 11:00 AM, Tiago Vignatti
<tiago.vignatti at nokia.com> wrote:
> This reverts commit da923d0bc15e99a8ed1986bd6f5df37f7af7284b.
>
> Signed-off-by: Tiago Vignatti <tiago.vignatti at nokia.com>
>
> ---
> Again this Matt's change is breaking my compilation under scratchbox, for arm
> architecture:
>
> then mv -f ".deps/sys.Tpo" ".deps/sys.Plo"; else rm -f ".deps/sys.Tpo";
> exit 1; fi
> gcc -DHAVE_CONFIG_H -I. -I../../../../hw/xfree86/x86emu -I../../../include
> -I../../../include -I../../../include -I../../../include -I../../../include
> -I../../../include -I../../../include -I../../../include
> -I../../../../hw/xfree86 -I../../../../hw/xfree86/include
> -I../../../../hw/xfree86/common -I../../../../hw/xfree86/os-support
> -I../../../../hw/xfree86/os-support/bus -I../../../../os -DPRE_RELEASE=0
> -DHAVE_DIX_CONFIG_H -Wall -Wpointer-arith -Wstrict-prototypes
> -Wmissing-prototypes -Wmissing-declarations -Wnested-externs
> -fno-strict-aliasing -Wbad-function-cast -Wold-style-definition
> -Wdeclaration-after-statement -D_BSD_SOURCE -DHAS_FCHOWN -DHAS_STICKY_DIR_BIT
> -DDBUS_API_SUBJECT_TO_CHANGE -I/usr/include/freetype2 -I/usr/include/pixman-1
> -I/usr/include/hal -I/usr/include/dbus-1.0 -I/usr/lib/dbus-1.0/include
> -I../../../../include -I../../../include -I../../../../Xext
> -I../../../../composite -I../../../../damageext -I../../../../xfixes
> -I../../../../Xi -I../../../../mi -I../../../../miext/shadow
> -I../../../../miext/damage -I../../../../render -I../../../../randr
> -I../../../../fb -fvisibility=hidden -DHAVE_XORG_CONFIG_H -fvisibility=hidden
> -DXF86PM -Wall -g -O2 -MT sys.lo -MD -MP -MF .deps/sys.Tpo -c
> ../../../../hw/xfree86/x86emu/sys.c -fPIC -DPIC -o .libs/sys.o
> ../../../../hw/xfree86/x86emu/sys.c: In function 'rdl':
> ../../../../hw/xfree86/x86emu/sys.c:143: warning: passing argument 1 of
> 'ldl_u' from incompatible pointer type
> ../../../../hw/xfree86/x86emu/sys.c: In function 'wrl':
> ../../../../hw/xfree86/x86emu/sys.c:225: warning: passing argument 2 of
> 'stl_u' from incompatible pointer type
> ../../../../hw/xfree86/x86emu/sys.c: In function 'X86EMU_setupPioFuncs':
> ../../../../hw/xfree86/x86emu/sys.c:376: error: 'X86EMU_pioFuncs' has no
> member named 'xf_outb'
> ../../../../hw/xfree86/x86emu/sys.c:377: error: 'X86EMU_pioFuncs' has no
> member named 'xf_outw'
> ../../../../hw/xfree86/x86emu/sys.c:378: error: 'X86EMU_pioFuncs' has no
> member named 'xf_outl'
>
>
> Sorry to not scream before about it, when the patch came to xorg-devel review.
>
> Now I could try to fix the issue of the patch, but I'm not happy with its
> approach: the patch is using a header from the server inside x86emu. There's
> no conceptual problem on it but it will complicates when we try to remove
> x86emu from the server (my ongoing work - using lrmi).
>
> So I'd be happy to just revert such patch now.
>
>
> hw/xfree86/x86emu/Makefile.am | 2 +-
> hw/xfree86/x86emu/sys.c | 181 ++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 181 insertions(+), 2 deletions(-)
>
> diff --git a/hw/xfree86/x86emu/Makefile.am b/hw/xfree86/x86emu/Makefile.am
> index e7368f5..df96977 100644
> --- a/hw/xfree86/x86emu/Makefile.am
> +++ b/hw/xfree86/x86emu/Makefile.am
> @@ -11,7 +11,7 @@ libx86emu_la_SOURCES = debug.c \
> sys.c \
> x86emu.h
>
> -INCLUDES = $(XORG_INCS)
> +INCLUDES =
>
> AM_CFLAGS = $(DIX_CFLAGS) $(XORG_CFLAGS)
>
> diff --git a/hw/xfree86/x86emu/sys.c b/hw/xfree86/x86emu/sys.c
> index 2ebf6f1..e15fb09 100644
> --- a/hw/xfree86/x86emu/sys.c
> +++ b/hw/xfree86/x86emu/sys.c
> @@ -48,13 +48,168 @@
> #ifndef NO_SYS_HEADERS
> #include <string.h>
> #endif
> -#include "compiler.h" /* for unaligned access functions */
> /*------------------------- Global Variables ------------------------------*/
>
> X86EMU_sysEnv _X86EMU_env; /* Global emulator machine state */
> X86EMU_intrFuncs _X86EMU_intrTab[256];
>
> /*----------------------------- Implementation ----------------------------*/
> +#if defined(__alpha__) || defined(__alpha)
> +/* to cope with broken egcs-1.1.2 :-(((( */
> +
> +#define ALPHA_UALOADS
> +/*
> + * inline functions to do unaligned accesses
> + * from linux/include/asm-alpha/unaligned.h
> + */
> +
> +/*
> + * EGCS 1.1 knows about arbitrary unaligned loads. Define some
> + * packed structures to talk about such things with.
> + */
> +
> +#if defined(__GNUC__)
> +struct __una_u64 { unsigned long x __attribute__((packed)); };
> +struct __una_u32 { unsigned int x __attribute__((packed)); };
> +struct __una_u16 { unsigned short x __attribute__((packed)); };
> +#endif
> +
> +static __inline__ unsigned long ldq_u(unsigned long * r11)
> +{
> +#if defined(__GNUC__)
> + const struct __una_u64 *ptr = (const struct __una_u64 *) r11;
> + return ptr->x;
> +#else
> + unsigned long r1,r2;
> + __asm__("ldq_u %0,%3\n\t"
> + "ldq_u %1,%4\n\t"
> + "extql %0,%2,%0\n\t"
> + "extqh %1,%2,%1"
> + :"=&r" (r1), "=&r" (r2)
> + :"r" (r11),
> + "m" (*r11),
> + "m" (*(const unsigned long *)(7+(char *) r11)));
> + return r1 | r2;
> +#endif
> +}
> +
> +static __inline__ unsigned long ldl_u(unsigned int * r11)
> +{
> +#if defined(__GNUC__)
> + const struct __una_u32 *ptr = (const struct __una_u32 *) r11;
> + return ptr->x;
> +#else
> + unsigned long r1,r2;
> + __asm__("ldq_u %0,%3\n\t"
> + "ldq_u %1,%4\n\t"
> + "extll %0,%2,%0\n\t"
> + "extlh %1,%2,%1"
> + :"=&r" (r1), "=&r" (r2)
> + :"r" (r11),
> + "m" (*r11),
> + "m" (*(const unsigned long *)(3+(char *) r11)));
> + return r1 | r2;
> +#endif
> +}
> +
> +static __inline__ unsigned long ldw_u(unsigned short * r11)
> +{
> +#if defined(__GNUC__)
> + const struct __una_u16 *ptr = (const struct __una_u16 *) r11;
> + return ptr->x;
> +#else
> + unsigned long r1,r2;
> + __asm__("ldq_u %0,%3\n\t"
> + "ldq_u %1,%4\n\t"
> + "extwl %0,%2,%0\n\t"
> + "extwh %1,%2,%1"
> + :"=&r" (r1), "=&r" (r2)
> + :"r" (r11),
> + "m" (*r11),
> + "m" (*(const unsigned long *)(1+(char *) r11)));
> + return r1 | r2;
> +#endif
> +}
> +
> +/*
> + * Elemental unaligned stores
> + */
> +
> +static __inline__ void stq_u(unsigned long r5, unsigned long * r11)
> +{
> +#if defined(__GNUC__)
> + struct __una_u64 *ptr = (struct __una_u64 *) r11;
> + ptr->x = r5;
> +#else
> + unsigned long r1,r2,r3,r4;
> +
> + __asm__("ldq_u %3,%1\n\t"
> + "ldq_u %2,%0\n\t"
> + "insqh %6,%7,%5\n\t"
> + "insql %6,%7,%4\n\t"
> + "mskqh %3,%7,%3\n\t"
> + "mskql %2,%7,%2\n\t"
> + "bis %3,%5,%3\n\t"
> + "bis %2,%4,%2\n\t"
> + "stq_u %3,%1\n\t"
> + "stq_u %2,%0"
> + :"=m" (*r11),
> + "=m" (*(unsigned long *)(7+(char *) r11)),
> + "=&r" (r1), "=&r" (r2), "=&r" (r3), "=&r" (r4)
> + :"r" (r5), "r" (r11));
> +#endif
> +}
> +
> +static __inline__ void stl_u(unsigned long r5, unsigned int * r11)
> +{
> +#if defined(__GNUC__)
> + struct __una_u32 *ptr = (struct __una_u32 *) r11;
> + ptr->x = r5;
> +#else
> + unsigned long r1,r2,r3,r4;
> +
> + __asm__("ldq_u %3,%1\n\t"
> + "ldq_u %2,%0\n\t"
> + "inslh %6,%7,%5\n\t"
> + "insll %6,%7,%4\n\t"
> + "msklh %3,%7,%3\n\t"
> + "mskll %2,%7,%2\n\t"
> + "bis %3,%5,%3\n\t"
> + "bis %2,%4,%2\n\t"
> + "stq_u %3,%1\n\t"
> + "stq_u %2,%0"
> + :"=m" (*r11),
> + "=m" (*(unsigned long *)(3+(char *) r11)),
> + "=&r" (r1), "=&r" (r2), "=&r" (r3), "=&r" (r4)
> + :"r" (r5), "r" (r11));
> +#endif
> +}
> +
> +static __inline__ void stw_u(unsigned long r5, unsigned short * r11)
> +{
> +#if defined(__GNUC__)
> + struct __una_u16 *ptr = (struct __una_u16 *) r11;
> + ptr->x = r5;
> +#else
> + unsigned long r1,r2,r3,r4;
> +
> + __asm__("ldq_u %3,%1\n\t"
> + "ldq_u %2,%0\n\t"
> + "inswh %6,%7,%5\n\t"
> + "inswl %6,%7,%4\n\t"
> + "mskwh %3,%7,%3\n\t"
> + "mskwl %2,%7,%2\n\t"
> + "bis %3,%5,%3\n\t"
> + "bis %2,%4,%2\n\t"
> + "stq_u %3,%1\n\t"
> + "stq_u %2,%0"
> + :"=m" (*r11),
> + "=m" (*(unsigned long *)(1+(char *) r11)),
> + "=&r" (r1), "=&r" (r2), "=&r" (r3), "=&r" (r4)
> + :"r" (r5), "r" (r11));
> +#endif
> +}
> +#endif
>
> /****************************************************************************
> PARAMETERS:
> @@ -107,7 +262,13 @@ u16 X86API rdw(
> }
> else
> #endif
> +#if defined(ALPHA_UALOADS)
> val = ldw_u((u16*)(M.mem_base + addr));
> +#elif defined(IA64_UALOADS)
> + val = uldw((u16*)(M.mem_base + addr));
> +#else
> + val = *(u16*)(M.mem_base + addr);
> +#endif
> DB( if (DEBUG_MEM_TRACE())
> printk("%#08x 2 -> %#x\n", addr, val);)
> return val;
> @@ -140,7 +301,13 @@ u32 X86API rdl(
> }
> else
> #endif
> +#if defined(ALPHA_UALOADS)
> val = ldl_u((u32*)(M.mem_base + addr));
> +#elif defined(IA64_UALOADS)
> + val = uldl((u32*)(M.mem_base + addr));
> +#else
> + val = *(u32*)(M.mem_base + addr);
> +#endif
> DB( if (DEBUG_MEM_TRACE())
> printk("%#08x 4 -> %#x\n", addr, val);)
> return val;
> @@ -192,7 +359,13 @@ DB( if (DEBUG_MEM_TRACE())
> }
> else
> #endif
> +#if defined(ALPHA_UALOADS)
> stw_u(val,(u16*)(M.mem_base + addr));
> +#elif defined(IA64_UALOADS)
> + ustw(val,(u16*)(M.mem_base + addr));
> +#else
> + *(u16*)(M.mem_base + addr) = val;
> +#endif
> }
>
> /****************************************************************************
> @@ -222,7 +395,13 @@ DB( if (DEBUG_MEM_TRACE())
> }
> else
> #endif
> +#if defined(ALPHA_UALOADS)
> stl_u(val,(u32*)(M.mem_base + addr));
> +#elif defined(IA64_UALOADS)
> + ustl(val,(u32*)(M.mem_base + addr));
> +#else
> + *(u32*)(M.mem_base + addr) = val;
> +#endif
> }
>
> /****************************************************************************
> --
> 1.6.3.3
The fix here isn't to add back in a bunch of duplicated Alpha assembly.
It this needs its own unaligned access code, then copy the platform
agnostic code from compiler.h. That is, the code like
static __inline__ uint32_t ldl_u(uint32_t *p)
{
const struct __una_u32 *ptr = (const struct __una_u32 *) p;
return ptr->x;
}
But please, do not add back in the Alpha assembly.
I understand that ARM-specific nastiness in compiler.h (lines
compiler.h:1017-1019) causes sys.c to not compile--there is an
appropriate fix and this is not it.
Maybe move ARM away from out{b,w,l} to the new pci access routines ajax wrote?
Matt
More information about the xorg-devel
mailing list