* [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations
@ 2010-01-01 15:41 Alexander Graf
2010-01-03 10:25 ` Stefan Weil
2010-01-14 15:13 ` Aurelien Jarno
0 siblings, 2 replies; 8+ messages in thread
From: Alexander Graf @ 2010-01-01 15:41 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien
The recent transition to always have the DCR helper functions take 32 bit
values broke the PPC64 target, as tlong became 64 bits there.
This patch moves all translate.c callers to a _tl function that simply
calls the uint32_t functions. That way we don't need to mess with TCG
trying to pass registers as uint32_t variables to functions.
Fixes PPC64 build with --enable-debug-tcg
Signed-off-by: Alexander Graf <agraf@suse.de>
Reported-by: Stefan Weil <weil@mail.berlios.de>
---
target-ppc/cpu.h | 2 ++
target-ppc/helper.h | 4 ++--
target-ppc/op_helper.c | 10 ++++++++++
target-ppc/translate.c | 12 ++++++------
4 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index d15bba1..60a8b68 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -733,6 +733,8 @@ void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs);
void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value);
#endif /* !defined(CONFIG_USER_ONLY) */
void ppc_store_msr (CPUPPCState *env, target_ulong value);
+void helper_store_dcr (uint32_t dcrn, uint32_t val);
+uint32_t helper_load_dcr (uint32_t dcrn);
void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 40d4ced..86f0af7 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -359,8 +359,8 @@ DEF_HELPER_2(divo, tl, tl, tl)
DEF_HELPER_2(divs, tl, tl, tl)
DEF_HELPER_2(divso, tl, tl, tl)
-DEF_HELPER_1(load_dcr, i32, i32);
-DEF_HELPER_2(store_dcr, void, i32, i32)
+DEF_HELPER_1(load_dcr_tl, tl, tl);
+DEF_HELPER_2(store_dcr_tl, void, tl, tl)
DEF_HELPER_1(load_dump_spr, void, i32)
DEF_HELPER_1(store_dump_spr, void, i32)
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index cea27f2..6c375d3 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -1844,6 +1844,11 @@ uint32_t helper_load_dcr (uint32_t dcrn)
return val;
}
+target_ulong helper_load_dcr_tl (target_ulong dcrn)
+{
+ return (uint32_t)helper_load_dcr((uint32_t)dcrn);
+}
+
void helper_store_dcr (uint32_t dcrn, uint32_t val)
{
if (unlikely(env->dcr_env == NULL)) {
@@ -1857,6 +1862,11 @@ void helper_store_dcr (uint32_t dcrn, uint32_t val)
}
}
+void helper_store_dcr_tl (target_ulong dcrn, target_ulong val)
+{
+ helper_store_dcr((uint32_t)dcrn, (uint32_t)val);
+}
+
#if !defined(CONFIG_USER_ONLY)
void helper_40x_rfci (void)
{
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index d4e81ce..d83d196 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -5565,7 +5565,7 @@ static void gen_mfdcr(DisasContext *ctx)
/* NIP cannot be restored if the memory exception comes from an helper */
gen_update_nip(ctx, ctx->nip - 4);
dcrn = tcg_const_tl(SPR(ctx->opcode));
- gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], dcrn);
+ gen_helper_load_dcr_tl(cpu_gpr[rD(ctx->opcode)], dcrn);
tcg_temp_free(dcrn);
#endif
}
@@ -5584,7 +5584,7 @@ static void gen_mtdcr(DisasContext *ctx)
/* NIP cannot be restored if the memory exception comes from an helper */
gen_update_nip(ctx, ctx->nip - 4);
dcrn = tcg_const_tl(SPR(ctx->opcode));
- gen_helper_store_dcr(dcrn, cpu_gpr[rS(ctx->opcode)]);
+ gen_helper_store_dcr_tl(dcrn, cpu_gpr[rS(ctx->opcode)]);
tcg_temp_free(dcrn);
#endif
}
@@ -5602,7 +5602,7 @@ static void gen_mfdcrx(DisasContext *ctx)
}
/* NIP cannot be restored if the memory exception comes from an helper */
gen_update_nip(ctx, ctx->nip - 4);
- gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
+ gen_helper_load_dcr_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
/* Note: Rc update flag set leads to undefined state of Rc0 */
#endif
}
@@ -5620,7 +5620,7 @@ static void gen_mtdcrx(DisasContext *ctx)
}
/* NIP cannot be restored if the memory exception comes from an helper */
gen_update_nip(ctx, ctx->nip - 4);
- gen_helper_store_dcr(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
+ gen_helper_store_dcr_tl(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
/* Note: Rc update flag set leads to undefined state of Rc0 */
#endif
}
@@ -5630,7 +5630,7 @@ static void gen_mfdcrux(DisasContext *ctx)
{
/* NIP cannot be restored if the memory exception comes from an helper */
gen_update_nip(ctx, ctx->nip - 4);
- gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
+ gen_helper_load_dcr_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
/* Note: Rc update flag set leads to undefined state of Rc0 */
}
@@ -5639,7 +5639,7 @@ static void gen_mtdcrux(DisasContext *ctx)
{
/* NIP cannot be restored if the memory exception comes from an helper */
gen_update_nip(ctx, ctx->nip - 4);
- gen_helper_store_dcr(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
+ gen_helper_store_dcr_tl(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
/* Note: Rc update flag set leads to undefined state of Rc0 */
}
--
1.6.0.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations
2010-01-01 15:41 [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations Alexander Graf
@ 2010-01-03 10:25 ` Stefan Weil
2010-01-14 15:13 ` Aurelien Jarno
1 sibling, 0 replies; 8+ messages in thread
From: Stefan Weil @ 2010-01-03 10:25 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-devel, aurelien
Alexander Graf schrieb:
> The recent transition to always have the DCR helper functions take 32 bit
> values broke the PPC64 target, as tlong became 64 bits there.
>
> This patch moves all translate.c callers to a _tl function that simply
> calls the uint32_t functions. That way we don't need to mess with TCG
> trying to pass registers as uint32_t variables to functions.
>
> Fixes PPC64 build with --enable-debug-tcg
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Reported-by: Stefan Weil <weil@mail.berlios.de>
Acked-by: Stefan Weil <weil@mail.berlios.de>
Happy new year,
Stefan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations
2010-01-01 15:41 [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations Alexander Graf
2010-01-03 10:25 ` Stefan Weil
@ 2010-01-14 15:13 ` Aurelien Jarno
2010-01-14 15:19 ` Alexander Graf
1 sibling, 1 reply; 8+ messages in thread
From: Aurelien Jarno @ 2010-01-14 15:13 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-devel
On Fri, Jan 01, 2010 at 04:41:06PM +0100, Alexander Graf wrote:
> The recent transition to always have the DCR helper functions take 32 bit
> values broke the PPC64 target, as tlong became 64 bits there.
>
> This patch moves all translate.c callers to a _tl function that simply
> calls the uint32_t functions. That way we don't need to mess with TCG
> trying to pass registers as uint32_t variables to functions.
>
> Fixes PPC64 build with --enable-debug-tcg
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Reported-by: Stefan Weil <weil@mail.berlios.de>
> ---
> target-ppc/cpu.h | 2 ++
> target-ppc/helper.h | 4 ++--
> target-ppc/op_helper.c | 10 ++++++++++
> target-ppc/translate.c | 12 ++++++------
> 4 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index d15bba1..60a8b68 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -733,6 +733,8 @@ void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs);
> void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value);
> #endif /* !defined(CONFIG_USER_ONLY) */
> void ppc_store_msr (CPUPPCState *env, target_ulong value);
> +void helper_store_dcr (uint32_t dcrn, uint32_t val);
> +uint32_t helper_load_dcr (uint32_t dcrn);
>
> void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
>
> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> index 40d4ced..86f0af7 100644
> --- a/target-ppc/helper.h
> +++ b/target-ppc/helper.h
> @@ -359,8 +359,8 @@ DEF_HELPER_2(divo, tl, tl, tl)
> DEF_HELPER_2(divs, tl, tl, tl)
> DEF_HELPER_2(divso, tl, tl, tl)
>
> -DEF_HELPER_1(load_dcr, i32, i32);
> -DEF_HELPER_2(store_dcr, void, i32, i32)
> +DEF_HELPER_1(load_dcr_tl, tl, tl);
> +DEF_HELPER_2(store_dcr_tl, void, tl, tl)
>
> DEF_HELPER_1(load_dump_spr, void, i32)
> DEF_HELPER_1(store_dump_spr, void, i32)
> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
> index cea27f2..6c375d3 100644
> --- a/target-ppc/op_helper.c
> +++ b/target-ppc/op_helper.c
> @@ -1844,6 +1844,11 @@ uint32_t helper_load_dcr (uint32_t dcrn)
> return val;
> }
>
> +target_ulong helper_load_dcr_tl (target_ulong dcrn)
> +{
> + return (uint32_t)helper_load_dcr((uint32_t)dcrn);
> +}
> +
> void helper_store_dcr (uint32_t dcrn, uint32_t val)
> {
> if (unlikely(env->dcr_env == NULL)) {
> @@ -1857,6 +1862,11 @@ void helper_store_dcr (uint32_t dcrn, uint32_t val)
> }
> }
>
> +void helper_store_dcr_tl (target_ulong dcrn, target_ulong val)
> +{
> + helper_store_dcr((uint32_t)dcrn, (uint32_t)val);
> +}
> +
I do wonder why we need to keep the old helper_load_dcr() and
helper_store_dcr() instead of modifying them directly. They doesn't seems
to be used elsewhere.
> #if !defined(CONFIG_USER_ONLY)
> void helper_40x_rfci (void)
> {
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index d4e81ce..d83d196 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -5565,7 +5565,7 @@ static void gen_mfdcr(DisasContext *ctx)
> /* NIP cannot be restored if the memory exception comes from an helper */
> gen_update_nip(ctx, ctx->nip - 4);
> dcrn = tcg_const_tl(SPR(ctx->opcode));
> - gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], dcrn);
> + gen_helper_load_dcr_tl(cpu_gpr[rD(ctx->opcode)], dcrn);
> tcg_temp_free(dcrn);
> #endif
> }
> @@ -5584,7 +5584,7 @@ static void gen_mtdcr(DisasContext *ctx)
> /* NIP cannot be restored if the memory exception comes from an helper */
> gen_update_nip(ctx, ctx->nip - 4);
> dcrn = tcg_const_tl(SPR(ctx->opcode));
> - gen_helper_store_dcr(dcrn, cpu_gpr[rS(ctx->opcode)]);
> + gen_helper_store_dcr_tl(dcrn, cpu_gpr[rS(ctx->opcode)]);
> tcg_temp_free(dcrn);
> #endif
> }
> @@ -5602,7 +5602,7 @@ static void gen_mfdcrx(DisasContext *ctx)
> }
> /* NIP cannot be restored if the memory exception comes from an helper */
> gen_update_nip(ctx, ctx->nip - 4);
> - gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
> + gen_helper_load_dcr_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
> /* Note: Rc update flag set leads to undefined state of Rc0 */
> #endif
> }
> @@ -5620,7 +5620,7 @@ static void gen_mtdcrx(DisasContext *ctx)
> }
> /* NIP cannot be restored if the memory exception comes from an helper */
> gen_update_nip(ctx, ctx->nip - 4);
> - gen_helper_store_dcr(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
> + gen_helper_store_dcr_tl(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
> /* Note: Rc update flag set leads to undefined state of Rc0 */
> #endif
> }
> @@ -5630,7 +5630,7 @@ static void gen_mfdcrux(DisasContext *ctx)
> {
> /* NIP cannot be restored if the memory exception comes from an helper */
> gen_update_nip(ctx, ctx->nip - 4);
> - gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
> + gen_helper_load_dcr_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
> /* Note: Rc update flag set leads to undefined state of Rc0 */
> }
>
> @@ -5639,7 +5639,7 @@ static void gen_mtdcrux(DisasContext *ctx)
> {
> /* NIP cannot be restored if the memory exception comes from an helper */
> gen_update_nip(ctx, ctx->nip - 4);
> - gen_helper_store_dcr(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
> + gen_helper_store_dcr_tl(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
> /* Note: Rc update flag set leads to undefined state of Rc0 */
> }
>
> --
> 1.6.0.2
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations
2010-01-14 15:13 ` Aurelien Jarno
@ 2010-01-14 15:19 ` Alexander Graf
2010-01-14 17:02 ` Aurelien Jarno
0 siblings, 1 reply; 8+ messages in thread
From: Alexander Graf @ 2010-01-14 15:19 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
On 14.01.2010, at 16:13, Aurelien Jarno wrote:
> On Fri, Jan 01, 2010 at 04:41:06PM +0100, Alexander Graf wrote:
>> The recent transition to always have the DCR helper functions take 32 bit
>> values broke the PPC64 target, as tlong became 64 bits there.
>>
>> This patch moves all translate.c callers to a _tl function that simply
>> calls the uint32_t functions. That way we don't need to mess with TCG
>> trying to pass registers as uint32_t variables to functions.
>>
>> Fixes PPC64 build with --enable-debug-tcg
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> Reported-by: Stefan Weil <weil@mail.berlios.de>
>> ---
>> target-ppc/cpu.h | 2 ++
>> target-ppc/helper.h | 4 ++--
>> target-ppc/op_helper.c | 10 ++++++++++
>> target-ppc/translate.c | 12 ++++++------
>> 4 files changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index d15bba1..60a8b68 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -733,6 +733,8 @@ void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs);
>> void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value);
>> #endif /* !defined(CONFIG_USER_ONLY) */
>> void ppc_store_msr (CPUPPCState *env, target_ulong value);
>> +void helper_store_dcr (uint32_t dcrn, uint32_t val);
>> +uint32_t helper_load_dcr (uint32_t dcrn);
>>
>> void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
>>
>> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
>> index 40d4ced..86f0af7 100644
>> --- a/target-ppc/helper.h
>> +++ b/target-ppc/helper.h
>> @@ -359,8 +359,8 @@ DEF_HELPER_2(divo, tl, tl, tl)
>> DEF_HELPER_2(divs, tl, tl, tl)
>> DEF_HELPER_2(divso, tl, tl, tl)
>>
>> -DEF_HELPER_1(load_dcr, i32, i32);
>> -DEF_HELPER_2(store_dcr, void, i32, i32)
>> +DEF_HELPER_1(load_dcr_tl, tl, tl);
>> +DEF_HELPER_2(store_dcr_tl, void, tl, tl)
>>
>> DEF_HELPER_1(load_dump_spr, void, i32)
>> DEF_HELPER_1(store_dump_spr, void, i32)
>> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
>> index cea27f2..6c375d3 100644
>> --- a/target-ppc/op_helper.c
>> +++ b/target-ppc/op_helper.c
>> @@ -1844,6 +1844,11 @@ uint32_t helper_load_dcr (uint32_t dcrn)
>> return val;
>> }
>>
>> +target_ulong helper_load_dcr_tl (target_ulong dcrn)
>> +{
>> + return (uint32_t)helper_load_dcr((uint32_t)dcrn);
>> +}
>> +
>> void helper_store_dcr (uint32_t dcrn, uint32_t val)
>> {
>> if (unlikely(env->dcr_env == NULL)) {
>> @@ -1857,6 +1862,11 @@ void helper_store_dcr (uint32_t dcrn, uint32_t val)
>> }
>> }
>>
>> +void helper_store_dcr_tl (target_ulong dcrn, target_ulong val)
>> +{
>> + helper_store_dcr((uint32_t)dcrn, (uint32_t)val);
>> +}
>> +
>
> I do wonder why we need to keep the old helper_load_dcr() and
> helper_store_dcr() instead of modifying them directly. They doesn't seems
> to be used elsewhere.
Last time I checked they were used in hw/*ppc*.c. Maybe mangled through funny preprocessor or function callback logic. But maybe I'm wrong :).
Alex
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations
2010-01-14 15:19 ` Alexander Graf
@ 2010-01-14 17:02 ` Aurelien Jarno
2010-01-14 17:04 ` Alexander Graf
0 siblings, 1 reply; 8+ messages in thread
From: Aurelien Jarno @ 2010-01-14 17:02 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-devel
On Thu, Jan 14, 2010 at 04:19:31PM +0100, Alexander Graf wrote:
>
> On 14.01.2010, at 16:13, Aurelien Jarno wrote:
>
> > On Fri, Jan 01, 2010 at 04:41:06PM +0100, Alexander Graf wrote:
> >> The recent transition to always have the DCR helper functions take 32 bit
> >> values broke the PPC64 target, as tlong became 64 bits there.
> >>
> >> This patch moves all translate.c callers to a _tl function that simply
> >> calls the uint32_t functions. That way we don't need to mess with TCG
> >> trying to pass registers as uint32_t variables to functions.
> >>
> >> Fixes PPC64 build with --enable-debug-tcg
> >>
> >> Signed-off-by: Alexander Graf <agraf@suse.de>
> >> Reported-by: Stefan Weil <weil@mail.berlios.de>
> >> ---
> >> target-ppc/cpu.h | 2 ++
> >> target-ppc/helper.h | 4 ++--
> >> target-ppc/op_helper.c | 10 ++++++++++
> >> target-ppc/translate.c | 12 ++++++------
> >> 4 files changed, 20 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> >> index d15bba1..60a8b68 100644
> >> --- a/target-ppc/cpu.h
> >> +++ b/target-ppc/cpu.h
> >> @@ -733,6 +733,8 @@ void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs);
> >> void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value);
> >> #endif /* !defined(CONFIG_USER_ONLY) */
> >> void ppc_store_msr (CPUPPCState *env, target_ulong value);
> >> +void helper_store_dcr (uint32_t dcrn, uint32_t val);
> >> +uint32_t helper_load_dcr (uint32_t dcrn);
> >>
> >> void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
> >>
> >> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> >> index 40d4ced..86f0af7 100644
> >> --- a/target-ppc/helper.h
> >> +++ b/target-ppc/helper.h
> >> @@ -359,8 +359,8 @@ DEF_HELPER_2(divo, tl, tl, tl)
> >> DEF_HELPER_2(divs, tl, tl, tl)
> >> DEF_HELPER_2(divso, tl, tl, tl)
> >>
> >> -DEF_HELPER_1(load_dcr, i32, i32);
> >> -DEF_HELPER_2(store_dcr, void, i32, i32)
> >> +DEF_HELPER_1(load_dcr_tl, tl, tl);
> >> +DEF_HELPER_2(store_dcr_tl, void, tl, tl)
> >>
> >> DEF_HELPER_1(load_dump_spr, void, i32)
> >> DEF_HELPER_1(store_dump_spr, void, i32)
> >> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
> >> index cea27f2..6c375d3 100644
> >> --- a/target-ppc/op_helper.c
> >> +++ b/target-ppc/op_helper.c
> >> @@ -1844,6 +1844,11 @@ uint32_t helper_load_dcr (uint32_t dcrn)
> >> return val;
> >> }
> >>
> >> +target_ulong helper_load_dcr_tl (target_ulong dcrn)
> >> +{
> >> + return (uint32_t)helper_load_dcr((uint32_t)dcrn);
> >> +}
> >> +
> >> void helper_store_dcr (uint32_t dcrn, uint32_t val)
> >> {
> >> if (unlikely(env->dcr_env == NULL)) {
> >> @@ -1857,6 +1862,11 @@ void helper_store_dcr (uint32_t dcrn, uint32_t val)
> >> }
> >> }
> >>
> >> +void helper_store_dcr_tl (target_ulong dcrn, target_ulong val)
> >> +{
> >> + helper_store_dcr((uint32_t)dcrn, (uint32_t)val);
> >> +}
> >> +
> >
> > I do wonder why we need to keep the old helper_load_dcr() and
> > helper_store_dcr() instead of modifying them directly. They doesn't seems
> > to be used elsewhere.
>
> Last time I checked they were used in hw/*ppc*.c. Maybe mangled through funny preprocessor or function callback logic. But maybe I'm wrong :).
>
I am not able to find them. I have tried to remove helper_load_dcr() and
helper_store_dcr() from target-ppc/cpu.h and the code still compiles.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations
2010-01-14 17:02 ` Aurelien Jarno
@ 2010-01-14 17:04 ` Alexander Graf
2010-02-06 16:15 ` Aurelien Jarno
0 siblings, 1 reply; 8+ messages in thread
From: Alexander Graf @ 2010-01-14 17:04 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
On 14.01.2010, at 18:02, Aurelien Jarno wrote:
> On Thu, Jan 14, 2010 at 04:19:31PM +0100, Alexander Graf wrote:
>>
>> On 14.01.2010, at 16:13, Aurelien Jarno wrote:
>>
>>> On Fri, Jan 01, 2010 at 04:41:06PM +0100, Alexander Graf wrote:
>>>> The recent transition to always have the DCR helper functions take 32 bit
>>>> values broke the PPC64 target, as tlong became 64 bits there.
>>>>
>>>> This patch moves all translate.c callers to a _tl function that simply
>>>> calls the uint32_t functions. That way we don't need to mess with TCG
>>>> trying to pass registers as uint32_t variables to functions.
>>>>
>>>> Fixes PPC64 build with --enable-debug-tcg
>>>>
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> Reported-by: Stefan Weil <weil@mail.berlios.de>
>>>> ---
>>>> target-ppc/cpu.h | 2 ++
>>>> target-ppc/helper.h | 4 ++--
>>>> target-ppc/op_helper.c | 10 ++++++++++
>>>> target-ppc/translate.c | 12 ++++++------
>>>> 4 files changed, 20 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>>> index d15bba1..60a8b68 100644
>>>> --- a/target-ppc/cpu.h
>>>> +++ b/target-ppc/cpu.h
>>>> @@ -733,6 +733,8 @@ void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs);
>>>> void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value);
>>>> #endif /* !defined(CONFIG_USER_ONLY) */
>>>> void ppc_store_msr (CPUPPCState *env, target_ulong value);
>>>> +void helper_store_dcr (uint32_t dcrn, uint32_t val);
>>>> +uint32_t helper_load_dcr (uint32_t dcrn);
>>>>
>>>> void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
>>>>
>>>> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
>>>> index 40d4ced..86f0af7 100644
>>>> --- a/target-ppc/helper.h
>>>> +++ b/target-ppc/helper.h
>>>> @@ -359,8 +359,8 @@ DEF_HELPER_2(divo, tl, tl, tl)
>>>> DEF_HELPER_2(divs, tl, tl, tl)
>>>> DEF_HELPER_2(divso, tl, tl, tl)
>>>>
>>>> -DEF_HELPER_1(load_dcr, i32, i32);
>>>> -DEF_HELPER_2(store_dcr, void, i32, i32)
>>>> +DEF_HELPER_1(load_dcr_tl, tl, tl);
>>>> +DEF_HELPER_2(store_dcr_tl, void, tl, tl)
>>>>
>>>> DEF_HELPER_1(load_dump_spr, void, i32)
>>>> DEF_HELPER_1(store_dump_spr, void, i32)
>>>> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
>>>> index cea27f2..6c375d3 100644
>>>> --- a/target-ppc/op_helper.c
>>>> +++ b/target-ppc/op_helper.c
>>>> @@ -1844,6 +1844,11 @@ uint32_t helper_load_dcr (uint32_t dcrn)
>>>> return val;
>>>> }
>>>>
>>>> +target_ulong helper_load_dcr_tl (target_ulong dcrn)
>>>> +{
>>>> + return (uint32_t)helper_load_dcr((uint32_t)dcrn);
>>>> +}
>>>> +
>>>> void helper_store_dcr (uint32_t dcrn, uint32_t val)
>>>> {
>>>> if (unlikely(env->dcr_env == NULL)) {
>>>> @@ -1857,6 +1862,11 @@ void helper_store_dcr (uint32_t dcrn, uint32_t val)
>>>> }
>>>> }
>>>>
>>>> +void helper_store_dcr_tl (target_ulong dcrn, target_ulong val)
>>>> +{
>>>> + helper_store_dcr((uint32_t)dcrn, (uint32_t)val);
>>>> +}
>>>> +
>>>
>>> I do wonder why we need to keep the old helper_load_dcr() and
>>> helper_store_dcr() instead of modifying them directly. They doesn't seems
>>> to be used elsewhere.
>>
>> Last time I checked they were used in hw/*ppc*.c. Maybe mangled through funny preprocessor or function callback logic. But maybe I'm wrong :).
>>
>
> I am not able to find them. I have tried to remove helper_load_dcr() and
> helper_store_dcr() from target-ppc/cpu.h and the code still compiles.
Hm, right. The only references are to ppc_dcr_read and ppc_dcr_write. I guess the other helpers are superfluous then.
Alex
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations
2010-01-14 17:04 ` Alexander Graf
@ 2010-02-06 16:15 ` Aurelien Jarno
2010-02-06 17:16 ` Alexander Graf
0 siblings, 1 reply; 8+ messages in thread
From: Aurelien Jarno @ 2010-02-06 16:15 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-devel
On Thu, Jan 14, 2010 at 06:04:25PM +0100, Alexander Graf wrote:
>
> On 14.01.2010, at 18:02, Aurelien Jarno wrote:
>
> > On Thu, Jan 14, 2010 at 04:19:31PM +0100, Alexander Graf wrote:
> >>
> >> On 14.01.2010, at 16:13, Aurelien Jarno wrote:
> >>
> >>> On Fri, Jan 01, 2010 at 04:41:06PM +0100, Alexander Graf wrote:
> >>>> The recent transition to always have the DCR helper functions take 32 bit
> >>>> values broke the PPC64 target, as tlong became 64 bits there.
> >>>>
> >>>> This patch moves all translate.c callers to a _tl function that simply
> >>>> calls the uint32_t functions. That way we don't need to mess with TCG
> >>>> trying to pass registers as uint32_t variables to functions.
> >>>>
> >>>> Fixes PPC64 build with --enable-debug-tcg
> >>>>
> >>>> Signed-off-by: Alexander Graf <agraf@suse.de>
> >>>> Reported-by: Stefan Weil <weil@mail.berlios.de>
> >>>> ---
> >>>> target-ppc/cpu.h | 2 ++
> >>>> target-ppc/helper.h | 4 ++--
> >>>> target-ppc/op_helper.c | 10 ++++++++++
> >>>> target-ppc/translate.c | 12 ++++++------
> >>>> 4 files changed, 20 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> >>>> index d15bba1..60a8b68 100644
> >>>> --- a/target-ppc/cpu.h
> >>>> +++ b/target-ppc/cpu.h
> >>>> @@ -733,6 +733,8 @@ void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs);
> >>>> void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value);
> >>>> #endif /* !defined(CONFIG_USER_ONLY) */
> >>>> void ppc_store_msr (CPUPPCState *env, target_ulong value);
> >>>> +void helper_store_dcr (uint32_t dcrn, uint32_t val);
> >>>> +uint32_t helper_load_dcr (uint32_t dcrn);
> >>>>
> >>>> void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
> >>>>
> >>>> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> >>>> index 40d4ced..86f0af7 100644
> >>>> --- a/target-ppc/helper.h
> >>>> +++ b/target-ppc/helper.h
> >>>> @@ -359,8 +359,8 @@ DEF_HELPER_2(divo, tl, tl, tl)
> >>>> DEF_HELPER_2(divs, tl, tl, tl)
> >>>> DEF_HELPER_2(divso, tl, tl, tl)
> >>>>
> >>>> -DEF_HELPER_1(load_dcr, i32, i32);
> >>>> -DEF_HELPER_2(store_dcr, void, i32, i32)
> >>>> +DEF_HELPER_1(load_dcr_tl, tl, tl);
> >>>> +DEF_HELPER_2(store_dcr_tl, void, tl, tl)
> >>>>
> >>>> DEF_HELPER_1(load_dump_spr, void, i32)
> >>>> DEF_HELPER_1(store_dump_spr, void, i32)
> >>>> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
> >>>> index cea27f2..6c375d3 100644
> >>>> --- a/target-ppc/op_helper.c
> >>>> +++ b/target-ppc/op_helper.c
> >>>> @@ -1844,6 +1844,11 @@ uint32_t helper_load_dcr (uint32_t dcrn)
> >>>> return val;
> >>>> }
> >>>>
> >>>> +target_ulong helper_load_dcr_tl (target_ulong dcrn)
> >>>> +{
> >>>> + return (uint32_t)helper_load_dcr((uint32_t)dcrn);
> >>>> +}
> >>>> +
> >>>> void helper_store_dcr (uint32_t dcrn, uint32_t val)
> >>>> {
> >>>> if (unlikely(env->dcr_env == NULL)) {
> >>>> @@ -1857,6 +1862,11 @@ void helper_store_dcr (uint32_t dcrn, uint32_t val)
> >>>> }
> >>>> }
> >>>>
> >>>> +void helper_store_dcr_tl (target_ulong dcrn, target_ulong val)
> >>>> +{
> >>>> + helper_store_dcr((uint32_t)dcrn, (uint32_t)val);
> >>>> +}
> >>>> +
> >>>
> >>> I do wonder why we need to keep the old helper_load_dcr() and
> >>> helper_store_dcr() instead of modifying them directly. They doesn't seems
> >>> to be used elsewhere.
> >>
> >> Last time I checked they were used in hw/*ppc*.c. Maybe mangled through funny preprocessor or function callback logic. But maybe I'm wrong :).
> >>
> >
> > I am not able to find them. I have tried to remove helper_load_dcr() and
> > helper_store_dcr() from target-ppc/cpu.h and the code still compiles.
>
> Hm, right. The only references are to ppc_dcr_read and ppc_dcr_write. I guess the other helpers are superfluous then.
>
I have applied a patch to fix the problem.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations
2010-02-06 16:15 ` Aurelien Jarno
@ 2010-02-06 17:16 ` Alexander Graf
0 siblings, 0 replies; 8+ messages in thread
From: Alexander Graf @ 2010-02-06 17:16 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel@nongnu.org
Am 06.02.2010 um 17:15 schrieb Aurelien Jarno <aurelien@aurel32.net>:
> On Thu, Jan 14, 2010 at 06:04:25PM +0100, Alexander Graf wrote:
>>
>> On 14.01.2010, at 18:02, Aurelien Jarno wrote:
>>
>>> On Thu, Jan 14, 2010 at 04:19:31PM +0100, Alexander Graf wrote:
>>>>
>>>> On 14.01.2010, at 16:13, Aurelien Jarno wrote:
>>>>
>>>>> On Fri, Jan 01, 2010 at 04:41:06PM +0100, Alexander Graf wrote:
>>>>>> The recent transition to always have the DCR helper functions
>>>>>> take 32 bit
>>>>>> values broke the PPC64 target, as tlong became 64 bits there.
>>>>>>
>>>>>> This patch moves all translate.c callers to a _tl function that
>>>>>> simply
>>>>>> calls the uint32_t functions. That way we don't need to mess
>>>>>> with TCG
>>>>>> trying to pass registers as uint32_t variables to functions.
>>>>>>
>>>>>> Fixes PPC64 build with --enable-debug-tcg
>>>>>>
>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>> Reported-by: Stefan Weil <weil@mail.berlios.de>
>>>>>> ---
>>>>>> target-ppc/cpu.h | 2 ++
>>>>>> target-ppc/helper.h | 4 ++--
>>>>>> target-ppc/op_helper.c | 10 ++++++++++
>>>>>> target-ppc/translate.c | 12 ++++++------
>>>>>> 4 files changed, 20 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>>>>> index d15bba1..60a8b68 100644
>>>>>> --- a/target-ppc/cpu.h
>>>>>> +++ b/target-ppc/cpu.h
>>>>>> @@ -733,6 +733,8 @@ void ppc_store_slb (CPUPPCState *env,
>>>>>> target_ulong rb, target_ulong rs);
>>>>>> void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong
>>>>>> value);
>>>>>> #endif /* !defined(CONFIG_USER_ONLY) */
>>>>>> void ppc_store_msr (CPUPPCState *env, target_ulong value);
>>>>>> +void helper_store_dcr (uint32_t dcrn, uint32_t val);
>>>>>> +uint32_t helper_load_dcr (uint32_t dcrn);
>>>>>>
>>>>>> void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const
>>>>>> char *fmt, ...));
>>>>>>
>>>>>> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
>>>>>> index 40d4ced..86f0af7 100644
>>>>>> --- a/target-ppc/helper.h
>>>>>> +++ b/target-ppc/helper.h
>>>>>> @@ -359,8 +359,8 @@ DEF_HELPER_2(divo, tl, tl, tl)
>>>>>> DEF_HELPER_2(divs, tl, tl, tl)
>>>>>> DEF_HELPER_2(divso, tl, tl, tl)
>>>>>>
>>>>>> -DEF_HELPER_1(load_dcr, i32, i32);
>>>>>> -DEF_HELPER_2(store_dcr, void, i32, i32)
>>>>>> +DEF_HELPER_1(load_dcr_tl, tl, tl);
>>>>>> +DEF_HELPER_2(store_dcr_tl, void, tl, tl)
>>>>>>
>>>>>> DEF_HELPER_1(load_dump_spr, void, i32)
>>>>>> DEF_HELPER_1(store_dump_spr, void, i32)
>>>>>> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
>>>>>> index cea27f2..6c375d3 100644
>>>>>> --- a/target-ppc/op_helper.c
>>>>>> +++ b/target-ppc/op_helper.c
>>>>>> @@ -1844,6 +1844,11 @@ uint32_t helper_load_dcr (uint32_t dcrn)
>>>>>> return val;
>>>>>> }
>>>>>>
>>>>>> +target_ulong helper_load_dcr_tl (target_ulong dcrn)
>>>>>> +{
>>>>>> + return (uint32_t)helper_load_dcr((uint32_t)dcrn);
>>>>>> +}
>>>>>> +
>>>>>> void helper_store_dcr (uint32_t dcrn, uint32_t val)
>>>>>> {
>>>>>> if (unlikely(env->dcr_env == NULL)) {
>>>>>> @@ -1857,6 +1862,11 @@ void helper_store_dcr (uint32_t dcrn,
>>>>>> uint32_t val)
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> +void helper_store_dcr_tl (target_ulong dcrn, target_ulong val)
>>>>>> +{
>>>>>> + helper_store_dcr((uint32_t)dcrn, (uint32_t)val);
>>>>>> +}
>>>>>> +
>>>>>
>>>>> I do wonder why we need to keep the old helper_load_dcr() and
>>>>> helper_store_dcr() instead of modifying them directly. They
>>>>> doesn't seems
>>>>> to be used elsewhere.
>>>>
>>>> Last time I checked they were used in hw/*ppc*.c. Maybe mangled
>>>> through funny preprocessor or function callback logic. But maybe
>>>> I'm wrong :).
>>>>
>>>
>>> I am not able to find them. I have tried to remove helper_load_dcr
>>> () and
>>> helper_store_dcr() from target-ppc/cpu.h and the code still
>>> compiles.
>>
>> Hm, right. The only references are to ppc_dcr_read and
>> ppc_dcr_write. I guess the other helpers are superfluous then.
>>
>
> I have applied a patch to fix the problem.
Thanks a lot! Sorry I didn't get to it :(.
Alex
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-02-06 17:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-01 15:41 [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations Alexander Graf
2010-01-03 10:25 ` Stefan Weil
2010-01-14 15:13 ` Aurelien Jarno
2010-01-14 15:19 ` Alexander Graf
2010-01-14 17:02 ` Aurelien Jarno
2010-01-14 17:04 ` Alexander Graf
2010-02-06 16:15 ` Aurelien Jarno
2010-02-06 17:16 ` Alexander Graf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).