diff mbox

MIPS: Correct MIPS16/microMIPS branch size calculation

Message ID alpine.DEB.1.10.1206080050430.23962@tp.orcam.me.uk
State New
Headers show

Commit Message

Maciej W. Rozycki June 8, 2012, 1:06 a.m. UTC 
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

Comments

Richard Henderson June 12, 2012, 3:01 p.m. UTC | #1 
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. UTC | #2 
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
diff mbox

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;
     }