Re: [Qemu-devel] tcg conditional set/move, round 3

[Aurelien Jarno]

On Sat, Dec 19, 2009 at 02:03:46PM +0100, Aurelien Jarno wrote:
> On Fri, Dec 18, 2009 at 01:38:24PM -0800, Richard Henderson wrote:
> > On 12/18/2009 03:37 AM, Laurent Desnogues wrote:
> >>>   tcg: Generic support for conditional set and conditional move.
> >>
> >> Needs cosmetics changes.
> >
> > Fixed, attachment 1.
> >
> >>>   tcg-x86_64: Implement setcond and movcond.
> >>
> >> Some cosmetics and comments, but overall good.
> >
> > Fixed, attachment 2.
> >
> >>>   tcg-i386: Implement small forward branches.
> >>
> >> I think this contains a bug.
> >
> > Fixed, attachment 3.  I've added an abort to patch_reloc to verify that  
> > the relocation is in range.  I've propagated the "small" flag to all of  
> > the branch functions so that...
> >
> >>>   tcg-i386: Simplify brcond2.
> >>
> >> I don't like the rewrite of brcond2.
> >
> > ... this patch is dropped.
> >
> >>>   tcg-i386: Implement setcond, movcond, setcond2.
> >>
> >> Not yet reviewed.
> >
> > Fixed, attachment 4.  Similar changes to the amd64 patch.
> >
> 
> 
> Could you please send the patches inline instead. It makes them easier
> to comment. 
> 
> Please find my comments here:
> - I am fine with the setcond instruction
> - For the movcond instruction, is there a real use for vtrue and vfalse
>   values? Most CPU actually implement a version with one value.
>   Implementing it with two values moves complexity within the arch
>   specific tcg code.
> - Do we really want to make movcond mandatory? It can be easily
>   implemented using setcond, and, or instructions.
> - The cmov instruction is not supported on all i386 CPU. While it is
>   unlikely that someone runs qemu-system on such a CPU, it is not
>   improbable that someone runs qemu-user on such a CPU. We should
>   probably provide an alternative code and a check in configure (e.g.
>   trying to compile asm inline code containing a cmov instruction).
 
Forget about that, I read the i386 patch to quickly.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net