Re: [PATCH v2 01/10] accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS

[Richard Henderson]

On 7/17/21 10:30 AM, Peter Maydell wrote:
> On Mon, 12 Jul 2021 at 16:42, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> > The space reserved for CF_COUNT_MASK was overly large.
> > Reduce to free up cflags bits and eliminate an extra test.
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   include/exec/exec-all.h   | 4 +++-
> >   accel/tcg/translate-all.c | 5 ++---
> >   2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> > index 754f4130c9..dfe82ed19c 100644
> > --- a/include/exec/exec-all.h
> > +++ b/include/exec/exec-all.h
> > @@ -492,7 +492,9 @@ struct TranslationBlock {
> >       target_ulong cs_base; /* CS base for this block */
> >       uint32_t flags; /* flags defining in which context the code was generated 
> > */
> >       uint32_t cflags;    /* compile flags */
> > -#define CF_COUNT_MASK  0x00007fff
> > +
> > +/* Note that TCG_MAX_INSNS is 512; we validate this match elsewhere. */
> > +#define CF_COUNT_MASK  0x000001ff
> >   #define CF_LAST_IO     0x00008000 /* Last insn may be an IO access.  */
> >   #define CF_MEMI_ONLY   0x00010000 /* Only instrument memory ops */
> >   #define CF_USE_ICOUNT  0x00020000
> > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> > index 4df26de858..997e44c73b 100644
> > --- a/accel/tcg/translate-all.c
> > +++ b/accel/tcg/translate-all.c
> > @@ -1430,9 +1430,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> >       if (max_insns == 0) {
> >           max_insns = CF_COUNT_MASK;
> >       }
> > -    if (max_insns > TCG_MAX_INSNS) {
> > -        max_insns = TCG_MAX_INSNS;
> > -    }
> > +    QEMU_BUILD_BUG_ON(CF_COUNT_MASK + 1 != TCG_MAX_INSNS);
> > +
>
> Previously we would allow max_insns = TCG_MAX_INSNS (512).
> Now we only allow it to be 511...

Hmm.  Well, 0 is supposed to map to "max", currently written as

    max_insns = cflags & CF_COUNT_MASK;
    if (max_insns == 0) {
        max_insns = CF_COUNT_MASK;
    }

I could just as easily map 0 -> TCG_MAX_INSNS instead.


r~