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 <ma...@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.al...@imgtec.com>
