[v14,03/34] target/i386: Use generic DISAS_* enumerators
diff mbox

Message ID 20170715094243.28371-4-rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson July 15, 2017, 9:42 a.m. UTC
This target is not sophisticated in its use of cleanups at the
end of the translation loop.  For the most part, any condition
that exits the TB is dealt with by emitting the exiting opcode
right then and there.  Therefore the only is_jmp indicator that
is needed is DISAS_NORETURN.

For two stack segment modifying cases, we have not yet exited
the TB (therefore DISAS_NORETURN feels wrong), but intend to exit.
The caller of gen_movl_seg_T0 currently checks for any non-zero
value, therefore DISAS_TOO_MANY seems acceptable for that usage.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/i386/translate.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

Comments

Emilio G. Cota July 21, 2017, 9:25 p.m. UTC | #1
On Fri, Jul 14, 2017 at 23:42:12 -1000, Richard Henderson wrote:
> This target is not sophisticated in its use of cleanups at the
> end of the translation loop.  For the most part, any condition
> that exits the TB is dealt with by emitting the exiting opcode
> right then and there.  Therefore the only is_jmp indicator that
> is needed is DISAS_NORETURN.
> 
> For two stack segment modifying cases, we have not yet exited
> the TB (therefore DISAS_NORETURN feels wrong), but intend to exit.
> The caller of gen_movl_seg_T0 currently checks for any non-zero
> value, therefore DISAS_TOO_MANY seems acceptable for that usage.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>

Reviewed-by: Emilio G. Cota <cota@braap.org>

		E.
Lluís Vilanova July 21, 2017, 10:35 p.m. UTC | #2
Richard Henderson writes:

> This target is not sophisticated in its use of cleanups at the
> end of the translation loop.  For the most part, any condition
> that exits the TB is dealt with by emitting the exiting opcode
> right then and there.  Therefore the only is_jmp indicator that
> is needed is DISAS_NORETURN.

> For two stack segment modifying cases, we have not yet exited
> the TB (therefore DISAS_NORETURN feels wrong), but intend to exit.
> The caller of gen_movl_seg_T0 currently checks for any non-zero
> value, therefore DISAS_TOO_MANY seems acceptable for that usage.

> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/i386/translate.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)

> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index cab9e32..3ffbf1b 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -31,6 +31,7 @@
>  #include "trace-tcg.h"
>  #include "exec/log.h"
 
> +#define DISAS_TOO_MANY 5

Why is this one not added as a generic define too (like DISAS_NORETURN in prev
patch)?


Thanks,
  Lluis
Richard Henderson July 22, 2017, 10:31 a.m. UTC | #3
On 07/21/2017 12:35 PM, Lluís Vilanova wrote:
>> +#define DISAS_TOO_MANY 5
> 
> Why is this one not added as a generic define too (like DISAS_NORETURN in prev
> patch)?

Because I use NORETURN in the arm cleanup, prior to conversion to the enum, 
whereas I do not use TOO_MANY outside the i386 port.  I suppose I could have, 
but at this stage it doesn't really matter, as it only lives 4 more patches til 
the generic enum comes in.


r~

Patch
diff mbox

diff --git a/target/i386/translate.c b/target/i386/translate.c
index cab9e32..3ffbf1b 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -31,6 +31,7 @@ 
 #include "trace-tcg.h"
 #include "exec/log.h"
 
+#define DISAS_TOO_MANY 5
 
 #define PREFIX_REPZ   0x01
 #define PREFIX_REPNZ  0x02
@@ -2153,6 +2154,7 @@  static inline void gen_goto_tb(DisasContext *s, int tb_num, target_ulong eip)
         tcg_gen_goto_tb(tb_num);
         gen_jmp_im(eip);
         tcg_gen_exit_tb((uintptr_t)s->tb + tb_num);
