On Thu, May 23, 2019 at 21:28:51 +0900, Tetsuya Isaki wrote:
> +/*
> + * AUDIO_ASR() does Arithmetic Shift Right operation.
> + * This macro should be used for audio wave data only.
> + *
> + * Division by power of two is replaced with shift operation in the most
> + * compiler, but even then rounding-to-zero occurs on negative value.
> + * What we handle here is the audio wave data that human hear, so we can
> + * ignore the rounding difference. Therefore we want to use faster
> + * arithmetic shift right operation. But the right shift operator ('>>')
> + * for negative integer is "implementation defined" behavior in C (note
> + * that it's not "undefined" behavior). So if implementation defines '>>'
> + * as ASR, we use it.
> + *
> + * Using ASR is 1.9 times faster than division on my amd64, and 1.3 times
> + * faster on my m68k. -- isaki 201801.
> + */
> +#if defined(__GNUC__)
> +/* gcc defines '>>' as ASR. */
> +#define AUDIO_ASR(value, shift) ((value) >> (shift))
> +#else
> +#define AUDIO_ASR(value, shift) ((value) / (1 << (shift)))
> +#endif
This feels inverted. The mathematically correct operation required
here is symmetric division (that rounds towards zero). Arithmetic
shift is flooring division (that rounds towards minus infinity). So
it feels confusing and wrong to *name* the operation the opposite of
what it really is.
-uwe
