On Fri, Jul 14, 2017 at 23:42:42 -1000, Richard Henderson wrote: > We need not check for ARM vs Thumb state in order to dispatch > disassembly of every instruction. > > Signed-off-by: Richard Henderson <r...@twiddle.net> > --- > target/arm/translate.c | 134 > +++++++++++++++++++++++++++++++------------------ > 1 file changed, 86 insertions(+), 48 deletions(-) > > diff --git a/target/arm/translate.c b/target/arm/translate.c > index ebe1c1a..d7c3c10 100644 > --- a/target/arm/translate.c > +++ b/target/arm/translate.c (snip) > +static void arm_post_translate_insn(CPUARMState *env, DisasContext *dc) > +{ > + /* Translation stops when a conditional branch is encountered. > + * Otherwise the subsequent code could get translated several times. > + * Also stop translation when a page boundary is reached. This > + * ensures prefetch aborts occur at the right place. > + * > + * We want to stop the TB if the next insn starts in a new page, > + * or if it spans between this page and the next. This means that > + * if we're looking at the last halfword in the page we need to > + * see if it's a 16-bit Thumb insn (which will fit in this TB) > + * or a 32-bit Thumb insn (which won't). > + * This is to avoid generating a silly TB with a single 16-bit insn > + * in it at the end of this page (which would execute correctly > + * but isn't very efficient). > + */ > + if (dc->base.is_jmp == DISAS_NEXT > + && (dc->pc >= dc->next_page_start > + || (dc->pc >= dc->next_page_start - 3 > + && insn_crosses_page(env, dc)))) { > + dc->base.is_jmp = DISAS_TOO_MANY; > } > > if (dc->condjmp && !dc->base.is_jmp) { > gen_set_label(dc->condlabel); > dc->condjmp = 0; > } > + dc->base.pc_next = dc->pc; > + translator_loop_temp_check(&dc->base); > +} > > - if (dc->base.is_jmp == DISAS_NEXT) { > - /* Translation stops when a conditional branch is encountered. > - * Otherwise the subsequent code could get translated several times. > - * Also stop translation when a page boundary is reached. This > - * ensures prefetch aborts occur at the right place. */ > - > - if (dc->pc >= dc->next_page_start || > - (dc->pc >= dc->next_page_start - 3 && > - insn_crosses_page(env, dc))) { > - /* We want to stop the TB if the next insn starts in a new page, > - * or if it spans between this page and the next. This means that > - * if we're looking at the last halfword in the page we need to > - * see if it's a 16-bit Thumb insn (which will fit in this TB) > - * or a 32-bit Thumb insn (which won't). > - * This is to avoid generating a silly TB with a single 16-bit > insn > - * in it at the end of this page (which would execute correctly > - * but isn't very efficient). > - */ > - dc->base.is_jmp = DISAS_TOO_MANY;
Note that we've moved the above hunk ("if (dc->base.is_jmp == DISAS_NEXT)") above the "if (dc->condjmp && !dc->base.is_jmp)" one; so when we now set is_jmp to DISAS_TOO_MANY, dc->condjmp might be wrongly cleared by the second hunk. My review missed this, but luckily TCG asserts caught it while booting debian-arm: [ OK ] Started Increase datagram queue length. systemd[1]: Started Increase datagram queue length. [ OK ] Mounted POSIX Message Queue File System. systemd[1]: Mounted POSIX Message Queue File System. qemu-system-arm: /data/src/qemu/tcg/tcg-op.c:2584: tcg_gen_goto_tb: Assertion `(tcg_ctx.goto_tb_issue_mask & (1 << idx)) == 0' failed. Aborted (core dumped) Keeping the original order fixes it. Emilio --8<-- --- target/arm/translate.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index 77fe6a9..9cbace5 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -11979,6 +11979,11 @@ static bool arm_pre_translate_insn(DisasContext *dc) static void arm_post_translate_insn(CPUARMState *env, DisasContext *dc) { + if (dc->condjmp && !dc->base.is_jmp) { + gen_set_label(dc->condlabel); + dc->condjmp = 0; + } + /* Translation stops when a conditional branch is encountered. * Otherwise the subsequent code could get translated several times. * Also stop translation when a page boundary is reached. This @@ -12000,10 +12005,6 @@ static void arm_post_translate_insn(CPUARMState *env, DisasContext *dc) dc->base.is_jmp = DISAS_TOO_MANY; } - if (dc->condjmp && !dc->base.is_jmp) { - gen_set_label(dc->condlabel); - dc->condjmp = 0; - } dc->base.pc_next = dc->pc; translator_loop_temp_check(&dc->base); } -- 2.7.4