diff mbox series

[v2,09/11] target/cris: Move delayed branch handling to tb_stop

Message ID 20210620213249.1494274-10-richard.henderson@linaro.org
State New
Headers show
Series target/cris: Convert to TranslatorOps | expand

Commit Message

Richard Henderson June 20, 2021, 9:32 p.m. UTC 
By moving the code here, we can re-use other end-of-tb code,
e.g. the evaluation of flags.  Honor single stepping.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/cris/translate.c | 82 ++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 37 deletions(-)

-- 
2.25.1

Comments

Edgar E. Iglesias June 21, 2021, 5:15 p.m. UTC | #1 
On Sun, Jun 20, 2021 at 02:32:47PM -0700, Richard Henderson wrote:
> By moving the code here, we can re-use other end-of-tb code,

> e.g. the evaluation of flags.  Honor single stepping.


Hi Richard,

Unfortunately this patch seems to break one of the CRIS tests.
tests/tcg/cris/bare/check_xarith.s

I have an old image with the test-case built in the rootfs...

Best regards,
Edgar


> 

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/cris/translate.c | 82 ++++++++++++++++++++++-------------------

>  1 file changed, 45 insertions(+), 37 deletions(-)

> 

> diff --git a/target/cris/translate.c b/target/cris/translate.c

> index 83b20162f1..0e925320b3 100644

> --- a/target/cris/translate.c

> +++ b/target/cris/translate.c

> @@ -55,6 +55,7 @@

>  /* is_jmp field values */

>  #define DISAS_JUMP    DISAS_TARGET_0 /* only pc was modified dynamically */

>  #define DISAS_UPDATE  DISAS_TARGET_1 /* cpu state was modified dynamically */

> +#define DISAS_DBRANCH DISAS_TARGET_2 /* pc update for delayed branch */

>  

>  /* Used by the decoder.  */

>  #define EXTRACT_FIELD(src, start, end) \

> @@ -3219,43 +3220,8 @@ static void cris_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)

>       * loop doing nothing for on this program location.

>       */

>      if (dc->delayed_branch && --dc->delayed_branch == 0) {

> -        if (dc->base.tb->flags & 7) {

> -            t_gen_movi_env_TN(dslot, 0);

> -        }

> -

> -        if (dc->cpustate_changed

> -            || !dc->flagx_known

> -            || (dc->flags_x != (dc->base.tb->flags & X_FLAG))) {

> -            cris_store_direct_jmp(dc);

> -        }

> -

> -        if (dc->clear_locked_irq) {

> -            dc->clear_locked_irq = 0;

> -            t_gen_movi_env_TN(locked_irq, 0);

> -        }

> -

> -        if (dc->jmp == JMP_DIRECT_CC) {

> -            TCGLabel *l1 = gen_new_label();

> -            cris_evaluate_flags(dc);

> -

> -            /* Conditional jmp.  */

> -            tcg_gen_brcondi_tl(TCG_COND_EQ, env_btaken, 0, l1);

> -            gen_goto_tb(dc, 1, dc->jmp_pc);

> -            gen_set_label(l1);

> -            gen_goto_tb(dc, 0, dc->pc);

> -            dc->base.is_jmp = DISAS_NORETURN;

> -            dc->jmp = JMP_NOJMP;

> -        } else if (dc->jmp == JMP_DIRECT) {

> -            cris_evaluate_flags(dc);

> -            gen_goto_tb(dc, 0, dc->jmp_pc);

> -            dc->base.is_jmp = DISAS_NORETURN;

> -            dc->jmp = JMP_NOJMP;

> -        } else {

> -            TCGv c = tcg_const_tl(dc->pc);

> -            t_gen_cc_jmp(env_btarget, c);

> -            tcg_temp_free(c);

> -            dc->base.is_jmp = DISAS_JUMP;

> -        }

> +        dc->base.is_jmp = DISAS_DBRANCH;

> +        return;

>      }

>  

>      /* Force an update if the per-tb cpu state has changed.  */

> @@ -3303,6 +3269,48 @@ static void cris_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)

>  

>      cris_evaluate_flags(dc);

>  

> +    /* Evaluate delayed branch destination and fold to another is_jmp case. */

> +    if (is_jmp == DISAS_DBRANCH) {

> +        if (dc->base.tb->flags & 7) {

> +            t_gen_movi_env_TN(dslot, 0);

> +        }

> +

> +        switch (dc->jmp) {

> +        case JMP_DIRECT:

> +            npc = dc->jmp_pc;

> +            is_jmp = DISAS_TOO_MANY;

> +            break;

> +

> +        case JMP_DIRECT_CC:

> +            /*

> +             * Use a conditional branch if either taken or not-taken path

> +             * can use goto_tb.  If neither can, then treat it as indirect.

> +             */

> +            if (likely(!dc->base.singlestep_enabled)

> +                && (use_goto_tb(dc, dc->jmp_pc) || use_goto_tb(dc, npc))) {

> +                TCGLabel *not_taken = gen_new_label();

> +

> +                tcg_gen_brcondi_tl(TCG_COND_EQ, env_btaken, 0, not_taken);

> +                gen_goto_tb(dc, 1, dc->jmp_pc);

> +                gen_set_label(not_taken);

> +

> +                /* not-taken case handled below. */

> +                is_jmp = DISAS_TOO_MANY;

> +                break;

> +            }

> +            tcg_gen_movi_tl(env_btarget, dc->jmp_pc);

> +            /* fall through */

> +

> +        case JMP_INDIRECT:

> +            t_gen_cc_jmp(env_btarget, tcg_constant_tl(npc));

> +            is_jmp = DISAS_JUMP;

> +            break;

> +

> +        default:

> +            g_assert_not_reached();

> +        }

> +    }

> +

>      if (unlikely(dc->base.singlestep_enabled)) {

>          switch (is_jmp) {

>          case DISAS_TOO_MANY:

> -- 

> 2.25.1

>
Richard Henderson June 21, 2021, 5:35 p.m. UTC | #2 
On 6/21/21 10:15 AM, Edgar E. Iglesias wrote:
> On Sun, Jun 20, 2021 at 02:32:47PM -0700, Richard Henderson wrote:

>> By moving the code here, we can re-use other end-of-tb code,

>> e.g. the evaluation of flags.  Honor single stepping.

> 

> Hi Richard,

> 

> Unfortunately this patch seems to break one of the CRIS tests.

> tests/tcg/cris/bare/check_xarith.s


Hum, that's currently marked

# no sure why
CRIS_BROKEN_TESTS += check_scc check_xarith

> I have an old image with the test-case built in the rootfs...


Can you share that?

r~
diff mbox series

Patch

diff --git a/target/cris/translate.c b/target/cris/translate.c
index 83b20162f1..0e925320b3 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -55,6 +55,7 @@ 
 /* is_jmp field values */
 #define DISAS_JUMP    DISAS_TARGET_0 /* only pc was modified dynamically */
 #define DISAS_UPDATE  DISAS_TARGET_1 /* cpu state was modified dynamically */
+#define DISAS_DBRANCH DISAS_TARGET_2 /* pc update for delayed branch */
 
 /* Used by the decoder.  */
 #define EXTRACT_FIELD(src, start, end) \
@@ -3219,43 +3220,8 @@  static void cris_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
      * loop doing nothing for on this program location.
      */
     if (dc->delayed_branch && --dc->delayed_branch == 0) {
-        if (dc->base.tb->flags & 7) {
-            t_gen_movi_env_TN(dslot, 0);
-        }
-
-        if (dc->cpustate_changed
-            || !dc->flagx_known
-            || (dc->flags_x != (dc->base.tb->flags & X_FLAG))) {
-            cris_store_direct_jmp(dc);
-        }
-
-        if (dc->clear_locked_irq) {
-            dc->clear_locked_irq = 0;
-            t_gen_movi_env_TN(locked_irq, 0);
-        }
-
-        if (dc->jmp == JMP_DIRECT_CC) {
-            TCGLabel *l1 = gen_new_label();
-            cris_evaluate_flags(dc);
-
-            /* Conditional jmp.  */
-            tcg_gen_brcondi_tl(TCG_COND_EQ, env_btaken, 0, l1);
-            gen_goto_tb(dc, 1, dc->jmp_pc);
-            gen_set_label(l1);
-            gen_goto_tb(dc, 0, dc->pc);
-            dc->base.is_jmp = DISAS_NORETURN;
-            dc->jmp = JMP_NOJMP;
-        } else if (dc->jmp == JMP_DIRECT) {
-            cris_evaluate_flags(dc);
-            gen_goto_tb(dc, 0, dc->jmp_pc);
-            dc->base.is_jmp = DISAS_NORETURN;
-            dc->jmp = JMP_NOJMP;
-        } else {
-            TCGv c = tcg_const_tl(dc->pc);
-            t_gen_cc_jmp(env_btarget, c);
-            tcg_temp_free(c);
-            dc->base.is_jmp = DISAS_JUMP;
-        }
+        dc->base.is_jmp = DISAS_DBRANCH;
+        return;
     }
 
     /* Force an update if the per-tb cpu state has changed.  */
@@ -3303,6 +3269,48 @@  static void cris_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
 
     cris_evaluate_flags(dc);
 
+    /* Evaluate delayed branch destination and fold to another is_jmp case. */
+    if (is_jmp == DISAS_DBRANCH) {
+        if (dc->base.tb->flags & 7) {
+            t_gen_movi_env_TN(dslot, 0);
+        }
+
+        switch (dc->jmp) {
+        case JMP_DIRECT:
+            npc = dc->jmp_pc;
+            is_jmp = DISAS_TOO_MANY;
+            break;
+
+        case JMP_DIRECT_CC:
+            /*
+             * Use a conditional branch if either taken or not-taken path
+             * can use goto_tb.  If neither can, then treat it as indirect.
+             */
+            if (likely(!dc->base.singlestep_enabled)
+                && (use_goto_tb(dc, dc->jmp_pc) || use_goto_tb(dc, npc))) {
+                TCGLabel *not_taken = gen_new_label();
+
+                tcg_gen_brcondi_tl(TCG_COND_EQ, env_btaken, 0, not_taken);
+                gen_goto_tb(dc, 1, dc->jmp_pc);
+                gen_set_label(not_taken);
+
+                /* not-taken case handled below. */
+                is_jmp = DISAS_TOO_MANY;
+                break;
+            }
+            tcg_gen_movi_tl(env_btarget, dc->jmp_pc);
+            /* fall through */
+
+        case JMP_INDIRECT:
+            t_gen_cc_jmp(env_btarget, tcg_constant_tl(npc));
+            is_jmp = DISAS_JUMP;
+            break;
+
+        default:
+            g_assert_not_reached();
+        }
+    }
+
     if (unlikely(dc->base.singlestep_enabled)) {
         switch (is_jmp) {
         case DISAS_TOO_MANY: