On 07/07/2014 11:13 AM, Bastian Koppelmann wrote: > diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c > index b9fbfad..5267fd0 100644 > --- a/target-tricore/op_helper.c > +++ b/target-tricore/op_helper.c > @@ -56,6 +56,198 @@ target_ulong helper_shac(CPUTRICOREState *env, > target_ulong r1, > return ret; > } > > +/* context save area (CSA) related helpers */ > + > +enum { > + CONTEXT_LOWER = 0, > + CONTEXT_UPPER = 1, > +}; > + > +static int cdc_increment(TCState *tc) > +{ > + tc->PSW++; > + return 0; > +} > + > +static int cdc_decrement(TCState *tc) > +{ > + tc->PSW--; > + return 0; > +}
I guess these will be filled in more later? > +static void save_context(CPUTRICOREState *env, int ea, int ul, > + target_ulong *new_FCX) > +{ > + *new_FCX = cpu_ldl_data(env, ea); > + cpu_stl_data(env, ea, env->active_tc.PCXI); > + if (ul == CONTEXT_UPPER) { > + cpu_stl_data(env, ea+4, env->active_tc.PSW); > + cpu_stl_data(env, ea+8, env->active_tc.gpr_a[10]); > + cpu_stl_data(env, ea+12, env->active_tc.gpr_a[11]); > + cpu_stl_data(env, ea+16, env->active_tc.gpr_d[8]); > + cpu_stl_data(env, ea+20, env->active_tc.gpr_d[9]); > + cpu_stl_data(env, ea+24, env->active_tc.gpr_d[10]); > + cpu_stl_data(env, ea+28, env->active_tc.gpr_d[11]); > + cpu_stl_data(env, ea+32, env->active_tc.gpr_a[12]); > + cpu_stl_data(env, ea+36, env->active_tc.gpr_a[13]); > + cpu_stl_data(env, ea+40, env->active_tc.gpr_a[14]); > + cpu_stl_data(env, ea+44, env->active_tc.gpr_a[15]); > + cpu_stl_data(env, ea+48, env->active_tc.gpr_d[12]); > + cpu_stl_data(env, ea+52, env->active_tc.gpr_d[13]); > + cpu_stl_data(env, ea+56, env->active_tc.gpr_d[14]); > + cpu_stl_data(env, ea+60, env->active_tc.gpr_d[15]); I wonder if this ought to be split into separate functions. Certainly you know for a call that it's going to be the upper context, so why test? > +static inline void gen_branch_cond(DisasContext *ctx, int cond, TCGv r1, > + TCGv r2, int16_t address) > +{ > + int jumpLabel; > + jumpLabel = gen_new_label(); > + tcg_gen_brcond_tl(cond, r1, r2, jumpLabel); > + > + gen_save_pc(ctx->pc + insn_bytes); insn_bytes should be part of ctx. Alternately, pre-compute ctx->next_pc = ctx->pc + insn_bytes, since that appears to be more useful. > + tcg_gen_exit_tb(0); > + > + gen_set_label(jumpLabel); > + gen_save_pc(ctx->pc + address * 2); > + tcg_gen_exit_tb(0); > + > +} Watch the useless blank lines. You'll want to emit goto_tb opcodes when appropriate. See examples in other targets for when this is possible. > + case OPC1_16_SB_CALL: > + gen_helper_2arg(call, ctx->pc, insn_bytes); > + gen_save_pc(ctx->pc+sign_extend(offset, 7)*2); > + tcg_gen_exit_tb(0); Why pass pc and insn_bytes to helper_call, when all that happens inside is that they're added together? Or for that matter why pass either at all since you could just as well emit tcg_gen_movi_tl(cpu_gen_a[11], ctx->pc + insn_bytes); And again you'll want to emit goto_tb if possible. > +/* SB-format */ > + case OPC1_16_SB_CALL: > + address = MASK_OP_SB_DISP8(ctx->opcode); > + gen_compute_branch(ctx, op1, 0, 0, 0, address); > + break; > + case OPC1_16_SB_J: > + address = MASK_OP_SB_DISP8(ctx->opcode); > + gen_compute_branch(ctx, op1, 0, 0, 0, address); > + break; > + case OPC1_16_SB_JNZ: > + address = sign_extend(MASK_OP_SB_DISP8(ctx->opcode), 7); > + gen_compute_branch(ctx, op1, 0, 0, 0, address); > + break; > + case OPC1_16_SB_JZ: > + address = sign_extend(MASK_OP_SB_DISP8(ctx->opcode), 7); > + gen_compute_branch(ctx, op1, 0, 0, 0, address); Surely all these should be one common call to gen_compute_branch. r~