Re: [PATCH v2 05/28] target/arm: Use gen_jmp for ISB and SB

[Peter Maydell]

On Thu, 8 Jul 2021 at 17:04, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 7/8/21 5:05 AM, Peter Maydell wrote:
> > On Wed, 30 Jun 2021 at 19:47, Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> Using gen_goto_tb directly misses the single-step check.
> >>
> >> Cc: qemu-arm@nongnu.org
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >>   target/arm/translate.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/target/arm/translate.c b/target/arm/translate.c
> >> index a0c6cfa902..8cd31feeaa 100644
> >> --- a/target/arm/translate.c
> >> +++ b/target/arm/translate.c
> >> @@ -8582,7 +8582,7 @@ static bool trans_ISB(DisasContext *s, arg_ISB *a)
> >>        * self-modifying code correctly and also to take
> >>        * any pending interrupts immediately.
> >>        */
> >> -    gen_goto_tb(s, 0, s->base.pc_next);
> >> +    gen_jmp(s, s->base.pc_next);
> >>       return true;
> >>   }
> >>
> >> @@ -8596,7 +8596,7 @@ static bool trans_SB(DisasContext *s, arg_SB *a)
> >>        * for TCG; MB and end the TB instead.
> >>        */
> >>       tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
> >> -    gen_goto_tb(s, 0, s->base.pc_next);
> >> +    gen_jmp(s, s->base.pc_next);
> >>       return true;
> >
> > Why isn't it enough here just to set is_jmp to DISAS_NEXT ?
>
> You mean DISAS_TOO_MANY?  That would work, yes.
> At the time I was just thinking of replacing one jump with another.

You've implicitly answered my question, which is that the main
translator loop code treats DISAS_NEXT as "keep adding insns to
the TB" :-)

It feels slightly like misuse to use DISAS_TOO_MANY, unless we
renamed it to something like DISAS_END_TB (which is what it's
actually doing).

thanks
-- PMM