[libre-riscv-dev] [Bug 117] RISCV FCLASS instruction needed

[bugzilla-daemon at libre-riscv.org]

http://bugs.libre-riscv.org/show_bug.cgi?id=117

--- Comment #10 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Jacob Lifshay from comment #7)
> (In reply to Luke Kenneth Casson Leighton from comment #6)
> > ok added fclass test, including adding some extra functions to
> > FPFormat, jacob can you verify the logic there, i had to add
> > not just the hardware-check, there's no softfloat-3 "fclass"
> > function so had to write that as well: that's unverified-check
> > against unverified-check.
> 
> FPFormat.is_nan is either misimplemented or misnamed/misdocumented. as
> written, it detects only quiet NaNs. to detect any NaN, the exponent has to
> be the max value and the mantissa non-zero.

done.  created is_quiet_nan as well

> 
> the documentation on FPFormat.mantissa_mask and FPFormat.is_zero is
> incorrect.

sorted.

> I would rename get_mantissa to get_mantissa_field, 

agreed, done.

> since it seems reasonable
> to add a get_mantissa_value function that includes the implicit 1 bit
> (except on denormals/zero -- implicit 0 bit for those cases) since that is
> the mathematical mantissa value, not the version missing the implicit bit.

good idea - leaving it for now though

> FPFormat.get_sign should be named get_sign_field and the docs adjusted,
> since I would expect get_sign to return 1 or -1, not 0 or 1.

yep, done

> is_subnormal needs a ')' in the doc string

done.

> I think I probably messed up this one: the doc string for mantissa_mask says
> exponent field

corrected.

> not that it matters much: FPFormat.is_nan_signalling: spell using one l,
> since that's how it's spelled in IEEE 754

done.

> handy addition: add a get_exponent_field that returns the bits without
> subtracting the bias and implement in terms of that, 

done.

> just compare
> get_exponent_field with exponent_(inf_nan|denormal_zero) without needing
> emax or e_sub, since that saves a subtraction and matches the rest of the
> class.

mmm... not keen on that: the hardware version always subtracts the exponent
bias (everywhere), which means that FPFormat is the odd one out.

> maybe add a exponent_mask property, like mantissa_mask, then implement
> get_exponent_field as:
> def get_exponent_field(self, x):
>     """ Returns the biased exponent of its input number, x. """
>     return (x & self.exponent_mask) >> self.m_width

yep done, and used it to create get_exponent (which _does_ subtract the bias)

> other than all that, FPFormat appears to be correct

cool.

does test_fpclass_pipe.py look reasonable, i.e. conformant to the FCLASS
spec?

-- 
You are receiving this mail because:
You are on the CC list for the bug.