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);
     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
 }
-- 
2.20.1

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/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

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~