On 7/29/19 7:32 AM, Peter Maydell wrote: > On Fri, 26 Jul 2019 at 18:50, Richard Henderson > <richard.hender...@linaro.org> wrote: >> >> We will shortly be calling this function much more often. >> >> Signed-off-by: Richard Henderson <richard.hender...@linaro.org> >> --- >> target/arm/translate.c | 28 ++++++++++++---------------- >> 1 file changed, 12 insertions(+), 16 deletions(-) >> >> diff --git a/target/arm/translate.c b/target/arm/translate.c >> index 53c46fcdc4..36419025db 100644 >> --- a/target/arm/translate.c >> +++ b/target/arm/translate.c >> @@ -7705,8 +7705,14 @@ static void arm_gen_condlabel(DisasContext *s) >> /* Skip this instruction if the ARM condition is false */ >> static void arm_skip_unless(DisasContext *s, uint32_t cond) >> { >> - arm_gen_condlabel(s); >> - arm_gen_test_cc(cond ^ 1, s->condlabel); >> + /* >> + * Conditionally skip the insn. Note that both 0xe and 0xf mean >> + * "always"; 0xf is not "never". >> + */ >> + if (cond < 0xe) { >> + arm_gen_condlabel(s); >> + arm_gen_test_cc(cond ^ 1, s->condlabel); >> + } >> } >> >> static void disas_arm_insn(DisasContext *s, unsigned int insn) >> @@ -7944,11 +7950,9 @@ static void disas_arm_insn(DisasContext *s, unsigned >> int insn) >> } >> goto illegal_op; >> } >> - if (cond != 0xe) { >> - /* if not always execute, we generate a conditional jump to >> - next instruction */ >> - arm_skip_unless(s, cond); >> - } >> + >> + arm_skip_unless(s, cond); >> + >> if ((insn & 0x0f900000) == 0x03000000) { >> if ((insn & (1 << 21)) == 0) { >> ARCH(6T2); >> @@ -12095,15 +12099,7 @@ static void >> thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) >> dc->insn = insn; >> >> if (dc->condexec_mask && !thumb_insn_is_unconditional(dc, insn)) { >> - uint32_t cond = dc->condexec_cond; >> - >> - /* >> - * Conditionally skip the insn. Note that both 0xe and 0xf mean >> - * "always"; 0xf is not "never". >> - */ >> - if (cond < 0x0e) { >> - arm_skip_unless(dc, cond); >> - } >> + arm_skip_unless(dc, dc->condexec_cond); >> } > > In the other callsites for arm_skip_unless() the cond argument > can never be 0xe or 0xf. > > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
In my original version I included cond in the fields collected by decodetree, and so every single trans_* function called arm_skip_unless, so that would not be the case there. I discarded that in the version posted here, but I still think it might be a cleaner design overall. In the short term, maybe I should just discard this patch? r~