[Pixman] [PATCH 2/6] mmx: compile on MIPS for Loongson MMI optimizations

[Søren Sandmann]

Matt Turner <mattst88 at gmail.com> writes:

> +/* vectors are stored in 64-bit floating-point registers */
> +typedef double __m64;

> [...]

> @@ -114,11 +118,14 @@ _mm_shuffle_pi16 (__m64 __A, int8_t const __N)
>   * uint64_t and __m64 values, then define USE_CVT_INTRINSICS.
>   * If __m64 and uint64_t values can just be cast to each other directly,
>   * then define USE_M64_CASTS.
> + * If __m64 is a double datatype, then define USE_M64_DOUBLE.
>   */
>  #ifdef _MSC_VER
>  # define M64_MEMBER m64_u64
>  #elif defined(__ICC)
>  # define USE_CVT_INTRINSICS
> +#elif defined(USE_LOONGSON_MMI)
> +# define USE_M64_DOUBLE
>  #elif defined(__GNUC__)
>  # define USE_M64_CASTS
>  #elif defined(__SUNPRO_C)
> @@ -136,7 +143,7 @@ _mm_shuffle_pi16 (__m64 __A, int8_t const __N)
>  # endif
>  #endif
>  
> -#if defined(USE_M64_CASTS) || defined(USE_CVT_INTRINSICS)
> +#if defined(USE_M64_CASTS) || defined(USE_CVT_INTRINSICS) || defined(USE_M64_DOUBLE)
>  typedef uint64_t mmxdatafield;
>  #else
>  typedef __m64 mmxdatafield;
> @@ -188,6 +195,8 @@ static const mmx_data_t c =
>  #    define MC(x) to_m64 (c.mmx_ ## x)
>  #elif defined(USE_M64_CASTS)
>  #    define MC(x) ((__m64)c.mmx_ ## x)
> +#elif defined(USE_M64_DOUBLE)
> +#    define MC(x) (*(__m64 *)&c.mmx_ ## x)
>  #else
>  #    define MC(x) c.mmx_ ## x
>  #endif

As far as I can tell, in the above mmxdatafield is defined to be a
uint64_t, and MC is defined to cast from uint64_t to __m64, which is a
double. Is there any reason to not just define mmxdatafield as double
and then drop the casting?

> @@ -347,13 +360,26 @@ static __inline__ uint32_t ldl_u(uint32_t *p)
>  static force_inline __m64
>  load (const uint32_t *v)
>  {
> +#ifdef USE_LOONGSON_MMI
> +    __m64 ret;
> +    asm("lwc1 %0, %1\n\t"
> +	: "=f" (ret)
> +	: "m" (*v)
> +    );
> +    return ret;

Please put a space before "(". There are several instances of this, in
this and the other patches, and also a lot of existing cases in
pixman-mmx.c that weren't introduced by this series. Since this series
changes all the ldq_u() calls in the first patch, we might as well get
all of it cleaned up.

> +#else
>      return _mm_cvtsi32_si64 (*v);
> +#endif
>  }
>  
>  static force_inline __m64
>  load8888 (const uint32_t *v)
>  {
> +#ifdef USE_LOONGSON_MMI
> +    return _mm_unpacklo_pi8_f (*(__m32 *)v, _mm_setzero_si64 ());
> +#else
>      return _mm_unpacklo_pi8 (load (v), _mm_setzero_si64 ());
> +#endif
>  }

Above you defined what load() does on Loongson, but then you don't use
it here? It doesn't look like load() is used elsewhere, though I could
have overlooked some.


Soren