[Speex-dev] [PATCH] Fix miscompile of SSE resampler

Jeff Wallace jeffdwallace at gmail.com
Mon Oct 26 11:26:54 PDT 2009


Out of curiosity, what compiler(s) got it wrong?

On Mon, Oct 26, 2009 at 9:41 AM, Thorvald Natvig <thorvald at natvig.com>wrote:

> From: Thorvald Natvig <slicer at users.sourceforge.net>
>
> Some optimizing compilers miscompile the current SSE optimizations when
> full optimizations are enabled. By using output value pointer instead of
> a return value, we can bypass this misbehaviour.
> ---
>  libspeex/resample.c     |    8 ++++----
>  libspeex/resample_sse.h |   24 ++++++++----------------
>  2 files changed, 12 insertions(+), 20 deletions(-)
>
> diff --git a/libspeex/resample.c b/libspeex/resample.c
> index 7b5a308..8131380 100644
> --- a/libspeex/resample.c
> +++ b/libspeex/resample.c
> @@ -361,7 +361,7 @@ static int
> resampler_basic_direct_single(SpeexResamplerState *st, spx_uint32_t c
>       sum = accum[0] + accum[1] + accum[2] + accum[3];
>  */
>  #else
> -      sum = inner_product_single(sinc, iptr, N);
> +      inner_product_single(&sum, sinc, iptr, N);
>  #endif
>
>       out[out_stride * out_sample++] = SATURATE32(PSHR32(sum, 15), 32767);
> @@ -412,7 +412,7 @@ static int
> resampler_basic_direct_double(SpeexResamplerState *st, spx_uint32_t c
>       }
>       sum = accum[0] + accum[1] + accum[2] + accum[3];
>  #else
> -      sum = inner_product_double(sinc, iptr, N);
> +      inner_product_double(&sum, sinc, iptr, N);
>  #endif
>
>       out[out_stride * out_sample++] = PSHR32(sum, 15);
> @@ -472,7 +472,7 @@ static int
> resampler_basic_interpolate_single(SpeexResamplerState *st, spx_uint3
>       sum = MULT16_32_Q15(interp[0],SHR32(accum[0], 1)) +
> MULT16_32_Q15(interp[1],SHR32(accum[1], 1)) +
> MULT16_32_Q15(interp[2],SHR32(accum[2], 1)) +
> MULT16_32_Q15(interp[3],SHR32(accum[3], 1));
>  #else
>       cubic_coef(frac, interp);
> -      sum = interpolate_product_single(iptr, st->sinc_table +
> st->oversample + 4 - offset - 2, N, st->oversample, interp);
> +      interpolate_product_single(&sum, iptr, st->sinc_table +
> st->oversample + 4 - offset - 2, N, st->oversample, interp);
>  #endif
>
>       out[out_stride * out_sample++] = SATURATE32(PSHR32(sum, 14), 32767);
> @@ -534,7 +534,7 @@ static int
> resampler_basic_interpolate_double(SpeexResamplerState *st, spx_uint3
>       sum = MULT16_32_Q15(interp[0],accum[0]) +
> MULT16_32_Q15(interp[1],accum[1]) + MULT16_32_Q15(interp[2],accum[2]) +
> MULT16_32_Q15(interp[3],accum[3]);
>  #else
>       cubic_coef(frac, interp);
> -      sum = interpolate_product_double(iptr, st->sinc_table +
> st->oversample + 4 - offset - 2, N, st->oversample, interp);
> +      interpolate_product_double(&sum, iptr, st->sinc_table +
> st->oversample + 4 - offset - 2, N, st->oversample, interp);
>  #endif
>
>       out[out_stride * out_sample++] = PSHR32(sum,15);
> diff --git a/libspeex/resample_sse.h b/libspeex/resample_sse.h
> index 64be8a1..86ff35e 100644
> --- a/libspeex/resample_sse.h
> +++ b/libspeex/resample_sse.h
> @@ -37,10 +37,9 @@
>  #include <xmmintrin.h>
>
>  #define OVERRIDE_INNER_PRODUCT_SINGLE
> -static inline float inner_product_single(const float *a, const float *b,
> unsigned int len)
> +static inline void inner_product_single(float *ret, const float *a, const
> float *b, unsigned int len)
>  {
>    int i;
> -   float ret;
>    __m128 sum = _mm_setzero_ps();
>    for (i=0;i<len;i+=8)
>    {
> @@ -49,14 +48,12 @@ static inline float inner_product_single(const float
> *a, const float *b, unsigne
>    }
>    sum = _mm_add_ps(sum, _mm_movehl_ps(sum, sum));
>    sum = _mm_add_ss(sum, _mm_shuffle_ps(sum, sum, 0x55));
> -   _mm_store_ss(&ret, sum);
> -   return ret;
> +   _mm_store_ss(ret, sum);
>  }
>
>  #define OVERRIDE_INTERPOLATE_PRODUCT_SINGLE
> -static inline float interpolate_product_single(const float *a, const float
> *b, unsigned int len, const spx_uint32_t oversample, float *frac) {
> +static inline void interpolate_product_single(float *ret, const float *a,
> const float *b, unsigned int len, const spx_uint32_t oversample, float
> *frac) {
>   int i;
> -  float ret;
>   __m128 sum = _mm_setzero_ps();
>   __m128 f = _mm_loadu_ps(frac);
>   for(i=0;i<len;i+=2)
> @@ -67,18 +64,16 @@ static inline float interpolate_product_single(const
> float *a, const float *b, u
>    sum = _mm_mul_ps(f, sum);
>    sum = _mm_add_ps(sum, _mm_movehl_ps(sum, sum));
>    sum = _mm_add_ss(sum, _mm_shuffle_ps(sum, sum, 0x55));
> -   _mm_store_ss(&ret, sum);
> -   return ret;
> +   _mm_store_ss(ret, sum);
>  }
>
>  #ifdef _USE_SSE2
>  #include <emmintrin.h>
>  #define OVERRIDE_INNER_PRODUCT_DOUBLE
>
> -static inline double inner_product_double(const float *a, const float *b,
> unsigned int len)
> +static inline void inner_product_double(double *ret, const float *a, const
> float *b, unsigned int len)
>  {
>    int i;
> -   double ret;
>    __m128d sum = _mm_setzero_pd();
>    __m128 t;
>    for (i=0;i<len;i+=8)
> @@ -92,14 +87,12 @@ static inline double inner_product_double(const float
> *a, const float *b, unsign
>       sum = _mm_add_pd(sum, _mm_cvtps_pd(_mm_movehl_ps(t, t)));
>    }
>    sum = _mm_add_sd(sum, _mm_unpackhi_pd(sum, sum));
> -   _mm_store_sd(&ret, sum);
> -   return ret;
> +   _mm_store_sd(ret, sum);
>  }
>
>  #define OVERRIDE_INTERPOLATE_PRODUCT_DOUBLE
> -static inline double interpolate_product_double(const float *a, const
> float *b, unsigned int len, const spx_uint32_t oversample, float *frac) {
> +static inline void interpolate_product_double(double *ret, const float *a,
> const float *b, unsigned int len, const spx_uint32_t oversample, float
> *frac) {
>   int i;
> -  double ret;
>   __m128d sum;
>   __m128d sum1 = _mm_setzero_pd();
>   __m128d sum2 = _mm_setzero_pd();
> @@ -121,8 +114,7 @@ static inline double interpolate_product_double(const
> float *a, const float *b,
>   sum2 = _mm_mul_pd(f2, sum2);
>   sum = _mm_add_pd(sum1, sum2);
>   sum = _mm_add_sd(sum, _mm_unpackhi_pd(sum, sum));
> -  _mm_store_sd(&ret, sum);
> -  return ret;
> +  _mm_store_sd(ret, sum);
>  }
>
>  #endif
> --
> 1.6.4.msysgit.0.19.gd78f4
>
> _______________________________________________
> Speex-dev mailing list
> Speex-dev at xiph.org
> http://lists.xiph.org/mailman/listinfo/speex-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.xiph.org/pipermail/speex-dev/attachments/20091026/d08c6510/attachment-0001.htm 


More information about the Speex-dev mailing list