[opus] [RFC PATCH v3] armv7: celt_pitch_xcorr: Introduce ARM neon intrinsics

Timothy B. Terriberry tterribe at xiph.org
Wed Dec 17 21:59:23 PST 2014


Almost there... just a few nits left.

Viswanath Puttagunta wrote:
> +if OPUS_ARM_NEON_INTR
> +CELT_SOURCES += $(CELT_SOURCES_ARM_NEON_INTR)
> +OPUS_ARM_NEON_INTR_CPPFLAGS = -mfpu=neon -O3

I'll repeat: I don't think you should change the optimization level here.

> +   /* Just unroll the rest of the loop */

I saw you decided to keep this unrolled, but you didn't actually answer 
my question.

> -#elif defined(OPUS_ARM_ASM) && defined(FIXED_POINT)
> +#endif
> +
> +#if defined(OPUS_ARM_NEON_INTR)
> +#include "arm/celt_neon_intr.c"
> +#endif
> +
> +#if ((defined(OPUS_ARM_ASM) && defined(FIXED_POINT)) \
> +  || defined(OPUS_ARM_NEON_INTR))
>   #include "arm/arm_celt_map.c"
>   #endif

You should keep the #elif (the intent was to _guarantee_ that only one 
of x86_celt_map.c or arm_celt_map.c would be included). Instead, just 
move the #if defined(OPUS_ARM_NEON_INTR) block inside that #elif (both 
in test_unit_mathops.c and test_unit_rotation.c).


More information about the opus mailing list