From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:36057)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1ZtxSI-00033Z-94
	for qemu-devel@nongnu.org; Wed, 04 Nov 2015 07:46:23 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1ZtxSF-0002wW-1D
	for qemu-devel@nongnu.org; Wed, 04 Nov 2015 07:46:22 -0500
Received: from mx1.redhat.com ([209.132.183.28]:49740)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1ZtxSE-0002wM-Rp
	for qemu-devel@nongnu.org; Wed, 04 Nov 2015 07:46:18 -0500
References: <1446473134-4330-1-git-send-email-pbonzini@redhat.com>
	<CAFEAcA97rV3sZTZ085uLKnTc5_hQeJVrnN0CHt97ryzxk5iwGA@mail.gmail.com>
	<563777D5.6050000@redhat.com>
	<CAFEAcA-e23HiTvxji+y5LetLY2S66MGPEdfok4vLjJFou_6FKw@mail.gmail.com>
	<5639DA14.3020507@twiddle.net> <5639E1C2.80902@redhat.com>
	<5639E691.4050203@twiddle.net>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <5639FE12.9000901@redhat.com>
Date: Wed, 4 Nov 2015 13:46:10 +0100
MIME-Version: 1.0
In-Reply-To: <5639E691.4050203@twiddle.net>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in
	fpackfix
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Richard Henderson <rth@twiddle.net>, Peter Maydell <peter.maydell@linaro.org>
Cc: Blue Swirl <blauwirbel@gmail.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, QEMU Developers <qemu-devel@nongnu.org>



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...

Paolo