On 04/26/2018 05:40 AM, Alex Bennée wrote:
> + bool arm_hp = !ieee && (src_sz == 16);
Surely you should be testing the destination format.
The float16_unpack_canonical step should be handling
the (non-)special cases to get into FloatParts.
It would probably be better to invert the parameter to be arm_hp rather than
ieee; that way you don't need to test the format; it'll be implied.
> + /* Our only option now is to "re-pack" the NaN. As the
> + * canonilization process doesn't mess with fraction bits for
> + * NaNs we do it all here
> + */
> + switch (src_sz) {
> + case 64:
> + break;
> + case 32:
> + a.frac = deposit64(0, 29, 22, extract64(a.frac, 0, 22));
> + break;
> + case 16:
> + a.frac = deposit64(0, 42, 9, extract64(a.frac, 0, 9));
> + break;
> + default:
> + g_assert_not_reached();
> + break;
> + }
> +
> + /* Get all the frac bits we need with MSB set */
This is assuming QNaN has MSB set. We also support SNaN with MSB set.
> + switch (dst_sz) {
> + case 64:
> + a.frac = (1ULL << 51) | extract64(a.frac, 0, 51);
> + break;
> + case 32:
> + a.frac = (1 << 22) | extract64(a.frac, 29, 22);
> + break;
> + case 16:
> + a.frac = (1 << 9) | extract64(a.frac, 42, 9);
> + break;
> + default:
> + g_assert_not_reached();
> + break;
> + }
This would be better with
a.frac = a.frac << (64 - src->frac_size) >> (64 - dst->frac_size);
a.cls = float_class_msnan;
which has no such magic numbers.
> + } else if (a.cls == float_class_inf && arm_hp) {
> + /* FPProcessException(FPExc_InvalidOp, fpcr); */
> + s->float_exception_flags |= float_flag_invalid;
> + /* Alt HP returns result = sign:Ones(M-1) - faking qnan
> + * forces round_canonical to use frac for the final bits
> + */
> + a.cls = float_class_qnan;
> + a.frac = -1;
Wow, that's... ugly. I would really prefer that you put the correct values
into the structure.
You'll need to pass ieee into float16_round_pack_canonical anyway, since
otherwise you'll get the wrong results for e.g. 1.111p31 (overflows to inf with
ieee hp; representable with armhp).
r~
