| Message ID | 1332191549-1311-2-git-send-email-sw@weilnetz.de |
|---|---|
| State | Under Review |
| Headers | show |
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; >
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
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
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;
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(-)