[Qemu-devel] [PATCH] target-arm: Fix intptr_t vs tcg_target_long

[Qemu-devel] [PATCH] target-arm: Fix intptr_t vs tcg_target_long

[Richard Henderson]

Fixes a build error when these are different, e.g. x32.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-arm/translate-a64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 08ac659..37e05e8 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -210,7 +210,7 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest)
     if (use_goto_tb(s, n, dest)) {
         tcg_gen_goto_tb(n);
         gen_a64_set_pc_im(dest);
-        tcg_gen_exit_tb((tcg_target_long)tb + n);
+        tcg_gen_exit_tb((intptr_t)tb + n);
         s->is_jmp = DISAS_TB_JUMP;
     } else {
         gen_a64_set_pc_im(dest);
-- 
1.8.5.3

Re: [Qemu-devel] [PATCH] target-arm: Fix intptr_t vs tcg_target_long

[Peter Maydell]

On 5 March 2014 18:14, Richard Henderson <rth@twiddle.net> wrote:
> Fixes a build error when these are different, e.g. x32.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

A quick grep of the uses of tcg_gen_exit_tb() suggests
we would be better to change this function to take
(TranslationBlock *tb, int tb_exit_code), possibly
also with a special case for 0 if "tcg_gen_exit_tb(NULL, 0)"
seems too verbose.

I'll apply this to target-arm as the fix for 2.0, though.

> ---
>  target-arm/translate-a64.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 08ac659..37e05e8 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -210,7 +210,7 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest)
>      if (use_goto_tb(s, n, dest)) {
>          tcg_gen_goto_tb(n);
>          gen_a64_set_pc_im(dest);
> -        tcg_gen_exit_tb((tcg_target_long)tb + n);
> +        tcg_gen_exit_tb((intptr_t)tb + n);
>          s->is_jmp = DISAS_TB_JUMP;
>      } else {
>          gen_a64_set_pc_im(dest);
> --
> 1.8.5.3
>

thanks
-- PMM

Re: [Qemu-devel] [PATCH] target-arm: Fix intptr_t vs tcg_target_long

[Richard Henderson]

On 03/10/2014 05:08 AM, Peter Maydell wrote:
> A quick grep of the uses of tcg_gen_exit_tb() suggests
> we would be better to change this function to take
> (TranslationBlock *tb, int tb_exit_code), possibly
> also with a special case for 0 if "tcg_gen_exit_tb(NULL, 0)"
> seems too verbose.

No, since the goto_tb cases use (tb | n), so you wind up casting to an integer
type anyway.


r~

Re: [Qemu-devel] [PATCH] target-arm: Fix intptr_t vs tcg_target_long

[Peter Maydell]

On 10 March 2014 16:01, Richard Henderson <rth@twiddle.net> wrote:
> On 03/10/2014 05:08 AM, Peter Maydell wrote:
>> A quick grep of the uses of tcg_gen_exit_tb() suggests
>> we would be better to change this function to take
>> (TranslationBlock *tb, int tb_exit_code), possibly
>> also with a special case for 0 if "tcg_gen_exit_tb(NULL, 0)"
>> seems too verbose.
>
> No, since the goto_tb cases use (tb | n), so you wind up casting to an integer
> type anyway.

I'm confused. I can't see any callers in git grep
output which use 'tb | n':
$ git grep gen_exit_tb | grep -c '|'
0

The only cases I see are:
(1) tcg_gen_exit_tb(0)
(2) tcg_gen_exit_tb((uintptr_t)tb)
(3) tcg_gen_exit_tb((uintptr_t)tb + n)

which I am proposing would be better written as
  tcg_gen_exit_tb(NULL, 0)
  tcg_gen_exit_tb(tb, 0)
  tcg_gen_exit_tb(tb, n)

and letting tcg_gen_exit_tb() do the cast to uintptr_t
and add.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] target-arm: Fix intptr_t vs tcg_target_long

[Richard Henderson]

On 03/10/2014 09:06 AM, Peter Maydell wrote:
> which I am proposing would be better written as
>   tcg_gen_exit_tb(NULL, 0)
>   tcg_gen_exit_tb(tb, 0)
>   tcg_gen_exit_tb(tb, n)
> 
> and letting tcg_gen_exit_tb() do the cast to uintptr_t
> and add.

Oh, I see.  Yes, that would be fine.


r~