+        s->is_jmp = DISAS_NORETURN;
     } else {
         /* jump to another page */
         gen_jmp_im(eip);
@@ -2173,7 +2175,6 @@  static inline void gen_jcc(DisasContext *s, int b,
 
         gen_set_label(l1);
         gen_goto_tb(s, 1, val);
-        s->is_jmp = DISAS_TB_JUMP;
     } else {
         l1 = gen_new_label();
         l2 = gen_new_label();
@@ -2243,12 +2244,14 @@  static void gen_movl_seg_T0(DisasContext *s, int seg_reg)
            because ss32 may change. For R_SS, translation must always
            stop as a special handling must be done to disable hardware
            interrupts for the next instruction */
-        if (seg_reg == R_SS || (s->code32 && seg_reg < R_FS))
-            s->is_jmp = DISAS_TB_JUMP;
+        if (seg_reg == R_SS || (s->code32 && seg_reg < R_FS)) {
+            s->is_jmp = DISAS_TOO_MANY;
+        }
     } else {
         gen_op_movl_seg_T0_vm(seg_reg);
-        if (seg_reg == R_SS)
-            s->is_jmp = DISAS_TB_JUMP;
+        if (seg_reg == R_SS) {
+            s->is_jmp = DISAS_TOO_MANY;
+        }
     }
 }
 
@@ -2420,7 +2423,7 @@  static void gen_exception(DisasContext *s, int trapno, target_ulong cur_eip)
     gen_update_cc_op(s);
     gen_jmp_im(cur_eip);
     gen_helper_raise_exception(cpu_env, tcg_const_i32(trapno));
-    s->is_jmp = DISAS_TB_JUMP;
+    s->is_jmp = DISAS_NORETURN;
 }
 
 /* Generate #UD for the current instruction.  The assumption here is that
@@ -2458,7 +2461,7 @@  static void gen_interrupt(DisasContext *s, int intno,
     gen_jmp_im(cur_eip);
     gen_helper_raise_interrupt(cpu_env, tcg_const_i32(intno),
                                tcg_const_i32(next_eip - cur_eip));
-    s->is_jmp = DISAS_TB_JUMP;
+    s->is_jmp = DISAS_NORETURN;
 }
 
 static void gen_debug(DisasContext *s, target_ulong cur_eip)
@@ -2466,7 +2469,7 @@  static void gen_debug(DisasContext *s, target_ulong cur_eip)
     gen_update_cc_op(s);
     gen_jmp_im(cur_eip);
     gen_helper_debug(cpu_env);
-    s->is_jmp = DISAS_TB_JUMP;
+    s->is_jmp = DISAS_NORETURN;
 }
 
 static void gen_set_hflag(DisasContext *s, uint32_t mask)
@@ -2541,7 +2544,7 @@  do_gen_eob_worker(DisasContext *s, bool inhibit, bool recheck_tf, TCGv jr)
     } else {
         tcg_gen_exit_tb(0);
     }
-    s->is_jmp = DISAS_TB_JUMP;
+    s->is_jmp = DISAS_NORETURN;
 }
 
 static inline void
@@ -2580,7 +2583,6 @@  static void gen_jmp_tb(DisasContext *s, target_ulong eip, int tb_num)
     set_cc_op(s, CC_OP_DYNAMIC);
     if (s->jmp_opt) {
         gen_goto_tb(s, tb_num, eip);
-        s->is_jmp = DISAS_TB_JUMP;
     } else {
         gen_jmp_im(eip);
         gen_eob(s);
@@ -6942,7 +6944,7 @@  static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             gen_update_cc_op(s);
             gen_jmp_im(pc_start - s->cs_base);
             gen_helper_pause(cpu_env, tcg_const_i32(s->pc - pc_start));
-            s->is_jmp = DISAS_TB_JUMP;
+            s->is_jmp = DISAS_NORETURN;
         }
         break;
     case 0x9b: /* fwait */
@@ -7187,7 +7189,7 @@  static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             gen_update_cc_op(s);
             gen_jmp_im(pc_start - s->cs_base);
             gen_helper_hlt(cpu_env, tcg_const_i32(s->pc - pc_start));
-            s->is_jmp = DISAS_TB_JUMP;
+            s->is_jmp = DISAS_NORETURN;
         }
         break;
     case 0x100:
@@ -7370,7 +7372,7 @@  static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             gen_helper_vmrun(cpu_env, tcg_const_i32(s->aflag - 1),
                              tcg_const_i32(s->pc - pc_start));
             tcg_gen_exit_tb(0);
-            s->is_jmp = DISAS_TB_JUMP;
+            s->is_jmp = DISAS_NORETURN;
             break;
 
         case 0xd9: /* VMMCALL */