Add a generic version of the common use_goto_tb test.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/translator.h | 10 ++++++++++
 accel/tcg/translator.c    | 11 +++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/exec/translator.h b/include/exec/translator.h
index 24232ead41..dd9c06d40d 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -145,6 +145,16 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
 
 void translator_loop_temp_check(DisasContextBase *db);
 
+/**
+ * translator_use_goto_tb
+ * @db: Disassembly context
+ * @dest: target pc of the goto
+ *
+ * Return true if goto_tb is allowed between the current TB
+ * and the destination PC.
+ */
+bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest);
+
 /*
  * Translator Load Functions
  *
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 1d32732198..59804af37b 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -31,6 +31,17 @@ void translator_loop_temp_check(DisasContextBase *db)
     }
 }
 
+bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest)
+{
+    /* Suppress goto_tb in the case of single-steping.  */
+    if (db->singlestep_enabled || singlestep) {
+        return false;
+    }
+
+    /* Check for the dest on the same page as the start of the TB.  */
+    return ((db->pc_first ^ dest) & TARGET_PAGE_MASK) == 0;
+}
+
 void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
                      CPUState *cpu, TranslationBlock *tb, int max_insns)
 {
-- 
2.25.1

On Sun, Jun 20, 2021 at 6:36 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Add a generic version of the common use_goto_tb test.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/translator.h | 10 ++++++++++
>  accel/tcg/translator.c    | 11 +++++++++++
>  2 files changed, 21 insertions(+)

Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>

-- 
Thanks.
-- Max

Hi Richard,

On 6/21/21 3:34 AM, Richard Henderson wrote:
> Add a generic version of the common use_goto_tb test.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/translator.h | 10 ++++++++++
>  accel/tcg/translator.c    | 11 +++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/include/exec/translator.h b/include/exec/translator.h
> index 24232ead41..dd9c06d40d 100644
> --- a/include/exec/translator.h
> +++ b/include/exec/translator.h
> @@ -145,6 +145,16 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
>  
>  void translator_loop_temp_check(DisasContextBase *db);
>  
> +/**
> + * translator_use_goto_tb
> + * @db: Disassembly context
> + * @dest: target pc of the goto
> + *
> + * Return true if goto_tb is allowed between the current TB
> + * and the destination PC.
> + */
> +bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest);
> +
>  /*
>   * Translator Load Functions
>   *
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index 1d32732198..59804af37b 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -31,6 +31,17 @@ void translator_loop_temp_check(DisasContextBase *db)
>      }
>  }
>  
> +bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest)
> +{
> +    /* Suppress goto_tb in the case of single-steping.  */
> +    if (db->singlestep_enabled || singlestep) {
> +        return false;
> +    }
> +

I notice various targets do:

#ifdef CONFIG_USER_ONLY
       return true;
#else

> +    /* Check for the dest on the same page as the start of the TB.  */
> +    return ((db->pc_first ^ dest) & TARGET_PAGE_MASK) == 0;

#endif

> +}

Is that OK to remove this case? If so, it might be worth a comment
somewhere.

Regards,

Phil.

On 6/21/21 5:50 AM, Philippe Mathieu-Daudé wrote:
> I notice various targets do:
> 
> #ifdef CONFIG_USER_ONLY
>         return true;
> #else
> 
>> +    /* Check for the dest on the same page as the start of the TB.  */
>> +    return ((db->pc_first ^ dest) & TARGET_PAGE_MASK) == 0;
> #endif
> 
>> +}
> Is that OK to remove this case? If so, it might be worth a comment
> somewhere.

I mentioned it in the cover letter.


r~

On 6/21/21 3:47 PM, Richard Henderson wrote:
> On 6/21/21 5:50 AM, Philippe Mathieu-Daudé wrote:
>> I notice various targets do:
>>
>> #ifdef CONFIG_USER_ONLY
>>         return true;
>> #else
>>
>>> +    /* Check for the dest on the same page as the start of the TB.  */
>>> +    return ((db->pc_first ^ dest) & TARGET_PAGE_MASK) == 0;
>> #endif
>>
>>> +}
>> Is that OK to remove this case? If so, it might be worth a comment
>> somewhere.
> 
> I mentioned it in the cover letter.

But the commit letter will vanish, so preferably mentioning
the change in the commit description (copying the cover letter
comment could work too):

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

From: Richard Henderson <richard.henderson@linaro.org>
> Add a generic version of the common use_goto_tb test.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/translator.h | 10 ++++++++++
>  accel/tcg/translator.c    | 11 +++++++++++
>  2 files changed, 21 insertions(+)

Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>