On 8 February 2011 17:51, Christophe Lyon <christophe.l...@st.com> wrote:
> On 08.02.2011 16:59, Peter Maydell wrote:
>> This is float32_to_float16() but it looks like you've missed
>> float16_to_float32() ?
>
> Well, actually I somewhat managed to persuade myself that it was not
> necessary to patch float16_to_float32(), but looking at it again it seems
> there are more bugs in that function:
> - the ieee parameter shouldn't be used (my ARM ARM does not mention a
> different behaviour when AH bit is set)
In the ARM ARM pseudocode, FPUnpack() of a 16 bit value looks
at the FPSCR AH bit, so float16_to_float32() does need to depend
on the ieee flag.
> - it calls float32ToCommonNaN(a) where 'a' is a float16 instead of a float32
It's only doing that to raise the exception, but you're right that
it ought to do things properly...
> - this means implementing float16ToCommonNaN, thus float16_is_signaling_nan()
...like that, yes.
I'm just working through a set of bugs in float32_to_float16
and float16_to_float32 where they generally mishandle
various things.
I have also just noticed this error in your patch:
+static bits16 commonNaNToFloat16( commonNaNT a STATUS_PARAM)
+{
+ bits16 mantissa = a.high>>57;
That should be ">> 54". (We're shifting a mantissa which is
in a 64 bit value down so that the top bits of it end up in
the bottom 10 bits (corresponding to the size of mantissa
on a float16); so we want to shift by 64-10 == 54.)
-- PMM
