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
