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