After thinking of again, for me, I still prefer to keep gen_cntlz() and others, the reason is below:
- gen_* (include gen_cntlz) are used in multiple areas, and most gen_* are not single statement. For each gen_*, printing insns is easy (and may be helpful). - decode* is for switch opcode (as branch), not for implementation (as leaf). - After we use individual functions for all unary opcode extensions, we can let decode* very simple (although they may have long code), if we let decode* printing insns, it will let them seem a little complex. Thanks. On 6/2/15 04:54, Chen Gang wrote: >> I can't help thinking, as I read all of these decode functions, that it would >> > be better if the output disassembly, i.e. qemu_log_mask(CPU_LOG_TB_IN_ASM, >> > *), >> > were to happen here, instead of being spread across 99 other functions. >> > >> > This has a side effect of reducing many of your functions to a single >> > statement, invoking another tcg generator, at which point it's worth >> > inlining them. >> > > OK, thanks. > >> > For example: >> > >> > static void decode_rrr_1_unary_y0(struct DisasContext *dc, >> > tilegx_bundle_bits bundle, >> > uint8_t rdst, uint8_t rsrc) >> > { >> > unsigned ext = get_UnaryOpcodeExtension_Y0(bundle); >> > const char *mnemonic; >> > TCGv vdst, vsrc; >> > >> > if (ext == NOP_UNARY_OPCODE_Y0 || ext == FNOP_UNARY_OPCODE_Y0) { >> > if (rsrc != 0 || rdst != 0) { >> > goto unimplemented; >> > } >> > qemu_log_mask(CPU_LOG_TB_IN_ASM, "(f)nop\n"); >> > return; >> > } >> > >> > vdst = dest_gr(dc, rdst); >> > vsrc = load_gr(dc, rsrc); >> > >> > switch (ext) { >> > case CNTLZ_UNARY_OPCODE_Y0: >> > gen_helper_cntlz(vdst, vsrc); >> > mnemonic = "cntlz"; >> > break; >> > case CNTTZ_UNARY_OPCODE_Y0: >> > gen_helper_cnttz(vdst, vsrc); >> > mnemonic = "cnttz"; >> > break; >> > case FSINGLE_PACK1_UNARY_OPCODE_Y0: >> > case PCNT_UNARY_OPCODE_Y0: >> > case REVBITS_UNARY_OPCODE_Y0: >> > case REVBYTES_UNARY_OPCODE_Y0: >> > case TBLIDXB0_UNARY_OPCODE_Y0: >> > case TBLIDXB1_UNARY_OPCODE_Y0: >> > case TBLIDXB2_UNARY_OPCODE_Y0: >> > case TBLIDXB3_UNARY_OPCODE_Y0: >> > default: >> > unimplemented: >> > qemu_log_mask(LOG_UNIMP, "UNIMP rrr_1_unary_y0, [" FMT64X "]\n", >> > bundle); >> > dc->exception = TILEGX_EXCP_OPCODE_UNIMPLEMENTED; >> > return; >> > } >> > >> > qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s r%d,r%d\n", >> > mnemonic, rdst, rsrc); >> > } >> > >> > static void decode_rrr_1_opcode_y0(struct DisasContext *dc, >> > tilegx_bundle_bits bundle) >> > { >> > unsigned ext = get_RRROpcodeExtension_Y0(bundle); >> > uint8_t rsrca = get_SrcA_Y0(bundle); >> > uint8_t rsrcb = get_SrcB_Y0(bundle); >> > uint8_t rdst = get_Dest_Y0(bundle); >> > const char *mnemonic; >> > TCGv vdst, vsrca, vsrcb; >> > >> > if (ext == UNARY_RRR_1_OPCODE_Y0) { >> > decode_rrr_1_unary_y0(dc, bundle, rdst, rsrc); >> > return; >> > } >> > >> > vdst = dest_gr(dc, rdst); >> > vsrca = load_gr(dc, rsrca); >> > vsrcb = load_gr(dc, rsrcb); >> > >> > switch (ext) { >> > case SHL1ADD_RRR_1_OPCODE_Y0: >> > gen_shladd(vdst, vsrca, vsrcb, 1, 0); >> > mnemonic = "shl1add"; >> > break; >> > case SHL2ADD_RRR_1_OPCODE_Y0: >> > gen_shladd(vdst, vsrca, vsrcb, 2, 0); >> > mnemonic = "shl2add"; >> > break; >> > case SHL3ADD_RRR_1_OPCODE_Y0: >> > gen_shladd(vdst, vsrca, vsrcb, 3, 0); >> > mnemonic = "shl3add"; >> > break; >> > default: >> > qemu_log_mask(LOG_UNIMP, "UNIMP rrr_1_opcode_y0, [" FMT64X "]\n", >> > bundle); >> > dc->exception = TILEGX_EXCP_OPCODE_UNIMPLEMENTED; >> > return; >> > } >> > qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s r%d,r%d,r%d\n", >> > mnemonic, rdst, rsrca, rsrcb); >> > } >> > > OK, thank you very much. > -- Chen Gang Open, share, and attitude like air, water, and life which God blessed