Re: [Qemu-devel] [PATCH v3] Execute _start and reach to the first function call successfully

[Chen Gang]

On 3/16/15 23:31, Richard Henderson wrote:
> On 03/13/2015 11:03 PM, Chen Gang wrote:
>> +static void decode_rrr_1_opcode_y0(struct DisasContext *dc,
>> +                                   tilegx_bundle_bits bundle)
>> +{
>> +    switch (get_RRROpcodeExtension_Y0(bundle)) {
>> +    case UNARY_RRR_1_OPCODE_Y0:
>> +        switch (get_UnaryOpcodeExtension_Y0(bundle)) {
>> +        case  FNOP_UNARY_OPCODE_Y0:
>> +            if (!get_SrcA_Y0(bundle) && !get_Dest_Y0(bundle)) {
>> +                gen_fnop();
>> +                return;
>> +            }
>> +            break;
>> +        default:
>> +            break;
>> +        }
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +
>> +    qemu_log_mask(LOG_UNIMP, "UNIMP rrr_1_opcode_y0, %16.16llx\n", bundle);
>> +    dc->exception = TILEGX_EXCP_OPCODE_UNIMPLEMENT;
>> +}
> 
> I think it would be helpful if, in the first patch, you put all of the opcode
> symbols into the proper place in the switch statements.  That way it's easy to
> tell at a glance what has yet to be implemented.  For instance, for this
> function you'd put
> 
> $ grep _UNARY opcode_tilegx.h  | grep _Y0
>   CNTLZ_UNARY_OPCODE_Y0 = 1,
>   CNTTZ_UNARY_OPCODE_Y0 = 2,
>   FNOP_UNARY_OPCODE_Y0 = 3,
>   FSINGLE_PACK1_UNARY_OPCODE_Y0 = 4,
>   NOP_UNARY_OPCODE_Y0 = 5,
>   PCNT_UNARY_OPCODE_Y0 = 6,
>   REVBITS_UNARY_OPCODE_Y0 = 7,
>   REVBYTES_UNARY_OPCODE_Y0 = 8,
>   TBLIDXB0_UNARY_OPCODE_Y0 = 9,
>   TBLIDXB1_UNARY_OPCODE_Y0 = 10,
>   TBLIDXB2_UNARY_OPCODE_Y0 = 11,
>   TBLIDXB3_UNARY_OPCODE_Y0 = 12,
> 
> in the get_UnaryOpcodeExtension_Y0 switch statement and
> 
> $ grep _RRR_1 opcode_tilegx.h  | grep _Y0
>   SHL1ADD_RRR_1_OPCODE_Y0 = 0,
>   SHL2ADD_RRR_1_OPCODE_Y0 = 1,
>   SHL3ADD_RRR_1_OPCODE_Y0 = 2,
>   UNARY_RRR_1_OPCODE_Y0 = 3,
> 
> in the get_RRROpcodeExtension_Y0 switch statement.
> 
> Likewise with all of the other "decode" functions that contain a switch.
> 

OK, thanks. It is a good idea to me.


>> +    qemu_log("fnop\n");
>> +    qemu_log("addi r%d, r%d, %d\n", rdst, rsrc, imm8);
> 
> Again, use qemu_log_mask(CPU_LOG_TB_IN_ASM).
> 

OK, thanks. I shall use qemu_log_mask in all areas within tilegx.

Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed