qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-arm] [PATCH 5/7] arm: Move condition-failed codep


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH 5/7] arm: Move condition-failed codepath generation out of if()
Date: Mon, 10 Apr 2017 10:22:25 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0

Hi Peter,

On 04/10/2017 07:39 AM, Peter Maydell wrote:
Move the code to generate the "condition failed" instruction
codepath out of the if (singlestepping) {} else {}. This
will allow adding support for handling a new is_jmp type
which can't be neatly split into "singlestepping case"
versus "not singlestepping case".

This makes also the code more readable :)


Signed-off-by: Peter Maydell <address@hidden>
---
 target/arm/translate.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index a1a0e73..87fd702 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -11988,9 +11988,9 @@ void gen_intermediate_code(CPUARMState *env, 
TranslationBlock *tb)
     /* At this stage dc->condjmp will only be set when the skipped
        instruction was a conditional branch or trap, and the PC has
        already been written.  */
+    gen_set_condexec(dc);
     if (unlikely(cs->singlestep_enabled || dc->ss_active)) {
         /* Unconditional and "condition passed" instruction codepath. */
-        gen_set_condexec(dc);
         switch (dc->is_jmp) {
         case DISAS_SWI:
             gen_ss_advance(dc);
@@ -12013,13 +12013,6 @@ void gen_intermediate_code(CPUARMState *env, 
TranslationBlock *tb)
             /* FIXME: Single stepping a WFI insn will not halt the CPU. */
             gen_singlestep_exception(dc);
         }
-        if (dc->condjmp) {
-            /* "Condition failed" instruction codepath. */
-            gen_set_label(dc->condlabel);
-            gen_set_condexec(dc);
-            gen_set_pc_im(dc, dc->pc);
-            gen_singlestep_exception(dc);
-        }
     } else {
         /* While branches must always occur at the end of an IT block,
            there are a few other things that can cause us to terminate
@@ -12029,7 +12022,6 @@ void gen_intermediate_code(CPUARMState *env, 
TranslationBlock *tb)
             - Hardware watchpoints.
            Hardware breakpoints have already been handled and skip this code.
          */
-        gen_set_condexec(dc);
         switch(dc->is_jmp) {
         case DISAS_NEXT:
             gen_goto_tb(dc, 1, dc->pc);
@@ -12069,11 +12061,17 @@ void gen_intermediate_code(CPUARMState *env, 
TranslationBlock *tb)
             gen_exception(EXCP_SMC, syn_aa32_smc(), 3);
             break;
         }
-        if (dc->condjmp) {
-            gen_set_label(dc->condlabel);
-            gen_set_condexec(dc);
+    }
+
+    if (dc->condjmp) {
+        /* "Condition failed" instruction codepath for the branch/trap insn */
+        gen_set_label(dc->condlabel);
+        gen_set_condexec(dc);
+        if (unlikely(cs->singlestep_enabled || dc->ss_active)) {

I'm custom to think "let's change that and think how to protect the code to help the next one who will modify it" and wonder if it isn't safer to define:

const bool singlestepping;

singlestepping = cs->singlestep_enabled || dc->ss_active;

Then use:

if unlikely(singlestepping)

At line 11993 and here, what do you think?

Either way:

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

+            gen_set_pc_im(dc, dc->pc);
+            gen_singlestep_exception(dc);
+        } else {
             gen_goto_tb(dc, 1, dc->pc);
-            dc->condjmp = 0;
         }
     }





reply via email to

[Prev in Thread] Current Thread [Next in Thread]