Patchwork MIPS: Correct MIPS16/microMIPS branch size calculation

login
register
mail settings
Submitter Maciej W. Rozycki
Date June 8, 2012, 1:06 a.m.
Message ID <alpine.DEB.1.10.1206080050430.23962@tp.orcam.me.uk>
Download mbox | patch
Permalink /patch/163707/
State New
Headers show

Comments

Maciej W. Rozycki - June 8, 2012, 1:06 a.m.
From: Nathan Froyd <froydnj@codesourcery.com>

 Nathan's original terse comment:

"Use MIPS_HFLAG_B16 to determine the address of a jump instruction when we 
need to restart a delay slot instruction."

and was not accompanied by a test case nor I have one offhand.

 However this change appears obviously correct to me, and the same 
calculation is already used in exception_resume_pc applied to ordinary, 
Debug and NMI exceptions.  This code on the other hand applies to reset 
exceptions and instruction restarts in the context of I/O.

Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
---

 Sent on behalf of Nathan, who's since left the company.  Please apply.

  Maciej

qemu-mips-b16.diff
Richard Henderson - June 12, 2012, 3:01 p.m.
On 2012-06-07 18:06, Maciej W. Rozycki wrote:
> "Use MIPS_HFLAG_B16 to determine the address of a jump instruction when we 
> need to restart a delay slot instruction."
> 
> and was not accompanied by a test case nor I have one offhand.
> 
>  However this change appears obviously correct to me, and the same 
> calculation is already used in exception_resume_pc applied to ordinary, 
> Debug and NMI exceptions.  This code on the other hand applies to reset 
> exceptions and instruction restarts in the context of I/O.

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~
Peter Maydell - June 12, 2012, 3:11 p.m.
On 8 June 2012 02:06, Maciej W. Rozycki <macro@codesourcery.com> wrote:
> From: Nathan Froyd <froydnj@codesourcery.com>
>
>  Nathan's original terse comment:
>
> "Use MIPS_HFLAG_B16 to determine the address of a jump instruction when we
> need to restart a delay slot instruction."
>
> and was not accompanied by a test case nor I have one offhand.
>
>  However this change appears obviously correct to me, and the same
> calculation is already used in exception_resume_pc applied to ordinary,
> Debug and NMI exceptions.  This code on the other hand applies to reset
> exceptions and instruction restarts in the context of I/O.

I would prefer the commit messages to be standalone justifications
for the changes. Part of cleaning up somebody else's patches for
submission to the list should include writing good commit messages
if the original patches were overly brief in that area.

-- PMM

Patch

Index: qemu-git-trunk/exec.c
===================================================================
--- qemu-git-trunk.orig/exec.c	2012-06-04 05:34:18.655419589 +0100
+++ qemu-git-trunk/exec.c	2012-06-04 05:42:53.295516541 +0100
@@ -4235,7 +4235,7 @@  void cpu_io_recompile(CPUArchState *env,
        branch.  */
 #if defined(TARGET_MIPS)
     if ((env->hflags & MIPS_HFLAG_BMASK) != 0 && n > 1) {
-        env->active_tc.PC -= 4;
+        env->active_tc.PC -= (env->hflags & MIPS_HFLAG_B16 ? 2 : 4);
         env->icount_decr.u16.low++;
         env->hflags &= ~MIPS_HFLAG_BMASK;
     }
Index: qemu-git-trunk/target-mips/translate.c
===================================================================
--- qemu-git-trunk.orig/target-mips/translate.c	2012-06-04 05:42:49.475411277 +0100
+++ qemu-git-trunk/target-mips/translate.c	2012-06-04 05:42:53.295516541 +0100
@@ -12796,7 +12796,8 @@  void cpu_state_reset(CPUMIPSState *env)
     if (env->hflags & MIPS_HFLAG_BMASK) {
         /* If the exception was raised from a delay slot,
            come back to the jump.  */
-        env->CP0_ErrorEPC = env->active_tc.PC - 4;
+        env->CP0_ErrorEPC = (env->active_tc.PC
+                             - (env->hflags & MIPS_HFLAG_B16 ? 2 : 4));
     } else {
         env->CP0_ErrorEPC = env->active_tc.PC;
     }