On Mon, Dec 28, 2009 at 08:52:28PM +0100, Aurelien Jarno wrote: > On Mon, Dec 28, 2009 at 08:48:22PM +0100, Aurelien Jarno wrote: > > On Mon, Dec 28, 2009 at 11:06:20AM -0800, Richard Henderson wrote: > > > On 12/24/2009 07:18 AM, Aurelien Jarno wrote: > > >> My main concern about this patch is that I don't really understand why > > >> the current fp exceptions, the current rounding mode or flush_to_zero > > >> mode are stored in FP_STATUS. I think it would be better to have > > >> dedicated variable(s) in the cpu state structure, as it is done in other > > >> emulated architectures. > > > > > > The internal format for these features is quite different (in terms of > > > bit ordering) than the native Alpha format. If I were to be forcing the > > > bits back into the Alpha format after each operation, that would be far > > > more shifting and masking than only doing the conversion to Alpha format > > > when the FPCR register is read or written. At present I'm only saving a > > > value and ORing it back in > > > > > > However, if you're convinced I should keep things in Alpha format all > > > the time, I can rearrange the patch along those lines. > > > > > > > If the format is too different, you should still use a dedicated > > variable in the cpu state. Thinking more, there is no guarantee that > > FP_STATUS is not overriden by another thread (or another CPU in system > > mode), if it's value is set in one TB, and read back in another TB. > > Forget that part, there is actually one FP_STATUS per CPU, so it should > work. My grep didn't work due to the #define... > > > Also there is no need to save FP_STATUS and restore it. It should be set > > to 0, and the result should be ORed with the dedicated variable. > >
To make it more clear, what I call save FP_STATUS is all the begin_* functions. They should be one or more variable added into the CPU state to hold those values. For example begin_fp_exception can then be replaced by a simple: set_float_exception_flags(0, &FP_STATUS); And then end_fp_exception the mask and shift operation can be replaced by: env->fp_exceptions |= FP_STATUS.float_exception_flags; There is no need to call set_float_exception_flags(). When reading the FPCR register, the value should be computed from env->fp_exceptions instead. Similarly there is no need to save or switch back to the default roundmode or flush_to_zero after and FP instruction, as long as all FP instructions set them before the actual FP code. It seems to be the case with your patch anyway. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
