* [Qemu-devel] [PATCH] target-mips: gdbstub: Clean up FPU register handling
@ 2014-11-20 11:08 Maciej W. Rozycki
2014-12-05 15:00 ` Leon Alrae
2014-12-05 18:46 ` [Qemu-devel] [PATCH v2] " Maciej W. Rozycki
0 siblings, 2 replies; 4+ messages in thread
From: Maciej W. Rozycki @ 2014-11-20 11:08 UTC (permalink / raw)
To: qemu-devel; +Cc: Leon Alrae, Aurelien Jarno
Rewrite the FPU register access parts of `mips_cpu_gdb_read_register'
and `mips_cpu_gdb_write_register' for consistency between each other.
Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
---
Hi,
This is the FPU register handling cleanup previously promised.
It was regression-tested by running the GDB test suite for the
mips-sde-elf target and a -EB (o32) multilib, on a 24Kf processor.
QEMU in the system emulation mode was used as the remote stub for GDB.
Apart from overall register accesses the test suite includes specific
tests that make manual function calls with FP arguments and/or FP
results and these rely on FP registers to be passed around correctly.
I also visually inspected test logs and verified that the values
reported for CP1.FIR and CP1.FCSR did not change.
Please apply.
Maciej
qemu-mips-gdbstub-cleanup.diff
Index: qemu-git-trunk/target-mips/gdbstub.c
===================================================================
--- qemu-git-trunk.orig/target-mips/gdbstub.c 2014-11-20 10:44:24.058944521 +0000
+++ qemu-git-trunk/target-mips/gdbstub.c 2014-11-20 10:44:28.058940153 +0000
@@ -29,8 +29,13 @@ int mips_cpu_gdb_read_register(CPUState
if (n < 32) {
return gdb_get_regl(mem_buf, env->active_tc.gpr[n]);
}
- if (env->CP0_Config1 & (1 << CP0C1_FP)) {
- if (n >= 38 && n < 70) {
+ if (env->CP0_Config1 & (1 << CP0C1_FP) && n >= 38 && n < 72) {
+ switch (n) {
+ case 70:
+ return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr31);
+ case 71:
+ return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr0);
+ default:
if (env->CP0_Status & (1 << CP0St_FR)) {
return gdb_get_regl(mem_buf,
env->active_fpu.fpr[n - 38].d);
@@ -39,12 +44,6 @@ int mips_cpu_gdb_read_register(CPUState
env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX]);
}
}
- switch (n) {
- case 70:
- return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr31);
- case 71:
- return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr0);
- }
}
switch (n) {
case 32:
@@ -64,8 +63,10 @@ int mips_cpu_gdb_read_register(CPUState
return gdb_get_regl(mem_buf, 0); /* fp */
case 89:
return gdb_get_regl(mem_buf, (int32_t)env->CP0_PRid);
- }
- if (n >= 73 && n <= 88) {
+ default:
+ if (n > 89) {
+ return 0;
+ }
/* 16 embedded regs. */
return gdb_get_regl(mem_buf, 0);
}
@@ -89,15 +90,7 @@ int mips_cpu_gdb_write_register(CPUState
env->active_tc.gpr[n] = tmp;
return sizeof(target_ulong);
}
- if (env->CP0_Config1 & (1 << CP0C1_FP)
- && n >= 38 && n < 72) {
- if (n < 70) {
- if (env->CP0_Status & (1 << CP0St_FR)) {
- env->active_fpu.fpr[n - 38].d = tmp;
- } else {
- env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX] = tmp;
- }
- }
+ if (env->CP0_Config1 & (1 << CP0C1_FP) && n >= 38 && n < 72) {
switch (n) {
case 70:
env->active_fpu.fcr31 = tmp & 0xFF83FFFF;
@@ -107,6 +100,12 @@ int mips_cpu_gdb_write_register(CPUState
case 71:
/* FIR is read-only. Ignore writes. */
break;
+ default:
+ if (env->CP0_Status & (1 << CP0St_FR))
+ env->active_fpu.fpr[n - 38].d = tmp;
+ else
+ env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX] = tmp;
+ break;
}
return sizeof(target_ulong);
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] target-mips: gdbstub: Clean up FPU register handling
2014-11-20 11:08 [Qemu-devel] [PATCH] target-mips: gdbstub: Clean up FPU register handling Maciej W. Rozycki
@ 2014-12-05 15:00 ` Leon Alrae
2014-12-05 18:56 ` Maciej W. Rozycki
2014-12-05 18:46 ` [Qemu-devel] [PATCH v2] " Maciej W. Rozycki
1 sibling, 1 reply; 4+ messages in thread
From: Leon Alrae @ 2014-12-05 15:00 UTC (permalink / raw)
To: Maciej W. Rozycki, qemu-devel; +Cc: Aurelien Jarno
On 20/11/2014 11:08, Maciej W. Rozycki wrote:
> Rewrite the FPU register access parts of `mips_cpu_gdb_read_register'
> and `mips_cpu_gdb_write_register' for consistency between each other.
>
> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> ---
> Hi,
>
> This is the FPU register handling cleanup previously promised.
>
> It was regression-tested by running the GDB test suite for the
> mips-sde-elf target and a -EB (o32) multilib, on a 24Kf processor.
> QEMU in the system emulation mode was used as the remote stub for GDB.
>
> Apart from overall register accesses the test suite includes specific
> tests that make manual function calls with FP arguments and/or FP
> results and these rely on FP registers to be passed around correctly.
> I also visually inspected test logs and verified that the values
> reported for CP1.FIR and CP1.FCSR did not change.
>
> Please apply.
>
> Maciej
>
> qemu-mips-gdbstub-cleanup.diff
> Index: qemu-git-trunk/target-mips/gdbstub.c
> ===================================================================
> --- qemu-git-trunk.orig/target-mips/gdbstub.c 2014-11-20 10:44:24.058944521 +0000
> +++ qemu-git-trunk/target-mips/gdbstub.c 2014-11-20 10:44:28.058940153 +0000
> @@ -29,8 +29,13 @@ int mips_cpu_gdb_read_register(CPUState
> if (n < 32) {
> return gdb_get_regl(mem_buf, env->active_tc.gpr[n]);
> }
> - if (env->CP0_Config1 & (1 << CP0C1_FP)) {
> - if (n >= 38 && n < 70) {
> + if (env->CP0_Config1 & (1 << CP0C1_FP) && n >= 38 && n < 72) {
> + switch (n) {
> + case 70:
> + return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr31);
> + case 71:
> + return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr0);
> + default:
> if (env->CP0_Status & (1 << CP0St_FR)) {
> return gdb_get_regl(mem_buf,
> env->active_fpu.fpr[n - 38].d);
> @@ -39,12 +44,6 @@ int mips_cpu_gdb_read_register(CPUState
> env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX]);
> }
> }
> - switch (n) {
> - case 70:
> - return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr31);
> - case 71:
> - return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr0);
> - }
> }
> switch (n) {
> case 32:
> @@ -64,8 +63,10 @@ int mips_cpu_gdb_read_register(CPUState
> return gdb_get_regl(mem_buf, 0); /* fp */
> case 89:
> return gdb_get_regl(mem_buf, (int32_t)env->CP0_PRid);
> - }
> - if (n >= 73 && n <= 88) {
> + default:
> + if (n > 89) {
> + return 0;
> + }
> /* 16 embedded regs. */
> return gdb_get_regl(mem_buf, 0);
> }
> @@ -89,15 +90,7 @@ int mips_cpu_gdb_write_register(CPUState
> env->active_tc.gpr[n] = tmp;
> return sizeof(target_ulong);
> }
> - if (env->CP0_Config1 & (1 << CP0C1_FP)
> - && n >= 38 && n < 72) {
> - if (n < 70) {
> - if (env->CP0_Status & (1 << CP0St_FR)) {
> - env->active_fpu.fpr[n - 38].d = tmp;
> - } else {
> - env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX] = tmp;
> - }
> - }
> + if (env->CP0_Config1 & (1 << CP0C1_FP) && n >= 38 && n < 72) {
> switch (n) {
> case 70:
> env->active_fpu.fcr31 = tmp & 0xFF83FFFF;
> @@ -107,6 +100,12 @@ int mips_cpu_gdb_write_register(CPUState
> case 71:
> /* FIR is read-only. Ignore writes. */
> break;
> + default:
> + if (env->CP0_Status & (1 << CP0St_FR))
> + env->active_fpu.fpr[n - 38].d = tmp;
> + else
> + env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX] = tmp;
Braces are missing here.
> + break;
> }
> return sizeof(target_ulong);
> }
>
Otherwise,
Reviewed-by: Leon Alrae <leon.alrae@imgtec.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH v2] target-mips: gdbstub: Clean up FPU register handling
2014-11-20 11:08 [Qemu-devel] [PATCH] target-mips: gdbstub: Clean up FPU register handling Maciej W. Rozycki
2014-12-05 15:00 ` Leon Alrae
@ 2014-12-05 18:46 ` Maciej W. Rozycki
1 sibling, 0 replies; 4+ messages in thread
From: Maciej W. Rozycki @ 2014-12-05 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Leon Alrae, Aurelien Jarno
Rewrite the FPU register access parts of `mips_cpu_gdb_read_register'
and `mips_cpu_gdb_write_register' for consistency between each other.
Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
---
Changes from v1:
- missing braces added for blocks required with conditional statements.
qemu-mips-gdbstub-cleanup.diff
Index: qemu-git-trunk/target-mips/gdbstub.c
===================================================================
--- qemu-git-trunk.orig/target-mips/gdbstub.c 2014-12-04 11:55:18.000000000 +0000
+++ qemu-git-trunk/target-mips/gdbstub.c 2014-12-05 18:39:46.348928321 +0000
@@ -29,8 +29,13 @@ int mips_cpu_gdb_read_register(CPUState
if (n < 32) {
return gdb_get_regl(mem_buf, env->active_tc.gpr[n]);
}
- if (env->CP0_Config1 & (1 << CP0C1_FP)) {
- if (n >= 38 && n < 70) {
+ if (env->CP0_Config1 & (1 << CP0C1_FP) && n >= 38 && n < 72) {
+ switch (n) {
+ case 70:
+ return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr31);
+ case 71:
+ return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr0);
+ default:
if (env->CP0_Status & (1 << CP0St_FR)) {
return gdb_get_regl(mem_buf,
env->active_fpu.fpr[n - 38].d);
@@ -39,12 +44,6 @@ int mips_cpu_gdb_read_register(CPUState
env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX]);
}
}
- switch (n) {
- case 70:
- return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr31);
- case 71:
- return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr0);
- }
}
switch (n) {
case 32:
@@ -64,8 +63,10 @@ int mips_cpu_gdb_read_register(CPUState
return gdb_get_regl(mem_buf, 0); /* fp */
case 89:
return gdb_get_regl(mem_buf, (int32_t)env->CP0_PRid);
- }
- if (n >= 73 && n <= 88) {
+ default:
+ if (n > 89) {
+ return 0;
+ }
/* 16 embedded regs. */
return gdb_get_regl(mem_buf, 0);
}
@@ -89,15 +90,7 @@ int mips_cpu_gdb_write_register(CPUState
env->active_tc.gpr[n] = tmp;
return sizeof(target_ulong);
}
- if (env->CP0_Config1 & (1 << CP0C1_FP)
- && n >= 38 && n < 72) {
- if (n < 70) {
- if (env->CP0_Status & (1 << CP0St_FR)) {
- env->active_fpu.fpr[n - 38].d = tmp;
- } else {
- env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX] = tmp;
- }
- }
+ if (env->CP0_Config1 & (1 << CP0C1_FP) && n >= 38 && n < 72) {
switch (n) {
case 70:
env->active_fpu.fcr31 = tmp & 0xFF83FFFF;
@@ -107,6 +100,13 @@ int mips_cpu_gdb_write_register(CPUState
case 71:
/* FIR is read-only. Ignore writes. */
break;
+ default:
+ if (env->CP0_Status & (1 << CP0St_FR)) {
+ env->active_fpu.fpr[n - 38].d = tmp;
+ } else {
+ env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX] = tmp;
+ }
+ break;
}
return sizeof(target_ulong);
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] target-mips: gdbstub: Clean up FPU register handling
2014-12-05 15:00 ` Leon Alrae
@ 2014-12-05 18:56 ` Maciej W. Rozycki
0 siblings, 0 replies; 4+ messages in thread
From: Maciej W. Rozycki @ 2014-12-05 18:56 UTC (permalink / raw)
To: Leon Alrae; +Cc: qemu-devel, Aurelien Jarno
On Fri, 5 Dec 2014, Leon Alrae wrote:
> > qemu-mips-gdbstub-cleanup.diff
> > Index: qemu-git-trunk/target-mips/gdbstub.c
> > ===================================================================
> > --- qemu-git-trunk.orig/target-mips/gdbstub.c 2014-11-20 10:44:24.058944521 +0000
> > +++ qemu-git-trunk/target-mips/gdbstub.c 2014-11-20 10:44:28.058940153 +0000
[...]
> > @@ -107,6 +100,12 @@ int mips_cpu_gdb_write_register(CPUState
> > case 71:
> > /* FIR is read-only. Ignore writes. */
> > break;
> > + default:
> > + if (env->CP0_Status & (1 << CP0St_FR))
> > + env->active_fpu.fpr[n - 38].d = tmp;
> > + else
> > + env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX] = tmp;
>
> Braces are missing here.
Fixed in v2, thanks for your review.
Maciej
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-12-05 18:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-20 11:08 [Qemu-devel] [PATCH] target-mips: gdbstub: Clean up FPU register handling Maciej W. Rozycki
2014-12-05 15:00 ` Leon Alrae
2014-12-05 18:56 ` Maciej W. Rozycki
2014-12-05 18:46 ` [Qemu-devel] [PATCH v2] " Maciej W. Rozycki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.