Patchwork [2/2] ppc: Use uintptr_t for arguments of ppc_tb_set_jmp_target

login
register
mail settings
Submitter Stefan Weil
Date March 19, 2012, 9:12 p.m.
Message ID <1332191549-1311-2-git-send-email-sw@weilnetz.de>
Download mbox | patch
Permalink /patch/147625/
State Under Review
Headers show

Comments

Stefan Weil - March 19, 2012, 9:12 p.m.
The previous commit changed function tb_set_jmp_target1 and is needed
for w64 hosts.

This patch is not needed for w64, but it synchronizes tb_set_jmp_target1
and ppc_tb_set_jmp_target so that both functions have the same signature.

Cc: malc <av1474@comtv.ru>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 exec-all.h             |    2 +-
 tcg/ppc/tcg-target.c   |    2 +-
 tcg/ppc64/tcg-target.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
malc - March 19, 2012, 9:33 p.m.
On Mon, 19 Mar 2012, Stefan Weil wrote:

> The previous commit changed function tb_set_jmp_target1 and is needed
> for w64 hosts.
> 
> This patch is not needed for w64, but it synchronizes tb_set_jmp_target1
> and ppc_tb_set_jmp_target so that both functions have the same signature.
> 
> Cc: malc <av1474@comtv.ru>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  exec-all.h             |    2 +-
>  tcg/ppc/tcg-target.c   |    2 +-
>  tcg/ppc64/tcg-target.c |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/exec-all.h b/exec-all.h
> index a6d6519..9ffd778 100644
> --- a/exec-all.h
> +++ b/exec-all.h
> @@ -199,7 +199,7 @@ static inline void tb_set_jmp_target1(uintptr_t jmp_addr, uintptr_t addr)
>      /* no need to flush icache explicitly */
>  }
>  #elif defined(_ARCH_PPC)
> -void ppc_tb_set_jmp_target(unsigned long jmp_addr, unsigned long addr);
> +void ppc_tb_set_jmp_target(uintptr_t jmp_addr, uintptr_t addr);
>  #define tb_set_jmp_target1 ppc_tb_set_jmp_target
>  #elif defined(__i386__) || defined(__x86_64__)
>  static inline void tb_set_jmp_target1(uintptr_t jmp_addr, uintptr_t addr)
> diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
> index b0aa914..57000e5 100644
> --- a/tcg/ppc/tcg-target.c
> +++ b/tcg/ppc/tcg-target.c
> @@ -1305,7 +1305,7 @@ static void tcg_out_brcond2 (TCGContext *s, const TCGArg *args,
>      tcg_out_bc (s, (BC | BI (7, CR_EQ) | BO_COND_TRUE), args[5]);
>  }
>  
> -void ppc_tb_set_jmp_target (unsigned long jmp_addr, unsigned long addr)
> +void ppc_tb_set_jmp_target (uintptr_t jmp_addr, uintptr_t addr)
>  {
>      uint32_t *ptr;
>      long disp = addr - jmp_addr;

This should become intptr_t then..
That said ppc32 code assumes 32bit addresses, and ppc64 tcg_taget_long
wide ones.. IOW needs some thinking.

> diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
> index 409a1ac..c286322 100644
> --- a/tcg/ppc64/tcg-target.c
> +++ b/tcg/ppc64/tcg-target.c
> @@ -1233,7 +1233,7 @@ static void tcg_out_brcond (TCGContext *s, TCGCond cond,
>      tcg_out_bc (s, tcg_to_bc[cond], label_index);
>  }
>  
> -void ppc_tb_set_jmp_target (unsigned long jmp_addr, unsigned long addr)
> +void ppc_tb_set_jmp_target (uintptr_t jmp_addr, uintptr_t addr)
>  {
>      TCGContext s;
>      unsigned long patch_size;
>
Andreas Färber - March 19, 2012, 9:56 p.m.
Am 19.03.2012 22:33, schrieb malc:
> On Mon, 19 Mar 2012, Stefan Weil wrote:
> 
>> The previous commit changed function tb_set_jmp_target1 and is needed
>> for w64 hosts.
>>
>> This patch is not needed for w64, but it synchronizes tb_set_jmp_target1
>> and ppc_tb_set_jmp_target so that both functions have the same signature.
>>
>> Cc: malc <av1474@comtv.ru>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>  exec-all.h             |    2 +-
>>  tcg/ppc/tcg-target.c   |    2 +-
>>  tcg/ppc64/tcg-target.c |    2 +-
>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/exec-all.h b/exec-all.h
>> index a6d6519..9ffd778 100644
>> --- a/exec-all.h
>> +++ b/exec-all.h
>> @@ -199,7 +199,7 @@ static inline void tb_set_jmp_target1(uintptr_t jmp_addr, uintptr_t addr)
>>      /* no need to flush icache explicitly */
>>  }
>>  #elif defined(_ARCH_PPC)
>> -void ppc_tb_set_jmp_target(unsigned long jmp_addr, unsigned long addr);
>> +void ppc_tb_set_jmp_target(uintptr_t jmp_addr, uintptr_t addr);
>>  #define tb_set_jmp_target1 ppc_tb_set_jmp_target
>>  #elif defined(__i386__) || defined(__x86_64__)
>>  static inline void tb_set_jmp_target1(uintptr_t jmp_addr, uintptr_t addr)
>> diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
>> index b0aa914..57000e5 100644
>> --- a/tcg/ppc/tcg-target.c
>> +++ b/tcg/ppc/tcg-target.c
>> @@ -1305,7 +1305,7 @@ static void tcg_out_brcond2 (TCGContext *s, const TCGArg *args,
>>      tcg_out_bc (s, (BC | BI (7, CR_EQ) | BO_COND_TRUE), args[5]);
>>  }
>>  
>> -void ppc_tb_set_jmp_target (unsigned long jmp_addr, unsigned long addr)
>> +void ppc_tb_set_jmp_target (uintptr_t jmp_addr, uintptr_t addr)
>>  {
>>      uint32_t *ptr;
>>      long disp = addr - jmp_addr;
> 
> This should become intptr_t then..

> That said ppc32 code assumes 32bit addresses, and ppc64 tcg_taget_long
> wide ones.. IOW needs some thinking.

Hm? On both host platforms relevant here, Linux and Darwin, long and
intptr_t should have the same width, on both ppc and ppc64, so no
practical difference. I was about to add my Acked-by - where do you see
issues? Or do you just see room for further code improvements elsewhere?

Andreas
malc - March 19, 2012, 11:16 p.m.
On Mon, 19 Mar 2012, Andreas F?rber wrote:

> Am 19.03.2012 22:33, schrieb malc:
> > On Mon, 19 Mar 2012, Stefan Weil wrote:
> > 
> >> The previous commit changed function tb_set_jmp_target1 and is needed
> >> for w64 hosts.
> >>
> >> This patch is not needed for w64, but it synchronizes tb_set_jmp_target1
> >> and ppc_tb_set_jmp_target so that both functions have the same signature.

[..snip..]

> > 
> > This should become intptr_t then..
> 
> > That said ppc32 code assumes 32bit addresses, and ppc64 tcg_taget_long
> > wide ones.. IOW needs some thinking.
> 
> Hm? On both host platforms relevant here, Linux and Darwin, long and
> intptr_t should have the same width, on both ppc and ppc64, so no
> practical difference. I was about to add my Acked-by - where do you see
> issues? Or do you just see room for further code improvements elsewhere?
> 
> Andreas

There's AIX and BSDs. long and intpr_t having same width is not the 
issue, the issue is(can be/whatever) careless replacement, for instance
ppc64 defines tb_set_jmp_target in terms of tcg_out_b and it's argument
is tcg_target_long, and quoting[1]

  Elsewhere I have opinioned that the only purpose for having 
  more than one type of integer in your programming language is so that 
  programmers can pick the wrong one.

What i'm saying is - the mere fact that i have to think about the
issue at all is telling.

There's no doubt that x86_64 change is a good thing (fixes win64), here
too many types are involved already, makes me uncomfortable.

[1] http://permalink.gmane.org/gmane.comp.lang.caml.inria/36258

Patch

diff --git a/exec-all.h b/exec-all.h
index a6d6519..9ffd778 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -199,7 +199,7 @@  static inline void tb_set_jmp_target1(uintptr_t jmp_addr, uintptr_t addr)
     /* no need to flush icache explicitly */
 }
 #elif defined(_ARCH_PPC)
-void ppc_tb_set_jmp_target(unsigned long jmp_addr, unsigned long addr);
+void ppc_tb_set_jmp_target(uintptr_t jmp_addr, uintptr_t addr);
 #define tb_set_jmp_target1 ppc_tb_set_jmp_target
 #elif defined(__i386__) || defined(__x86_64__)
 static inline void tb_set_jmp_target1(uintptr_t jmp_addr, uintptr_t addr)
diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index b0aa914..57000e5 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -1305,7 +1305,7 @@  static void tcg_out_brcond2 (TCGContext *s, const TCGArg *args,
     tcg_out_bc (s, (BC | BI (7, CR_EQ) | BO_COND_TRUE), args[5]);
 }
 
-void ppc_tb_set_jmp_target (unsigned long jmp_addr, unsigned long addr)
+void ppc_tb_set_jmp_target (uintptr_t jmp_addr, uintptr_t addr)
 {
     uint32_t *ptr;
     long disp = addr - jmp_addr;
diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
index 409a1ac..c286322 100644
--- a/tcg/ppc64/tcg-target.c
+++ b/tcg/ppc64/tcg-target.c
@@ -1233,7 +1233,7 @@  static void tcg_out_brcond (TCGContext *s, TCGCond cond,
     tcg_out_bc (s, tcg_to_bc[cond], label_index);
 }
 
-void ppc_tb_set_jmp_target (unsigned long jmp_addr, unsigned long addr)
+void ppc_tb_set_jmp_target (uintptr_t jmp_addr, uintptr_t addr)
 {
     TCGContext s;
     unsigned long patch_size;