On Sun, Jan 02, 2011 at 02:04:11PM +0000, Peter Maydell wrote: > On 2 January 2011 13:23, Aurelien Jarno <aurel...@aurel32.net> wrote: > > On Thu, Dec 16, 2010 at 11:51:17AM +0000, Peter Maydell wrote: > >> IEEE754 doesn't specify precisely what NaN should be returned as > >> the result of an operation on two input NaNs. This is therefore > >> target-specific. Abstract out the code in propagateFloat*NaN() > >> which was implementing the x87 propagation rules, so that it > >> can be easily replaced on a per-target basis. > > > I am basically find with the idea. I have tried to implement that for > > MIPS, but it seems your current implementation doesn't allow correct > > propagation for MIPS: if one of the two operand are a sNaN, the result > > should be a *default* qNaN. > > So if the input is a QNaN it's passed through but if it's an SNaN > you get the default QNaN? I guess it makes sense since MIPS
Correct. > has SNAN_BIT_IS_ONE and you can't just silence a NaN by > flipping the bit (because you might end up with a non-NaN if > the final significand is all-zeroes). [...and the existing code that > tries to do that is therefore wrong, presumably for both MIPS > and HPPA.] This is also correct. We should probably change it to the default sNaN if the resulting significand is all-zeroes. > Could we have a target-specific "silence this SNaN" function? You mean a target-specific version of floatXX_maybe_silence_nan()? At least having a default version (for !SNAN_BIT_IS_ONE) and a version for MIPS, HPPA (which is btw not emulated), etc. > Then the top level functions could use those rather than doing > their own bit-flipping, and I think that would do the right thing > for MIPS (you'd implement silence-NaN as "return the default > QNaN", and you implement pickNaN() to return the SNaN.) It seems, it would be the good way to do it. I am going to do a quick hack to see if this solution can work and if it is the case (it seems to be), apply your patch. > > It seems that we should pass the operands to the pickNaN() function and > > return the result instead of a flag. That means having one pickNaN > > function per float type, but that can probably be handled by macros or > > by having a common function for targets on which its possible to do so. > > As you might have guessed I was definitely trying to avoid having > to actually pass the operands to pickNaN()... I agree with you that if it can be avoided, it's the best option. > > Note however that the current implementation provides the correct > > result, as the result is converted in op_helper.c: > > > > if (GET_FP_CAUSE(env->active_fpu.fcr31) & FP_INVALID) > > wt2 = FLOAT_QNAN32; > > ...incidentally, I don't know MIPS but this code looks a bit suspect to me: > set_float_exception_flags(0, &env->active_fpu.fp_status); \ > wt2 = float32_ ## name (fst0, fst1, &env->active_fpu.fp_status); \ > update_fcr31(); \ > if (GET_FP_CAUSE(env->active_fpu.fcr31) & FP_INVALID) \ > wt2 = FLOAT_QNAN32; \ > > Isn't it clearing the FP exception flags for each instruction when > they ought to be cumulative? > It only clears the softfloat ones, the real ones are kept in CPU register fcr31, which is updated by update_fcr31(). This allows to correctly emulate fcr31, as it contains a part that cumulates IEEE754 flags and can be reset by software, and a part that cumulates exception flags, which triggers exceptions if enabled, and which can be reset independently. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net