Re: [Qemu-devel] qemu host-utils.c

[Thiemo Seufer]

J. Mayer wrote:
> 
> On Wed, 2007-10-24 at 18:37 +0100, Thiemo Seufer wrote:
> > J. Mayer wrote:
> > > 
> > > On Wed, 2007-10-24 at 12:20 +0200, Fabrice Bellard wrote:
> > > > I strongly suggest to reuse my code which was in target-i386/helper.c 
> > > > revision 1.80 which was far easier to validate. Moreover, integer 
> > > > divisions from target-i386/helper.c should be put in the same file.
> > > 
> > > I fully agree with this. I still use the same code in the PowerPC
> > > op_helper.c file because I never conviced myself that the host_utils
> > > version was bug-free. I would likely switch to the common version if I
> > > could be sure it cannot lead to any regression.
> > 
> > Like this? Questions/Comments I have:
> 
> [...]
> 
> > - The x86-64 assembler is untested for this version, could you check
> >   it works for you?
> 
> I did a small test program, comparing the result of the Fabrice
> implementation and the x86_64 optimized implementation results in signed
> and unsigned case. I used the code from the CVS from host-utils.c for
> the optimized case and from target-ppc/op_helper.c for the C code case.
> For my tests vectors, I first used a "walking-one" like pattern
> generation algorithm (including the 0 argument cases) then purely random
> numbers. I did more than 2^32 tests with no differences between the two
> implementations.

Thanks.

> What I suggest, to be safe:
> - do not change the current host-utils API and keep the x86_64 optimised
> case as it is. This way, we are sure not to break anything.

If with API you mean the change in argument order, I reverted to
Fabrice's original x86-64 API definition, which happens to be the
same as used on ppc. I'm reasonably confident that mips64 is correct.

> - just merge Fabrice's code to replace the non-x86_64 code.
> As using this API could lead to more optimisations in the PowerPC
> implementation code, I can wait for you to commit this part and remove
> the "private" helpers as soon as you'll have commited.

I left out the ppc changes from my commit.

> I will then also sanitize the Alpha case, which seems broken, even when
> running on 64 bits hosts.
> I don't know much for Sparc, then I won't change it. 

That comment was meant to point out a bug, not to urge you to do all
the work. :-)


Thiemo