[PATCH 66/77] target/microblaze: Use tcg_gen_lookup_and_goto_ptr

Richard Henderson posted 77 patches 5 years, 5 months ago
Maintainers: Laurent Vivier <laurent@vivier.eu>, Richard Henderson <rth@twiddle.net>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
There is a newer version of this series
[PATCH 66/77] target/microblaze: Use tcg_gen_lookup_and_goto_ptr
Posted by Richard Henderson 5 years, 5 months ago
When goto_tb cannot be used due to branch page crossing,
or due to indirect jumping, tcg_gen_lookup_and_goto_ptr
can be used instead.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 6f9b20d391..5bd771671b 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -152,7 +152,7 @@ static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
         tcg_gen_exit_tb(dc->base.tb, n);
     } else {
         tcg_gen_movi_i32(cpu_pc, dest);
-        tcg_gen_exit_tb(NULL, 0);
+        tcg_gen_lookup_and_goto_ptr();
     }
     dc->base.is_jmp = DISAS_NORETURN;
 }
@@ -1811,7 +1811,7 @@ static void mb_tr_tb_stop(DisasContextBase *dcb, CPUState *cs)
         if (unlikely(cs->singlestep_enabled)) {
             gen_raise_exception(dc, EXCP_DEBUG);
         } else {
-            tcg_gen_exit_tb(NULL, 0);
+            tcg_gen_lookup_and_goto_ptr();
         }
         return;
 
-- 
2.25.1


Re: [PATCH 66/77] target/microblaze: Use tcg_gen_lookup_and_goto_ptr
Posted by Edgar E. Iglesias 5 years, 5 months ago
On Tue, Aug 25, 2020 at 01:59:39PM -0700, Richard Henderson wrote:
> When goto_tb cannot be used due to branch page crossing,
> or due to indirect jumping, tcg_gen_lookup_and_goto_ptr
> can be used instead.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Hi Richard,

This patch is for some reason causing some of our tests to fail.
The PMU Firmware on the ZynqMP gets stuck.
Looking at logs nothing obvious shows up, it just gets stuck.
Bisected it to this patch, any ideas?

Cheers,
Edgar

> ---
>  target/microblaze/translate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index 6f9b20d391..5bd771671b 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -152,7 +152,7 @@ static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
>          tcg_gen_exit_tb(dc->base.tb, n);
>      } else {
>          tcg_gen_movi_i32(cpu_pc, dest);
> -        tcg_gen_exit_tb(NULL, 0);
> +        tcg_gen_lookup_and_goto_ptr();
>      }
>      dc->base.is_jmp = DISAS_NORETURN;
>  }
> @@ -1811,7 +1811,7 @@ static void mb_tr_tb_stop(DisasContextBase *dcb, CPUState *cs)
>          if (unlikely(cs->singlestep_enabled)) {
>              gen_raise_exception(dc, EXCP_DEBUG);
>          } else {
> -            tcg_gen_exit_tb(NULL, 0);
> +            tcg_gen_lookup_and_goto_ptr();
>          }
>          return;
>  
> -- 
> 2.25.1
> 

Re: [PATCH 66/77] target/microblaze: Use tcg_gen_lookup_and_goto_ptr
Posted by Richard Henderson 5 years, 5 months ago
On 8/27/20 11:33 PM, Edgar E. Iglesias wrote:
> On Tue, Aug 25, 2020 at 01:59:39PM -0700, Richard Henderson wrote:
>> When goto_tb cannot be used due to branch page crossing,
>> or due to indirect jumping, tcg_gen_lookup_and_goto_ptr
>> can be used instead.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> 
> Hi Richard,
> 
> This patch is for some reason causing some of our tests to fail.
> The PMU Firmware on the ZynqMP gets stuck.
> Looking at logs nothing obvious shows up, it just gets stuck.
> Bisected it to this patch, any ideas?

Failure to raise an exception properly, I think.  This patch makes it much less
likely to return to the main cpu loop.

The problem with the bisection, I think, is that this patch is in the middle.
It might be worthwhile to apply it directly to master and see what happens.

That said, I don't see what we could have missed in translate, setting
cpustate_changed...


r~