Re: [Qemu-devel] [PATCH 1/6] softfloat: remove HPPA specific code

[Peter Maydell]

On 6 January 2011 18:13, Stuart Brady <address@hidden> wrote:
> On Thu, Jan 06, 2011 at 08:58:17AM +0000, Peter Maydell wrote:
>> On 5 January 2011 23:13, Stuart Brady <address@hidden> wrote:
>> > I do have a few concerns regarding SoftFloat, though:
>> >
>> >   FIXMEs should be left in the code (or a document maintained on the
>> >   Wiki) to keep track of which architectures have been considered
>> >   (which I believe are x86, arm, mips, ppc) and which ones haven't.
>> >   This is in reference to one particular FIXME that was removed,
>> >   but perhaps shouldn't have been.
>>
>> Which one? The only one I know I removed was the one in
>> the patch that implemented the thing it was complaining about,
>> but perhaps I took out another by accident...
>
> The patch is question was:
>
>   softfloat: Implement flushing input denormals to zero
>
> The comment is:
>
>   /* FIXME: Flush-To-Zero only effects results.  Denormal inputs should
>      also be flushed to zero.  */

The point of that FIXME is that it is saying "softfloat doesn't implement
the feature of flushing denormal inputs to zero". The patch implements
that feature in softfloat. Therefore the FIXME should be removed,
because it has been fixed :-)

> Do you know whether ARM is the only target architecture supported by
> QEMU that requires this behaviour?

As I said in the patch series cover letter, I suspect you could implement
a MIPS FCCR bit using the softfloat feature the patch adds. But that's
totally irrelevant to whether that FIXME comment should be deleted,
because that FIXME isn't about any architecture but about a softfloat
feature (or lack of it). Existing architectures are no more or less broken
than they were before, because they are unaffected by the patch series.

> If there are any architectures where we simply don't know whether the
> current behaviour is correct, we should document that somewhere.

That would be all of them, I rather suspect. I don't imagine anybody
has run any of the qemu emulated models through a rigorous
validation process for any architecture, so we can't say "we know
this is correct". I'm going through fixing things for ARM as I encounter
them and I have a huge list of corner cases where we don't do the
right thing.

>> >   Is there any plan to deal with use of float*_is_quiet_nan(), where
>> >   float*_is_any_nan() was intended?  These should really either be
>> >   fixed (and tested), or if not, a FIXME should be added.
>>
>> I was planning to do the ARM related ones, at least (although
>> they are in the pretty-much-dead FPE-emulation code for
>> linux user-mode).

I've now submitted a patchset to do that.

-- PMM