From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:36250)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1QNMpp-00073Q-E3
	for qemu-devel@nongnu.org; Fri, 20 May 2011 06:21:34 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1QNMpo-0008HA-G6
	for qemu-devel@nongnu.org; Fri, 20 May 2011 06:21:33 -0400
Received: from mail-iw0-f173.google.com ([209.85.214.173]:62372)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1QNMpo-0008H2-AJ
	for qemu-devel@nongnu.org; Fri, 20 May 2011 06:21:32 -0400
Received: by iwl42 with SMTP id 42so3448618iwl.4
	for <qemu-devel@nongnu.org>; Fri, 20 May 2011 03:21:31 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <1305468801-6015-6-git-send-email-aurelien@aurel32.net>
References: <1305468801-6015-1-git-send-email-aurelien@aurel32.net>
	<1305468801-6015-6-git-send-email-aurelien@aurel32.net>
Date: Fri, 20 May 2011 11:21:31 +0100
Message-ID: <BANLkTimX3j7wM9HgceS4RLx_H0x-NuekjQ@mail.gmail.com>
From: Peter Maydell <peter.maydell@linaro.org>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [Qemu-devel] [PATCH 05/11] target-i386: remove old code
	handling float64
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: Aurelien Jarno <aurelien@aurel32.net>
Cc: qemu-devel@nongnu.org

On 15 May 2011 15:13, Aurelien Jarno <aurelien@aurel32.net> wrote:
> Now that target-i386 uses softfloat, floatx80 is always available and
> there is no need anymore to have code handling both float64 and floax80.
>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

This patch is OK in terms of how it leaves the code, but I think some
parts of it are out of sequence with the rest of the patchset.

For instance:
> -#ifdef FLOATX80
> -#define USE_X86LDOUBLE
> -#endif

We've already removed the FLOATX80 define in a previous patch,
so if we don't delete the x86 use of it until this patch then
the behaviour will briefly flip-flop as you go through the patch
stack, which could be bad for bisection.

> -#if defined(CONFIG_SOFTFLOAT)
> -# define floatx_lg2 make_floatx80( 0x3ffd, 0x9a209a84fbcff799LL )
> -# define floatx_l2e make_floatx80( 0x3fff, 0xb8aa3b295c17f0bcLL )
> -# define floatx_l2t make_floatx80( 0x4000, 0xd49a784bcd1b8afeLL )
> -#else
> -# define floatx_lg2 (0.30102999566398119523L)
> -# define floatx_l2e (1.44269504088896340739L)
> -# define floatx_l2t (3.32192809488736234781L)
> -#endif

Similarly, this #ifdeffery should have gone away when we took
out CONFIG_SOFTFLOAT, not later.

(Also the patch was a bit of a pig to review because it combines
several distinct mostly-mechanical transformations.)

-- PMM