Re: [Qemu-devel] [patch] target-alpha: squashed fpu qualifiers patch

[Aurelien Jarno]

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
address@hidden                 http://www.aurel32.net