| Message ID | 1493407045-24172-2-git-send-email-cota@braap.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Apr 28, 2017 at 15:17:24 -0400, Emilio G. Cota wrote: > Perf numbers in next commit's log. > > Signed-off-by: Emilio G. Cota <cota@braap.org> > --- > target/arm/translate-a64.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c > index 24de30d..5b691fc 100644 > --- a/target/arm/translate-a64.c > +++ b/target/arm/translate-a64.c > @@ -373,8 +373,7 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest) > } else if (s->singlestep_enabled) { > gen_exception_internal(EXCP_DEBUG); > } else { > - tcg_gen_exit_tb(0); > - s->is_jmp = DISAS_TB_JUMP; I'm not sure about removing this line though. Would it be better to leave it? I can't see how TB_JUMP ends up doing anything in the rest of the file. Thanks, E.
On 04/28/2017 09:22 PM, Emilio G. Cota wrote: > On Fri, Apr 28, 2017 at 15:17:24 -0400, Emilio G. Cota wrote: >> Perf numbers in next commit's log. >> >> Signed-off-by: Emilio G. Cota <cota@braap.org> >> --- >> target/arm/translate-a64.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c >> index 24de30d..5b691fc 100644 >> --- a/target/arm/translate-a64.c >> +++ b/target/arm/translate-a64.c >> @@ -373,8 +373,7 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest) >> } else if (s->singlestep_enabled) { >> gen_exception_internal(EXCP_DEBUG); >> } else { >> - tcg_gen_exit_tb(0); >> - s->is_jmp = DISAS_TB_JUMP; > > I'm not sure about removing this line though. Would it be better to leave it? > I can't see how TB_JUMP ends up doing anything in the rest of the file. Why not just replace this with s->is_jmp = DISAS_JUMP and not emit the lookup_and_goto_ptr here at all? r~
On Sat, Apr 29, 2017 at 12:30:08 +0200, Richard Henderson wrote: > On 04/28/2017 09:22 PM, Emilio G. Cota wrote: > >On Fri, Apr 28, 2017 at 15:17:24 -0400, Emilio G. Cota wrote: > >>+++ b/target/arm/translate-a64.c > >>@@ -373,8 +373,7 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest) > >> } else if (s->singlestep_enabled) { > >> gen_exception_internal(EXCP_DEBUG); > >> } else { > >>- tcg_gen_exit_tb(0); > >>- s->is_jmp = DISAS_TB_JUMP; > > > >I'm not sure about removing this line though. Would it be better to leave it? > >I can't see how TB_JUMP ends up doing anything in the rest of the file. > > Why not just replace this with > > s->is_jmp = DISAS_JUMP > > and not emit the lookup_and_goto_ptr here at all? If we don't emit anything here, we get the error you reported in the other message (icount whatever in cpu-exec.c:599). I think this is due to callers assuming get_goto_tb does indeed generate code, instead of deferring it via is_jmp. For example: if (cond < 0x0e) { /* genuinely conditional branches */ TCGLabel *label_match = gen_new_label(); arm_gen_test_cc(cond, label_match); gen_goto_tb(s, 0, s->pc); gen_set_label(label_match); gen_goto_tb(s, 1, addr); } else { [...] So the simplest solution here seems to just emit the goto_ptr helper in gen_goto_tb(). Regarding the setting of is_jmp to DISAS_TB_JUMP, after having looked at the code more closely, I think it shouldn't be removed, since this is the way we break out of the loop in gen_intermediate_code(), thereby marking this instruction as the last of the current TB. I have updated patch 1/2 accordingly. You can cherry-pick it from: https://github.com/cota/qemu/tree/tcg-next-v5+ Thanks, Emilio
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index 24de30d..5b691fc 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -373,8 +373,7 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest) } else if (s->singlestep_enabled) { gen_exception_internal(EXCP_DEBUG); } else { - tcg_gen_exit_tb(0); - s->is_jmp = DISAS_TB_JUMP; + tcg_gen_lookup_and_goto_ptr(cpu_pc); } } }
Perf numbers in next commit's log. Signed-off-by: Emilio G. Cota <cota@braap.org> --- target/arm/translate-a64.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)