[Qemu-devel] Re: [PATCH 0/6] tcg conditional set/move, round 2

[Laurent Desnogues <laurent.desnogues@gmail.com>]

On Thu, Dec 17, 2009 at 8:32 PM, Richard Henderson <rth@twiddle.net> wrote:
>> funny how you seem to follow the same paths I did months ago :-)
>
> Heh.  They are the obvious points for improvement in the emulation.
> Hopefully we can get them finished this time, so that some third
> person doesn't go through the same thing next year.  =)

I agree :-)

>> - cosmetics: don't use tabs and be sure not to have
>>   whitespaces at end of lines
>
> Grr, I thought I'd been extra careful about that.  Fixed.
>
>> - the changes to tcg-op.h in patch 6 should go with patch 1
>
> Done.
>
>> - outside of the small parameter handling, I'd prefer
>>   you provide a separate patch for tcg_out_brcond2;
>>   I don't think the changes using c1, c2 and c3 really
>>   belong to setcond/movcond.
>
> The i386 part has been split into 3 patches, though they
> are sequentially dependent.
>
> I've left off the sparc backend patch, as well as the patches
> for the translators.  Let's take care of these first.

Thanks, that will make things easier.

>
> r~
> ---
>
> Richard Henderson (6):

This is a first pass of review.  It's far complete (in particular no
testing was done).

Here is a summary of my comments:

>  tcg: Generic support for conditional set and conditional move.

Needs cosmetics changes.

>  tcg: Add tcg_invert_cond.

OK.

>  tcg-x86_64: Implement setcond and movcond.

Some cosmetics and comments, but overall good.

>  tcg-i386: Implement small forward branches.

I think this contains a bug.

>  tcg-i386: Simplify brcond2.

I don't like the rewrite of brcond2.

>  tcg-i386: Implement setcond, movcond, setcond2.

Not yet reviewed.


Laurent

>
>  elf.h                   |    2 +
>  tcg/README              |   26 ++++-
>  tcg/i386/tcg-target.c   |  278 ++++++++++++++++++++++++++++++++++++++--------
>  tcg/tcg-op.h            |   91 +++++++++++++++
>  tcg/tcg-opc.h           |    5 +
>  tcg/tcg.c               |   23 +++-
>  tcg/tcg.h               |    5 +
>  tcg/x86_64/tcg-target.c |   65 +++++++++++-
>  8 files changed, 436 insertions(+), 59 deletions(-)
>
>