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
