1. Patchew
  2. QEMU
  3. [PATCH v2 00/10] tcg: breakpoint reorg
  4. View patch

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

Richard Henderson posted 10 patches 3 years, 8 months ago
Maintainers: Laurent Vivier <laurent@vivier.eu>, Eduardo Habkost <ehabkost@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Cornelia Huck <cohuck@redhat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Bin Meng <bin.meng@windriver.com>, David Hildenbrand <david@redhat.com>, Thomas Huth <thuth@redhat.com>, Marek Vasut <marex@denx.de>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Michael Rolnik <mrolnik@gmail.com>, Peter Maydell <peter.maydell@linaro.org>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Alistair Francis <alistair.francis@wdc.com>, Greg Kurz <groug@kaod.org>, Richard Henderson <richard.henderson@linaro.org>, Max Filippov <jcmvbkbc@gmail.com>, Taylor Simpson <tsimpson@quicinc.com>, Palmer Dabbelt <palmer@dabbelt.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, Chris Wulff <crwulff@gmail.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Artyom Tarasenko <atar4qemu@gmail.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Stafford Horne <shorne@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>
There is a newer version of this series
[PATCH v2 01/10] accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS
Posted by Richard Henderson 3 years, 8 months ago 
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);
+
     if (cpu->singlestep_enabled || singlestep) {
         max_insns = 1;
     }
-- 
2.25.1
Re: [PATCH v2 01/10] accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS
Posted by Peter Maydell 3 years, 8 months ago 
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...

-- PMM
Re: [PATCH v2 01/10] accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS
Posted by Richard Henderson 3 years, 8 months ago 
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~
Re: [PATCH v2 01/10] accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS
Posted by Alex Bennée 3 years, 8 months ago 
Richard Henderson <richard.henderson@linaro.org> writes:

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

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.