[opus] [Aarch64 v2 02/18] Reorganize ARM CPU #ifdefs.

Timothy B. Terriberry tterribe at xiph.org
Tue Dec 8 09:13:05 PST 2015


Jonathan Lennox wrote:
> -# if defined(FIXED_POINT)
> +# if defined(FIXED_POINT) && \
> +	((defined(OPUS_ARM_MAY_HAVE_NEON) && !defined(OPUS_ARM_PRESUME_NEON)) || \
> +		(defined(OPUS_ARM_MAY_HAVE_MEDIA) && !defined(OPUS_ARM_PRESUME_MEDIA)) || \
> +		(defined(OPUS_ARM_MAY_HAVE_EDSP) && !defined(OPUS_ARM_PRESUME_EDSP)))
>   opus_val32 (*const CELT_PITCH_XCORR_IMPL[OPUS_ARCHMASK+1])(const opus_val16 *,
>       const opus_val16 *, opus_val32 *, int , int) = {

Maybe I'm missing something, but...

> -/*Is run-time CPU detection enabled on this platform?*/
> -# if defined(OPUS_HAVE_RTCD) && (defined(OPUS_ARM_ASM) \
> -   || (defined(OPUS_ARM_MAY_HAVE_NEON_INTR) \
> -   && !defined(OPUS_ARM_PRESUME_NEON_INTR)))
> +# if defined(OPUS_HAVE_RTCD) && \
> +	(defined(FIXED_POINT) && \
> +		((defined(OPUS_ARM_MAY_HAVE_NEON) && !defined(OPUS_ARM_PRESUME_NEON)) || \
> +			(defined(OPUS_ARM_MAY_HAVE_MEDIA) && !defined(OPUS_ARM_PRESUME_MEDIA)) || \
> +			(defined(OPUS_ARM_MAY_HAVE_EDSP) && !defined(OPUS_ARM_PRESUME_EDSP)))) || \
> +	(!defined(FIXED_POINT) && \
> +		(defined(OPUS_ARM_MAY_HAVE_NEON_INTR) && !defined(OPUS_ARM_PRESUME_NEON_INTR)))
>   extern
>   #  if defined(FIXED_POINT)
>   opus_val32
>   #  else
>   void
>   #  endif
>   (*const CELT_PITCH_XCORR_IMPL[OPUS_ARCHMASK+1])(const opus_val16 *,
>         const opus_val16 *, opus_val32 *, int, int);

Shouldn't the first case have a corresponding update in the #else clause 
for the !defined(FIXED_POINT) case?

>     celt_pitch_xcorr_c,               /* ARMv4 */
> diff --git a/celt/arm/pitch_arm.h b/celt/arm/pitch_arm.h
> index eaf61c9..bd41774 100644
> --- a/celt/arm/pitch_arm.h
> +++ b/celt/arm/pitch_arm.h
> @@ -46,7 +46,13 @@ opus_val32 celt_pitch_xcorr_edsp(const opus_val16 *_x, const opus_val16 *_y,
>       opus_val32 *xcorr, int len, int max_pitch);
>   #  endif
>
> -#  if !defined(OPUS_HAVE_RTCD)
> +#  if (defined(OPUS_ARM_MAY_HAVE_NEON) || \
> +		defined(OPUS_ARM_MAY_HAVE_MEDIA) || \
> +		defined(OPUS_ARM_MAY_HAVE_EDSP)) && \
> +	(!defined(OPUS_HAVE_RTCD) || \
> +		defined(OPUS_ARM_PRESUME_NEON) || \
> +		(defined(OPUS_ARM_PRESUME_MEDIA) && !defined(OPUS_ARM_MAY_HAVE_NEON)) || \
> +		(defined(OPUS_ARM_PRESUME_EDSP) && !defined(OPUS_ARM_MAY_HAVE_NEON) && !defined(OPUS_ARM_MAY_HAVE_MEDIA)))
>   #   define OVERRIDE_PITCH_XCORR (1)
>   #   define celt_pitch_xcorr(_x, _y, xcorr, len, max_pitch, arch) \
>     ((void)(arch),PRESUME_NEON(celt_pitch_xcorr)(_x, _y, xcorr, len, max_pitch))
> @@ -66,10 +72,13 @@ void celt_pitch_xcorr_float_neon(const opus_val16 *_x, const opus_val16 *_y,
>
>   #endif /* end !FIXED_POINT */
>

And shouldn't this be the inverse of the previous two?


More information about the opus mailing list