On 04/01/2013 03:28 PM, Thomas Martitz wrote:
> Am 01.04.2013 13:13, schrieb Peter Meerwald:
>> Hello Tanu,
>>
>> thanks for reviewing!
>>
>>>>> + case PA_SAMPLE_S16NE:
>>>>> + {
>>>>> + int16_t *d = (int16_t *) dst, *s = (int16_t *) src;
>>>>> +
>>>>> + for (i = n>> 2; i> 0; i--) {
>>>>> + d[0] += (s[0] + s[1])/2;
>>>>> + d[1] += (s[2] + s[3])/2;
>>>>> + d[2] += (s[4] + s[5])/2;
>>>>> + d[3] += (s[6] + s[7])/2;
>>>> You probably meant '=', not '+='?
>>>>
>>>> Also, s[0] + s[1] can overflow, so the inputs should be divided
>>>> individually before summing.
>>> Or the inputs could be cast to uint32_t, which I guess would be better
>>> than dividing multiple times.
>>
>> I think (s[0] + s[1])/2 is correct; at least as long as sizeof(int) >
>> sizeof(short); short gets promoted to int -- see 'integer promotion in
>> C99'; also my compiler thinks above is correct :)
>>
>
>
> Why should int16+int16 be promoted to int32? Pretty sure this code is
> prone to overflow.
I was pretty sure too, until I tried compiling a test program. I think
this text in the C99 standard[1] (section 6.3.1.1) means that shorts are
always promoted to ints, even if both operands of a calculation are shorts:
"The following may be used in an expression wherever an int or unsigned
int may be used:
- An object or expression with an integer type whose integer
conversion rank is less than or equal to the rank of int and unsigned int.
- A bit-field of type _Bool, int, signed int, or unsigned int.
If an int can represent all values of the original type, the value is
converted to an int; otherwise, it is converted to an unsigned int.
These are called the integer promotions. All other types are unchanged
by the integer promotions."
I have trouble understanding that text, but that's the closest thing
that I found that would explain the observed behavior.
There's also this example in section 5.1.2.3:
"EXAMPLE 2
In executing the fragment
char c1, c2;
/* ... */
c1 = c1 + c2;
the "integer promotions" require that the abstract machine promote the
value of each variable to int size and then add the two ints and
truncate the sum. Provided the addition of two chars can be done without
overflow, or with overflow wrapping silently to produce the correct
result, the actual execution need only produce the same result, possibly
omitting the promotions."
[1] http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf
--
Tanu
