Paolo Bonzini <pbonz...@redhat.com> writes: > On 04/11/2015 12:05, Richard Henderson wrote: >> 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. > > That does the job, but it also does look like a typo...
Make the implicit conversion explicit then.
