[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/7] softfloat: Fix sNaN handling in FP conversi
Maciej W. Rozycki
Re: [Qemu-devel] [PATCH 1/7] softfloat: Fix sNaN handling in FP conversion operations
Fri, 6 Feb 2015 19:35:31 +0000 (GMT)
Alpine 2.11 (LFD 23 2013-08-11)
On Fri, 6 Feb 2015, Peter Maydell wrote:
> > What I think would make sense here is instead of say `float32_to_float64'
> > making a call to `float64_maybe_silence_nan' directly, we'd have a static
> > inline function or a macro called say `float64_convert_silence_nan'
> > invoked where the former is in my patch proposed. That function in turn
> > would call the former function by default and which could be overridden by
> > something else for individual targets that require it.
> This still doesn't work for ARM, because you can't do the
> silencing after conversion. You have to have a way for
> targets to put in target-specific behaviour directly in the
> 64-to-32 conversion process, before data is lost in the narrowing.
Somehow I missed from your description that ARM actually requires actions
at an earlier stage, sorry. What I understood is that it is only the sign
bit that is lost, and that could be easily handled with a bespoke version
of `float64_convert_silence_nan', etc. Is that the sign bit is supposed
to be preserved in quietening an sNaN iff truncation of a non-zero wider
trailing significand field produced all-zeros in the narrower result, and
otherwise set to zero then?
> > And I think we shouldn't be making our decisions based on x86 despite its
> > ubiquity -- it is, especially as the x87 implementation is concerned, the
> > earliest attempt to implement IEEE 754 with all the oddities resulting.
> > Or more specifically not even that but more of an FP implementation that
> > (as of the 8087 FPU) Intel wanted to become the basis of the upcoming IEEE
> > 754 standard, and mostly succeeded in principle, but not in some details,
> > some of which were later corrected with the 80287 and 80387
> > implementations, and some of which had to stay, for compatibility reasons.
> My point is that you mustn't regress other targets. So either you
> need to retain the same behaviour for targets which don't specifically
> add new code/flags/whatever, or you need to go through and look up
> all the other architecture specifications to check that you haven't
> broken them (ie that any changes you make are actually improving their
> accuracy). This patch isn't doing the first of those, so you need
> to do the second...
I agree making sure no target has regressed is a valuable goal, but TBH
I'd welcome some cooperation from target maintainers here, especially as
the current situation looks like core code is non-compliant and I find it
hard to believe all the processors out there (except MIPS) got their IEEE
754 implementation wrong. So what this change does I'd expect actually to
*progress* most of the targets, except from maybe a couple of oddballs.
And I would very much appreciate feedback from target maintainers of
these oddball architectures, after all they know the internals of the
processors they care of the best!
> > Besides are you sure?
> I was asking if you'd checked all the targets whose behaviour you're
> changing, not making a definite "you have broken x86" statement.
> x86 is just the most likely because as you say it is the weirdest
> and least compliant.
We can surely sort out the level of support x87 requires in further
updates to the proposed change. I'd expect x86's more modern SSE
implementation of IEEE 754, that I'm less familiar with (which I guess
just shows my age), got the details just as the standard expects them,
which in turn implies more complication on our side. But if we set the
minimum at 80387, then as it stands it does not appear to me that this
change will regress anything.
But as I say, I think it would be most productive if target maintainers
of at least ARM and x86, and maybe other architectures as well if they
have quirks in this area, chimed in and identified issues. I think it is
impractical to require submitters of a generic change like this to chase
architecture manuals of all the processors affected and study and
understand their instruction sets to infer whether the change is going to
have a negative effect. Especially given that from the current code and
the standard it is supposed to implement it is clear to me that the code
I also think that papering the issue over by fixing the thing up somehow
in the MIPS TCG and letting other people worry about their architecture of
interest is not the way to go, although it clearly looks like the path of
least resistance to me.
> >> I think this means that:
> >> (1) we want to put handling of silencing the signaling NaNs
> >> into the NaN conversion functions themselves (since it's
> >> too late to do it correctly once the commonNaNtoFloat16
> >> function has thrown away data)
> >> (2) that handling needs to be target specific, as most details
> >> of quiet vs signaling NaNs are
> >> I note in passing that the float*ToCommonNaN and commonNaNToFloat*
> >> functions are used only in the back-to-back call sequence of
> >> return commonNaNToFloatX(floatYToCommonNaN(...))
> >> for handling the NaN inputs on FP-to-FP conversion.
> > I believe my proposal addresses your concerns in a burden-free way for
> > target maintainers who look after processors that implement the IEEE 754
> > standard as it stands.
> I don't, which is why I made the comment above. If you're going to
> fix up NaN handling in the float-to-float conversion routines we
> should do it in a way that lets us handle the behaviour of
> target CPUs we care about.
No argument about maintaining correct emulation where it already is such,
sure. Please note that `float64_maybe_silence_nan' and friends are
already target-specific, which should generally let us deal with things,
and I am sort of surprised ARM sets certain rules for sNaN silencing for
conversions and different ones for other arithmetic operations. Or is it
really that an input sNaN's sign bit is propagated in the single weird
corner case only?