Re: [Qemu-devel] [PULL 24/49] target-i386: Remove gen_op_movl_T0_im*

[Kevin Wolf <kwolf@redhat.com>]

Am 07.01.2014 um 22:00 hat Richard Henderson geschrieben:
> Propagate the definition of gen_op_movl_T0_im to all users.
> The function gen_op_movl_T0_imu was unused.
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target-i386/translate.c | 32 ++++++++++----------------------
>  1 file changed, 10 insertions(+), 22 deletions(-)
> 
> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index 0671371..46eabe4 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -252,16 +252,6 @@ static void gen_update_cc_op(DisasContext *s)
>      }
>  }
>  
> -static inline void gen_op_movl_T0_im(int32_t val)
> -{
> -    tcg_gen_movi_tl(cpu_T[0], val);
> -}
> -
> -static inline void gen_op_movl_T0_imu(uint32_t val)
> -{
> -    tcg_gen_movi_tl(cpu_T[0], val);
> -}
> -
>  static inline void gen_op_movl_T1_im(int32_t val)
>  {
>      tcg_gen_movi_tl(cpu_T[1], val);
> @@ -3500,13 +3490,13 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
>              }
>              val = cpu_ldub_code(env, s->pc++);
>              if (is_xmm) {
> -                gen_op_movl_T0_im(val);
> +                tcg_gen_movi_tl(cpu_T[0], val);
>                  tcg_gen_st32_tl(cpu_T[0], cpu_env, offsetof(CPUX86State,xmm_t0.XMM_L(0)));
>                  tcg_gen_movi_tl(cpu_T[0], 0);
>                  tcg_gen_st32_tl(cpu_T[0], cpu_env, offsetof(CPUX86State,xmm_t0.XMM_L(1)));
>                  op1_offset = offsetof(CPUX86State,xmm_t0);
>              } else {
> -                gen_op_movl_T0_im(val);
> +                tcg_gen_movi_tl(cpu_T[0], val);
>                  tcg_gen_st32_tl(cpu_T[0], cpu_env, offsetof(CPUX86State,mmx_t0.MMX_L(0)));
>                  tcg_gen_movi_tl(cpu_T[0], 0);
>                  tcg_gen_st32_tl(cpu_T[0], cpu_env, offsetof(CPUX86State,mmx_t0.MMX_L(1)));
> @@ -5380,7 +5370,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>              val = insn_get(env, s, ot);
>          else
>              val = (int8_t)insn_get(env, s, MO_8);
> -        gen_op_movl_T0_im(val);
> +        tcg_gen_movi_tl(cpu_T[0], val);
>          gen_push_T0(s);
>          break;
>      case 0x8f: /* pop Ev */
> @@ -5509,7 +5499,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>              gen_lea_modrm(env, s, modrm);
>          }
>          val = insn_get(env, s, ot);
> -        gen_op_movl_T0_im(val);
> +        tcg_gen_movi_tl(cpu_T[0], val);
>          if (mod != 3) {
>              gen_op_st_v(s, ot, cpu_T[0], cpu_A0);
>          } else {
> @@ -5685,7 +5675,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>          break;
>      case 0xb0 ... 0xb7: /* mov R, Ib */
>          val = insn_get(env, s, MO_8);
> -        gen_op_movl_T0_im(val);
> +        tcg_gen_movi_tl(cpu_T[0], val);
>          gen_op_mov_reg_T0(MO_8, (b & 7) | REX_B(s));
>          break;
>      case 0xb8 ... 0xbf: /* mov R, Iv */
> @@ -5704,7 +5694,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>              ot = dflag ? MO_32 : MO_16;
>              val = insn_get(env, s, ot);
>              reg = (b & 7) | REX_B(s);
> -            gen_op_movl_T0_im(val);
> +            tcg_gen_movi_tl(cpu_T[0], val);
>              gen_op_mov_reg_T0(ot, reg);
>          }
>          break;
> @@ -6508,12 +6498,11 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>          else
>              ot = dflag ? MO_32 : MO_16;
>          val = cpu_ldub_code(env, s->pc++);
> -        gen_op_movl_T0_im(val);
>          gen_check_io(s, ot, pc_start - s->cs_base,
>                       SVM_IOIO_TYPE_MASK | svm_is_rep(prefixes));

This removal is wrong. gen_check_io() uses cpu_T[0] internally in order
to check the I/O bitmap. Now I get a #GP even for legitimate cases.

The following patch (which is essentially a partial revert) makes it
work for me again. Should I send it as a proper patch or would you
prefer to clean up gen_check_io()? I guess some more explicit passing of
the port number couldn't hurt there, but I'm not familiar with TCG and
who knows what I would break while doing this...

Kevin


diff --git a/target-i386/translate.c b/target-i386/translate.c
index b0f2279..5dd2450 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -6284,6 +6284,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
     case 0xe5:
         ot = mo_b_d32(b, dflag);
         val = cpu_ldub_code(env, s->pc++);
+        tcg_gen_movi_tl(cpu_T[0], val);
         gen_check_io(s, ot, pc_start - s->cs_base,
                      SVM_IOIO_TYPE_MASK | svm_is_rep(prefixes));
         if (use_icount)
@@ -6300,6 +6301,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
     case 0xe7:
         ot = mo_b_d32(b, dflag);
         val = cpu_ldub_code(env, s->pc++);
+        tcg_gen_movi_tl(cpu_T[0], val);
         gen_check_io(s, ot, pc_start - s->cs_base,
                      svm_is_rep(prefixes));
         gen_op_mov_v_reg(ot, cpu_T[1], R_EAX);