From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=36287 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PZmHJ-0007uA-PX for qemu-devel@nongnu.org; Mon, 03 Jan 2011 10:24:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PZmHI-0003zs-Dk for qemu-devel@nongnu.org; Mon, 03 Jan 2011 10:24:57 -0500 Received: from hall.aurel32.net ([88.191.126.93]:47750) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PZmHI-0003zg-7r for qemu-devel@nongnu.org; Mon, 03 Jan 2011 10:24:56 -0500 Message-ID: <4D21EA42.7070106@aurel32.net> Date: Mon, 03 Jan 2011 16:24:50 +0100 From: Aurelien Jarno MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 2/6] softfloat: fix float{32, 64}_maybe_silence_nan() for MIPS References: <1294065273-30274-1-git-send-email-aurelien@aurel32.net> <1294065273-30274-3-git-send-email-aurelien@aurel32.net> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-devel@nongnu.org Peter Maydell a écrit : > On 3 January 2011 14:34, Aurelien Jarno wrote: >> On targets that define sNaN with the sNaN bit as one, simply clearing >> this bit may correspond to an infinite value. >> >> Convert it to a default NaN if SNAN_BIT_IS_ONE, as it corresponds to >> the MIPS implementation, the only emulated CPU with SNAN_BIT_IS_ONE. >> When other CPU of this type are added, this might be updated to include >> more cases. > > This patch doesn't apply to master: > >> Signed-off-by: Aurelien Jarno >> --- >> fpu/softfloat-specialize.h | 12 ++++++------ >> 1 files changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h >> index f23bd6a..31481e7 100644 >> --- a/fpu/softfloat-specialize.h >> +++ b/fpu/softfloat-specialize.h >> @@ -107,13 +107,13 @@ int float32_is_signaling_nan( float32 a_ ) >> float32 float32_maybe_silence_nan( float32 a_ ) >> { >> if (float32_is_signaling_nan(a_)) { >> - bits32 a = float32_val(a_); > > ...on master this line is > uint32_t a = float32_val(a_); > > (different type) so the patch doesn't apply. Oops, yes, my patch series should have started by a patch fixing types, but i made a mistake selecting the commits to send. Will fix that in a v2. > Other than that, looks OK. I think I'd like a comment somewhere > along the lines of > /* Rules for silencing a signaling NaN are target-specific. Typically > * targets with !SNAN_BIT_IS_ONE use the rule that the NaN > * is silenced by setting the bit. Targets where SNAN_BIT_IS_ONE > * must do something more complicated, because clearing the > * bit might turn a NaN into an infinity. This code is correct for > * MIPS but new targets might need something different. > */ > > Or you could have the #ifdefs be on TARGET_whatever so > that it's clear (because it won't compile) that adding a new > TARGET_FOO means you have to check behaviour in this > area. But I don't feel very strongly about that. > Ok, thanks for the review, will fix that. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net