From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58939) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zu7cC-0000ZQ-P6 for qemu-devel@nongnu.org; Wed, 04 Nov 2015 18:37:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zu7c9-0001aN-Go for qemu-devel@nongnu.org; Wed, 04 Nov 2015 18:37:16 -0500 Received: from s16892447.onlinehome-server.info ([82.165.15.123]:41051) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zu7c9-0001aJ-AG for qemu-devel@nongnu.org; Wed, 04 Nov 2015 18:37:13 -0500 Message-ID: <563A968D.705@ilande.co.uk> Date: Wed, 04 Nov 2015 23:36:45 +0000 From: Mark Cave-Ayland MIME-Version: 1.0 References: <1446473134-4330-1-git-send-email-pbonzini@redhat.com> <563777D5.6050000@redhat.com> <5639DA14.3020507@twiddle.net> <5639E1C2.80902@redhat.com> <5639E691.4050203@twiddle.net> 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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson , Paolo Bonzini , Peter Maydell Cc: Blue Swirl , QEMU Developers On 04/11/15 11: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. I suspect Richard knows more about this part of SPARC emulation than I do, so I'd be fine with a solution similar to the above if everyone agress. Let me know if you need me to send a SPARC pull request, although it will probably be quicker coming from Paolo/Richard at the moment. ATB, Mark.