From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38607) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z9pxC-000100-EN for qemu-devel@nongnu.org; Tue, 30 Jun 2015 03:27:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z9px9-0007WM-8R for qemu-devel@nongnu.org; Tue, 30 Jun 2015 03:27:38 -0400 Received: from cantor2.suse.de ([195.135.220.15]:49022 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z9px9-0007WE-1k for qemu-devel@nongnu.org; Tue, 30 Jun 2015 03:27:35 -0400 Message-ID: <559244E5.2080908@suse.de> Date: Tue, 30 Jun 2015 09:27:33 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1435259818-6864-1-git-send-email-aurelien@aurel32.net> <55910FCB.30900@twiddle.net> <20150629202401.GN931@aurel32.net> <55922EF6.4050605@twiddle.net> In-Reply-To: <55922EF6.4050605@twiddle.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] target-s390x: fix CONVERT TO BINARY (CVD, CVDY) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson , Aurelien Jarno Cc: qemu-devel@nongnu.org On 30.06.15 07:53, Richard Henderson wrote: > On 06/29/2015 09:24 PM, Aurelien Jarno wrote: >> On 2015-06-29 10:28, Richard Henderson wrote: >>> On 06/25/2015 08:16 PM, Aurelien Jarno wrote: >>>> for (shift =3D 4; (shift < 64) && bin; shift +=3D 4) { >>>> - int current_number =3D bin % 10; >>>> - >>>> - dec |=3D (current_number) << shift; >>>> + dec |=3D (bin % 10) << shift; >>>> bin /=3D 10; >>>> } >>> >>> You've changed from 32-bit division to 64-bit division just to solve = a >>> problem with the shift. Better to just change the type of >>> current_number >>> there. >> >> Changing the type of current_number instead of bin would indeed solve >> the shift issue, but not the -2^31 case. As we take the absolute value= , >> we need a 64-bit variable to hold the corresponding 2^31 value. >> >=20 > Ah, true enough. I suppose adding a 32-bit unsigned variable with whic= h > to do the division is more trouble than it's worth. >=20 > Reviewed-by: Richard Henderson Thanks, applied to s390-next. Alex