[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target-i386: implement FPREM and FPREM1 using s
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH] target-i386: implement FPREM and FPREM1 using softfloat only |
Date: |
Fri, 29 Jun 2012 15:13:43 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120421 Thunderbird/12.0 |
Am 28.06.2012 01:00, schrieb Catalin Patulea:
> Hey guys,
>
> I've been hacking up the FPREM and FPREM1 i386 instructions (without KVM) to
> be implemented using SoftFloat only. This was motivated by some correctness
> issues
> that we noticed with the current implementation which follows the AMD
> datasheet.I believe the datasheet explains the operation as if intermediate
> results had "infinite" precision, but in this case intermediate rounding
> causes a loss of precision at the final output.
>
> My reference was TestFloat [1], specifically the floatx80_rem suite, which
> tests the FPREM instruction quite thoroughly (FPREM1 is technically untested
> but very similar). The current code fails about half the suite; with the
> patch, all tests pass.
>
> There are still lots of dark corners - the code originates from Bochs' copy
> of SoftFloat and I tried to port the Bochs exception handling logic as much
> as I could. Many details still elude me though (see comments in the code).
> TestFloat does test some of the exception logic but not as thoroughly as I
> would have liked. If anyone can offer some guidance here, I am happy to fix
> up the patch.
>
> That's about it.. let me know what you think.
>
> Catalin
>
> [1] http://www.jhauser.us/arithmetic/TestFloat.html
>
> ---
> fpu/softfloat.c | 182
> +++++++++++++++++++++++++++++++++++++++++++++++
> fpu/softfloat.h | 3 +
> target-i386/op_helper.c | 163 +++++++++++++++++-------------------------
> 3 files changed, 249 insertions(+), 99 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index b29256a..9c6e4a3 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -5234,6 +5234,16 @@ int floatx80_unordered_quiet( floatx80 a, floatx80 b
> STATUS_PARAM )
> }
>
>
> /*----------------------------------------------------------------------------
> +| Returns 1 if the extended double-precision floating-point value `a' is an
> +| unsupported; otherwise returns 0.
> +*----------------------------------------------------------------------------*/
> +int floatx80_is_unsupported(floatx80 a)
> +{
> + return (extractFloatx80Exp(a) &&
> + !(extractFloatx80Frac(a) & LIT64(0x8000000000000000)));
> +}
Disclaimer: It's been a while that I had to learn IEEE 754.
Unsupported for some i386 instruction? Or unsupported in the SoftFloat
library due to some limitation?
Please run scripts/checkpatch.pl for CODING_STYLE issues, I spotted one
slightly misplaced if brace, and one empty line at the bottom seemed to
have indentation (git-am complains about that, too).
The patch description will need to be cleaned up (not be letter-style).
I'm not too thrilled to introduce more uses of int32 (we started
converting int16 to int_fast16_t) but I won't veto. It would be nice if
you could review for any potential int32 vs. int32_t breakages though,
to not make things worse than they are.
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg