On 10/12/18 10:30 AM, Bastian Koppelmann wrote: > +# Argument sets: > +&cl rs1 rd > +&cl_dw uimm rs1 rd > +&ciw nzuimm rd > +&cs rs1 rs2 > +&cs_dw uimm rs1 rs2
I guess this is good enough for now. What I'd like to see is something like &i imm rs1 rd !extern which tells decodetree that the arg_i structure has been declared elsewhere. Then you can write > +@cl_w ... ... ... .. ... .. &i imm=%uimm_cl_w rs1=%rs1_3 rd=%rs2_3 lw 010 ... ... .. ... 00 @cl_w and then the same trans_lw can be used. > +c_flw_ld 011 --- ... -- ... 00 @cl #Note: Must parse uimm > manually Why? It looks identical to @cl_w to me. > +static bool trans_c_addi4spn(DisasContext *ctx, arg_c_addi4spn *a, > + uint16_t insn) > +{ > + if (a->nzuimm == 0) { > + /* Reserved in ISA */ > + gen_exception_illegal(ctx); > + return true; This should definitely be return false, as that kind of instruction overlap is exactly what the return value is for. (There is no current support for overlapping insn patterns, but it is planned.) > +static bool trans_c_flw_ld(DisasContext *ctx, arg_c_flw_ld *a, uint16_t insn) > +{ > +#ifdef TARGET_RISCV32 > + /* C.FLW ( RV32FC-only ) */ > + arg_c_lw *tmp = g_new0(arg_c_lw, 1); > + extract_cl_w(tmp, insn); > + arg_flw arg = { .rd = tmp->rd, .rs1 = tmp->rs1, .imm = tmp->uimm }; > + return trans_flw(ctx, &arg, insn); > +#else > + /* C.LD ( RV64C/RV128C-only ) */ > + arg_c_fld *tmp = g_new0(arg_c_fld, 1); > + extract_cl_d(tmp, insn); > + arg_ld arg = { .rd = tmp->rd, .rs1 = tmp->rs1, .imm = tmp->uimm }; > + return trans_ld(ctx, &arg, insn); > +#endif Oh I see what you're after wrt manual parsing above. But why g_new0 and missing free. Why not stack allocation? > +static int64_t ex_rvc_register(int reg) return type should be int. r~