Re: [Qemu-devel] [PATCH 00/16 v1] tilegx: Firstly add tilegx target for linux-user

[Richard Henderson <rth@twiddle.net>]

First off, this isn't v1.  The previous patch set you posted was v12.

On 08/20/2015 02:32 PM, Chen Gang wrote:
> Chen Gang (16):
>    linux-user: tilegx: Firstly add architecture related features
>    linux-user: Support tilegx architecture in linux-user
>    linux-user/syscall.c: conditionalize syscalls which are not defined in
>      tilegx
>    target-tilegx: Add opcode basic implementation from Tilera Corporation
>    target-tilegx/opcode_tilegx.h: Modify it to fit QEMU usage
>    target-tilegx: Add special register information from Tilera
>      Corporation
>    target-tilegx: Add cpu basic features for linux-user
>    target-tilegx: Add several helpers for instructions translation
>    target-tilegx: Generate tcg instructions to finish "Hello world"

This patch is too big.  Way way too big.

We've discussed this before, and you changed the v9 patch set in the correct 
direction (though it wasn't split far enough).  But then you merged them all 
back together in v10.  That's just unacceptable.

>    target-tilegx: Add TILE-Gx building files
>    target-tilegx: Add related feature to support iret instruction
>    target-tilegx: Implement decode_rrr_8_opcode_y0
>    target-tilegx: Use int8_t instead of uint8_t for imm8 in gen_st_add()
>    linux-user: main: Use negative qemu errno for syscall return errno
>    tilegx: Match with the latest qemu master tree

These three patches are fixes for previous patches should have been merged back 
into the respective original patches.

>    target-tilegx: Implement additional instructions in normal working
>      flow

This patch is also too big.

In addition, I believe there's a better way to organize the decoding.  I 
mentioned this before in one of the review cycles, but this was not followed up 
from either your side or mine.  I've spent the evening flushing out the idea 
and will follow up with a patch set.


r~