Patchwork target-mips: 64-bit FPU for user-mode emulation.

login
register
mail settings
Submitter Thomas Schwinge
Date June 9, 2013, 5:46 p.m.
Message ID <87vc5nxjm2.fsf@kepler.schwinge.homeip.net>
Download mbox | patch
Permalink /patch/250161/
State New
Headers show

Comments

Thomas Schwinge - June 9, 2013, 5:46 p.m.
Switch to 64-bit FPU only for n32 and n64 ABIs, but not o32.  Fixup for
commit 68473f15d4c9948986618f63828825beafcaf1cf.

Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>

---

Hi!

On Sun, 10 Feb 2013 10:30:46 -0800, Richard Henderson <rth@twiddle.net> wrote:
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 3b77b53..b3b8dc6 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -15972,6 +15972,14 @@ void cpu_state_reset(CPUMIPSState *env)
>  
>  #if defined(CONFIG_USER_ONLY)
>      env->CP0_Status = (MIPS_HFLAG_UM << CP0St_KSU);
> +# ifdef TARGET_MIPS64
> +    /* Enable 64-bit register mode.  */
> +    env->CP0_Status |= (1 << CP0St_PX);
> +# endif
> +# ifdef TARGET_ABI_MIPSN64
> +    /* Enable 64-bit address mode.  */
> +    env->CP0_Status |= (1 << CP0St_UX);
> +# endif
>      /* Enable access to the CPUNum, SYNCI_Step, CC, and CCRes RDHWR
>         hardware registers.  */
>      env->CP0_HWREna |= 0x0000000F;
> @@ -15981,6 +15989,11 @@ void cpu_state_reset(CPUMIPSState *env)
>      if (env->CP0_Config3 & (1 << CP0C3_DSPP)) {
>          env->CP0_Status |= (1 << CP0St_MX);
>      }
> +    /* Enable 64-bit FPU if the target cpu supports it.  */
> +    env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
> +    if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
> +        env->CP0_Status |= (1 << CP0St_FR);
> +    }
>  #else
>      if (env->hflags & MIPS_HFLAG_BMASK) {
>          /* If the exception was raised from a delay slot,

In my reading of the relevant documents, the latter change is not correct
for o32, and empirically has "interesting" effects on the glibc math
testsuite, for example.  Keeping the FR register unset for o32 I'm
proposing to fix with the following patch:

---
 target-mips/translate.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)



Grüße,
 Thomas
Maciej W. Rozycki - June 10, 2013, 2:21 p.m.
On Sun, 9 Jun 2013, Thomas Schwinge wrote:

> In my reading of the relevant documents, the latter change is not correct
> for o32, and empirically has "interesting" effects on the glibc math
> testsuite, for example.  Keeping the FR register unset for o32 I'm
> proposing to fix with the following patch:

 Correct, unless (until?) the -mfp64 o32 ABI extension is implemented for 
Linux, CP0.Status.FR must remain 0 for o32 programs.

> diff --git target-mips/translate.c target-mips/translate.c
> index 0a53203..51837d4 100644
> --- target-mips/translate.c
> +++ target-mips/translate.c
> @@ -15962,10 +15962,12 @@ void cpu_state_reset(CPUMIPSState *env)
>      if (env->CP0_Config3 & (1 << CP0C3_DSPP)) {
>          env->CP0_Status |= (1 << CP0St_MX);
>      }
> -    /* Enable 64-bit FPU if the target cpu supports it.  */
> +# if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
> +    /* Enable 64-bit FPU if the target CPU supports it.  */
>      if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
>          env->CP0_Status |= (1 << CP0St_FR);

 This is not entirely correct, older 64-bit FPUs, i.e. any before the 
MIPS32/MIPS64 rev. 2 ISA (e.g. R4000, R10000, 5Kf, etc.) won't have the 
CP1.FIR.F64 bit set; it was only defined at that ISA level because for 
earlier architecture revisions the type of the FPU could have been 
inferred from the type of the CPU.  Therefore the condition has to be 
changed, perhaps the best way would simply be just checking in the 
CP0.Status mask if the FR bit is writable.

 Also I suppose there must be an else clause here:

    } else {
        fprintf(stderr, "A 64-bit FPU required for NewABI emulation\n");
        exit(1);

or suchlike because the NewABI mandates full 64-bit FPU operation (or the 
condtion might be changed to an assertion instead and the emulated 
environment checked elsewhere earlier on, because a 64-bit CPU is required 
for NewABI operation anyway and a 64-bit CPU can't ever have a 32-bit FPU 
-- I don't know QEMU well enough to be sure offhand, please check).

>      }
> +# endif
>  #else
>      if (env->hflags & MIPS_HFLAG_BMASK) {
>          /* If the exception was raised from a delay slot,

  Maciej
Thomas Schwinge - Aug. 5, 2013, 2:15 p.m.
Hi!

On Sun, 28 Jul 2013 02:39:23 +0000, Petar Jovanovic <Petar.Jovanovic@imgtec.com> wrote:
> Will there be an update to this patch?

| Switch to 64-bit FPU only for n32 and n64 ABIs, but not o32.  Fixup for
| commit 68473f15d4c9948986618f63828825beafcaf1cf.
| [...]

My patch has to be reworked per Maciej's comments,
<http://news.gmane.org/find-root.php?message_id=%3Calpine.DEB.1.10.1306091926470.16287%40tp.orcam.me.uk%3E>,
which I have not yet managed to allocate the time to do, and can't tell
when it's going to happen -- but I still have it on my TODO list.


Grüße,
 Thomas

Patch

diff --git target-mips/translate.c target-mips/translate.c
index 0a53203..51837d4 100644
--- target-mips/translate.c
+++ target-mips/translate.c
@@ -15962,10 +15962,12 @@  void cpu_state_reset(CPUMIPSState *env)
     if (env->CP0_Config3 & (1 << CP0C3_DSPP)) {
         env->CP0_Status |= (1 << CP0St_MX);
     }
-    /* Enable 64-bit FPU if the target cpu supports it.  */
+# if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
+    /* Enable 64-bit FPU if the target CPU supports it.  */
     if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
         env->CP0_Status |= (1 << CP0St_FR);
     }
+# endif
 #else
     if (env->hflags & MIPS_HFLAG_BMASK) {
         /* If the exception was raised from a delay slot,