Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix

[Richard Henderson <rth@twiddle.net>]

On 11/04/2015 11:45 AM, Paolo Bonzini wrote:
>>>>>>>            int32_t src = rs2 >> (word * 32);
>>>>>>> -        int64_t scaled = src << scale;
>>>>>>> +        int64_t scaled = (int64_t)src << scale;
>>>>>>>            int64_t from_fixed = scaled >> 16;
...
>>
>> I do think we'd be better served by casting to uint64_t on that line.
>> Note that fpackfix requires the same correction.  And it wouldn't hurt
>> to cast to uint32_t in fpack16, lest we anger the self-same shifting gods.
>
> Hmmm.. say src = -0x80000000, scale = 1;
>
> scaled     = (uint64_t)-0x8000000 << 1 = 0xffffffff00000000
> from_fixed = 0xffffffff00000000 >> 16  = 0x0000ffffffff0000
>
> Now from_fixed is positive and you get 32767 instead of -32768.  In
> other words, we would have to cast to uint64_t on the scaled assignment,
> and back to int64_t on the from_fixed assignment.  I must be
> misunderstanding your suggestion.

   int64_t scaled = (uint64_t)src << scale;

I.e. one explicit conversion and one implicit conversion.


r~