Message ID | 1341523740-22711-4-git-send-email-peter.maydell@linaro.org |
---|---|
State | Accepted |
Commit | bedc2ac1a746e61e2a42c98603922c488b82cddb |
Headers | show |
Please see comments below. Am 05.07.2012 23:29, schrieb Peter Maydell: > Rephrase some of the expressions used to select an entry > in the SSE op table arrays so that it's clearer that they > don't overrun the op table array size. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target-i386/translate.c | 12 ++++++------ > 1 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/target-i386/translate.c b/target-i386/translate.c > index 5899e09..1988dae 100644 > --- a/target-i386/translate.c > +++ b/target-i386/translate.c > @@ -2964,16 +2964,16 @@ static const SSEFunc_0_pl sse_op_table3aq[] = { > > static const SSEFunc_i_p sse_op_table3bi[] = { > gen_helper_cvttss2si, > - gen_helper_cvttsd2si, > gen_helper_cvtss2si, > + gen_helper_cvttsd2si, > gen_helper_cvtsd2si > }; > > #ifdef TARGET_X86_64 > static const SSEFunc_l_p sse_op_table3bq[] = { > gen_helper_cvttss2sq, > - gen_helper_cvttsd2sq, > gen_helper_cvtss2sq, > + gen_helper_cvttsd2sq, > gen_helper_cvtsd2sq > }; > #endif > @@ -3571,12 +3571,12 @@ static void gen_sse(DisasContext *s, int b, target_ulong pc_start, int rex_r) > op1_offset = offsetof(CPUX86State,xmm_regs[reg]); > tcg_gen_addi_ptr(cpu_ptr0, cpu_env, op1_offset); > if (ot == OT_LONG) { > - SSEFunc_0_pi sse_fn_pi = sse_op_table3ai[(b >> 8) - 2]; > + SSEFunc_0_pi sse_fn_pi = sse_op_table3ai[(b >> 8) & 1]; > tcg_gen_trunc_tl_i32(cpu_tmp2_i32, cpu_T[0]); > sse_fn_pi(cpu_ptr0, cpu_tmp2_i32); > } else { > #ifdef TARGET_X86_64 > - SSEFunc_0_pl sse_fn_pl = sse_op_table3aq[(b >> 8) - 2]; > + SSEFunc_0_pl sse_fn_pl = sse_op_table3aq[(b >> 8) & 1]; > sse_fn_pl(cpu_ptr0, cpu_T[0]); > #else > goto illegal_op; > @@ -3635,13 +3635,13 @@ static void gen_sse(DisasContext *s, int b, target_ulong pc_start, int rex_r) Here I suggest to reorder the case statements: case 0x22c: /* cvttss2si */ case 0x22d: /* cvtss2si */ case 0x32c: /* cvttsd2si */ case 0x32d: /* cvtsd2si */ That's the numeric order, and it matches the order in the modified array again. > > tcg_gen_addi_ptr(cpu_ptr0, cpu_env, op2_offset); > if (ot == OT_LONG) { > SSEFunc_i_p sse_fn_i_p = > - sse_op_table3bi[(b >> 8) - 2 + (b & 1) * 2]; > + sse_op_table3bi[((b >> 7) & 2) | (b & 1)]; > sse_fn_i_p(cpu_tmp2_i32, cpu_ptr0); > tcg_gen_extu_i32_tl(cpu_T[0], cpu_tmp2_i32); > } else { > #ifdef TARGET_X86_64 > SSEFunc_l_p sse_fn_l_p = > - sse_op_table3bq[(b >> 8) - 2 + (b & 1) * 2]; > + sse_op_table3bq[((b >> 7) & 2) | (b & 1)]; > sse_fn_l_p(cpu_T[0], cpu_ptr0); > #else > goto illegal_op; Maybe a 2-dimensional array would make that code even more readable: sse_op_table3bq[(b >> 8) & 1][b & 1]; Anyway, you may add a Reviewed-by: Stefan Weil <sw@weilnetz.de> Regards, Stefan W.
diff --git a/target-i386/translate.c b/target-i386/translate.c index 5899e09..1988dae 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -2964,16 +2964,16 @@ static const SSEFunc_0_pl sse_op_table3aq[] = { static const SSEFunc_i_p sse_op_table3bi[] = { gen_helper_cvttss2si, - gen_helper_cvttsd2si, gen_helper_cvtss2si, + gen_helper_cvttsd2si, gen_helper_cvtsd2si }; #ifdef TARGET_X86_64 static const SSEFunc_l_p sse_op_table3bq[] = { gen_helper_cvttss2sq, - gen_helper_cvttsd2sq, gen_helper_cvtss2sq, + gen_helper_cvttsd2sq, gen_helper_cvtsd2sq }; #endif @@ -3571,12 +3571,12 @@ static void gen_sse(DisasContext *s, int b, target_ulong pc_start, int rex_r) op1_offset = offsetof(CPUX86State,xmm_regs[reg]); tcg_gen_addi_ptr(cpu_ptr0, cpu_env, op1_offset); if (ot == OT_LONG) { - SSEFunc_0_pi sse_fn_pi = sse_op_table3ai[(b >> 8) - 2]; + SSEFunc_0_pi sse_fn_pi = sse_op_table3ai[(b >> 8) & 1]; tcg_gen_trunc_tl_i32(cpu_tmp2_i32, cpu_T[0]); sse_fn_pi(cpu_ptr0, cpu_tmp2_i32); } else { #ifdef TARGET_X86_64 - SSEFunc_0_pl sse_fn_pl = sse_op_table3aq[(b >> 8) - 2]; + SSEFunc_0_pl sse_fn_pl = sse_op_table3aq[(b >> 8) & 1]; sse_fn_pl(cpu_ptr0, cpu_T[0]); #else goto illegal_op; @@ -3635,13 +3635,13 @@ static void gen_sse(DisasContext *s, int b, target_ulong pc_start, int rex_r) tcg_gen_addi_ptr(cpu_ptr0, cpu_env, op2_offset); if (ot == OT_LONG) { SSEFunc_i_p sse_fn_i_p = - sse_op_table3bi[(b >> 8) - 2 + (b & 1) * 2]; + sse_op_table3bi[((b >> 7) & 2) | (b & 1)]; sse_fn_i_p(cpu_tmp2_i32, cpu_ptr0); tcg_gen_extu_i32_tl(cpu_T[0], cpu_tmp2_i32); } else { #ifdef TARGET_X86_64 SSEFunc_l_p sse_fn_l_p = - sse_op_table3bq[(b >> 8) - 2 + (b & 1) * 2]; + sse_op_table3bq[((b >> 7) & 2) | (b & 1)]; sse_fn_l_p(cpu_T[0], cpu_ptr0); #else goto illegal_op;
Rephrase some of the expressions used to select an entry in the SSE op table arrays so that it's clearer that they don't overrun the op table array size. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target-i386/translate.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)