| Message ID | 20210210221053.18050-17-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | plugins/next pre-PR (hwprofile, regression fixes, icount count fix) | expand |
Hi Alex, On 2/10/21 11:10 PM, Alex Bennée wrote: > When we exit a block under icount with instructions left to execute we > might need a shorter than normal block to take us to the next > deterministic event. Instead of creating a throwaway block on demand > we use the existing compile flags mechanism to ensure we fetch (or > compile and fetch) a block with exactly the number of instructions we > need. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Message-Id: <20210209182749.31323-8-alex.bennee@linaro.org> > > --- > v2 > - drop pointless assert > --- > accel/tcg/cpu-exec.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index d9ef69121c..5b6a4fe84b 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -730,16 +730,17 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb, > /* Ensure global icount has gone forward */ > icount_update(cpu); > /* Refill decrementer and continue execution. */ > - insns_left = MIN(0xffff, cpu->icount_budget); > + insns_left = MIN(CF_COUNT_MASK, cpu->icount_budget); Can you describe this change a bit please? > cpu_neg(cpu)->icount_decr.u16.low = insns_left; > cpu->icount_extra = cpu->icount_budget - insns_left; > - if (!cpu->icount_extra && insns_left < tb->icount) { > - /* Execute any remaining instructions, then let the main loop > - * handle the next event. > - */ > - if (insns_left > 0) { > - cpu_exec_nocache(cpu, insns_left, tb, false); > - } > + > + /* > + * If the next tb has more instructions than we have left to > + * execute we need to ensure we find/generate a TB with exactly > + * insns_left instructions in it. > + */ > + if (!cpu->icount_extra && insns_left > 0 && insns_left < tb->icount) { > + cpu->cflags_next_tb = (tb->cflags & ~CF_COUNT_MASK) | insns_left; > } > #endif > } >
On 2/11/21 2:21 AM, Philippe Mathieu-Daudé wrote: >> - insns_left = MIN(0xffff, cpu->icount_budget); >> + insns_left = MIN(CF_COUNT_MASK, cpu->icount_budget); > > Can you describe this change a bit please? Replacing a magic number with its proper symbol. r~
On 2/10/21 2:10 PM, Alex Bennée wrote: > When we exit a block under icount with instructions left to execute we > might need a shorter than normal block to take us to the next > deterministic event. Instead of creating a throwaway block on demand > we use the existing compile flags mechanism to ensure we fetch (or > compile and fetch) a block with exactly the number of instructions we > need. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Message-Id: <20210209182749.31323-8-alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 2/11/21 7:48 PM, Richard Henderson wrote: > On 2/11/21 2:21 AM, Philippe Mathieu-Daudé wrote: >>> - insns_left = MIN(0xffff, cpu->icount_budget); >>> + insns_left = MIN(CF_COUNT_MASK, cpu->icount_budget); >> >> Can you describe this change a bit please? > > Replacing a magic number with its proper symbol. I am confuse because I see: #define CF_COUNT_MASK 0x00007fff
Philippe Mathieu-Daudé <f4bug@amsat.org> writes: > On 2/11/21 7:48 PM, Richard Henderson wrote: >> On 2/11/21 2:21 AM, Philippe Mathieu-Daudé wrote: >>>> - insns_left = MIN(0xffff, cpu->icount_budget); >>>> + insns_left = MIN(CF_COUNT_MASK, cpu->icount_budget); >>> >>> Can you describe this change a bit please? >> >> Replacing a magic number with its proper symbol. > > I am confuse because I see: > > #define CF_COUNT_MASK 0x00007fff This is the largest partial count you can store in the CF flags (0x8000 is used for LAST_IO). The decrement field can handle the full u16 although in practice I would never expect a final block to be more than a few instructions. We could probably shorten the mask without any deleterious effect if we needed to scrape together any more CFLAGS. -- Alex Bennée
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index d9ef69121c..5b6a4fe84b 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -730,16 +730,17 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb, /* Ensure global icount has gone forward */ icount_update(cpu); /* Refill decrementer and continue execution. */ - insns_left = MIN(0xffff, cpu->icount_budget); + insns_left = MIN(CF_COUNT_MASK, cpu->icount_budget); cpu_neg(cpu)->icount_decr.u16.low = insns_left; cpu->icount_extra = cpu->icount_budget - insns_left; - if (!cpu->icount_extra && insns_left < tb->icount) { - /* Execute any remaining instructions, then let the main loop - * handle the next event. - */ - if (insns_left > 0) { - cpu_exec_nocache(cpu, insns_left, tb, false); - } + + /* + * If the next tb has more instructions than we have left to + * execute we need to ensure we find/generate a TB with exactly + * insns_left instructions in it. + */ + if (!cpu->icount_extra && insns_left > 0 && insns_left < tb->icount) { + cpu->cflags_next_tb = (tb->cflags & ~CF_COUNT_MASK) | insns_left; } #endif }
When we exit a block under icount with instructions left to execute we might need a shorter than normal block to take us to the next deterministic event. Instead of creating a throwaway block on demand we use the existing compile flags mechanism to ensure we fetch (or compile and fetch) a block with exactly the number of instructions we need. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-Id: <20210209182749.31323-8-alex.bennee@linaro.org> --- v2 - drop pointless assert --- accel/tcg/cpu-exec.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) -- 2.20.1