Message ID | 1402499992-64851-22-git-send-email-leon.alrae@imgtec.com |
---|---|
State | New |
Headers | show |
On Wed, Jun 11, 2014 at 04:19:51PM +0100, Leon Alrae wrote: > After selecting CPU in QEMU the base ISA will not change. Therefore > introducing *_arch function pointers that are set in cpu_state_reset to > point at the appropriate SPECIAL and SPECIAL3 decoding functions, and avoid > unnecessary 'if' statements. > > Signed-off-by: Leon Alrae <leon.alrae@imgtec.com> > --- > target-mips/translate.c | 32 +++++++++++++++++++++----------- > 1 files changed, 21 insertions(+), 11 deletions(-) > > diff --git a/target-mips/translate.c b/target-mips/translate.c > index de35b77..7ff7829 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -15634,6 +15634,13 @@ out: > tcg_temp_free(t1); > } > > +/* Some instructions from MIPS32R6 and pre-MIPS32R6 have identical encoding. > + > + decode_opc_*_arch are pointing at the appropriate decoding functions > + depending on a base ISA supported by selected MIPS CPU. */ > +static void (*decode_opc_special_arch) (CPUMIPSState*, DisasContext*); > +static void (*decode_opc_special3_arch) (CPUMIPSState*, DisasContext*); > + > static void decode_opc_special_r6(CPUMIPSState *env, DisasContext *ctx) > { > int rs, rt, rd, sa; > @@ -16002,11 +16009,8 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx) > break; > #endif > default: > - if (ctx->insn_flags & ISA_MIPS32R6) { > - decode_opc_special_r6(env, ctx); > - } else { > - decode_opc_special_legacy(env, ctx); > - } > + decode_opc_special_arch(env, ctx); > + break; > } > } > > @@ -16799,12 +16803,9 @@ static void decode_opc_special3(CPUMIPSState *env, DisasContext *ctx) > tcg_temp_free(t0); > } > break; > - default: /* Invalid */ > - if (ctx->insn_flags & ISA_MIPS32R6) { > - decode_opc_special3_r6(env, ctx); > - } else { > - decode_opc_special3_legacy(env, ctx); > - } > + default: > + decode_opc_special3_arch(env, ctx); > + break; > } > } > > @@ -17831,6 +17832,15 @@ void cpu_state_reset(CPUMIPSState *env) > env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0; > env->insn_flags = env->cpu_model->insn_flags; > > + /* Select decoding functions appropriate for supported ISA */ > + if (env->insn_flags & ISA_MIPS32R6) { > + decode_opc_special_arch = decode_opc_special_r6; > + decode_opc_special3_arch = decode_opc_special3_r6; > + } else { > + decode_opc_special_arch = decode_opc_special_legacy; > + decode_opc_special3_arch = decode_opc_special3_legacy; > + } > + > #if defined(CONFIG_USER_ONLY) > env->CP0_Status = (MIPS_HFLAG_UM << CP0St_KSU); > # ifdef TARGET_MIPS64 I have mixed filling about that. While it clearly makes the code more readable, I am not sure it would be any faster given that it might kill branch prediction. Any opinion from others?
On 06/19/2014 03:18 PM, Aurelien Jarno wrote: > I have mixed filling about that. While it clearly makes the code more > readable, I am not sure it would be any faster given that it might kill > branch prediction. Any opinion from others? Given the function pointer is used only once, I don't think it's useful. With normal branch predictors we'll do much better with direct branches, and probably inlining. r~
diff --git a/target-mips/translate.c b/target-mips/translate.c index de35b77..7ff7829 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -15634,6 +15634,13 @@ out: tcg_temp_free(t1); } +/* Some instructions from MIPS32R6 and pre-MIPS32R6 have identical encoding. + + decode_opc_*_arch are pointing at the appropriate decoding functions + depending on a base ISA supported by selected MIPS CPU. */ +static void (*decode_opc_special_arch) (CPUMIPSState*, DisasContext*); +static void (*decode_opc_special3_arch) (CPUMIPSState*, DisasContext*); + static void decode_opc_special_r6(CPUMIPSState *env, DisasContext *ctx) { int rs, rt, rd, sa; @@ -16002,11 +16009,8 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx) break; #endif default: - if (ctx->insn_flags & ISA_MIPS32R6) { - decode_opc_special_r6(env, ctx); - } else { - decode_opc_special_legacy(env, ctx); - } + decode_opc_special_arch(env, ctx); + break; } } @@ -16799,12 +16803,9 @@ static void decode_opc_special3(CPUMIPSState *env, DisasContext *ctx) tcg_temp_free(t0); } break; - default: /* Invalid */ - if (ctx->insn_flags & ISA_MIPS32R6) { - decode_opc_special3_r6(env, ctx); - } else { - decode_opc_special3_legacy(env, ctx); - } + default: + decode_opc_special3_arch(env, ctx); + break; } } @@ -17831,6 +17832,15 @@ void cpu_state_reset(CPUMIPSState *env) env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0; env->insn_flags = env->cpu_model->insn_flags; + /* Select decoding functions appropriate for supported ISA */ + if (env->insn_flags & ISA_MIPS32R6) { + decode_opc_special_arch = decode_opc_special_r6; + decode_opc_special3_arch = decode_opc_special3_r6; + } else { + decode_opc_special_arch = decode_opc_special_legacy; + decode_opc_special3_arch = decode_opc_special3_legacy; + } + #if defined(CONFIG_USER_ONLY) env->CP0_Status = (MIPS_HFLAG_UM << CP0St_KSU); # ifdef TARGET_MIPS64
After selecting CPU in QEMU the base ISA will not change. Therefore introducing *_arch function pointers that are set in cpu_state_reset to point at the appropriate SPECIAL and SPECIAL3 decoding functions, and avoid unnecessary 'if' statements. Signed-off-by: Leon Alrae <leon.alrae@imgtec.com> --- target-mips/translate.c | 32 +++++++++++++++++++++----------- 1 files changed, 21 insertions(+), 11 deletions(-)