diff mbox series

Fix MIPS add.s after 1ace099f2acb952eaaef0ba7725879949a7e4406

Message ID 20200703161515.25966-1-Alexander.Richardson@cl.cam.ac.uk
State New
Headers show
Series Fix MIPS add.s after 1ace099f2acb952eaaef0ba7725879949a7e4406 | expand

Commit Message

Alex Richardson July 3, 2020, 4:15 p.m. UTC 
After merging latest QEMU upstream into our CHERI fork, I noticed that
some of the FPU tests in our MIPS baremetal testsuite
(https://github.com/CTSRD-CHERI/cheritest) started failing. It turns out
this commit accidentally changed add.s into a subtract.

Signed-off-by: Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
---
 target/mips/fpu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Aleksandar Markovic July 3, 2020, 4:33 p.m. UTC | #1 
On Friday, July 3, 2020, Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
wrote:

> After merging latest QEMU upstream into our CHERI fork, I noticed that
> some of the FPU tests in our MIPS baremetal testsuite
> (https://github.com/CTSRD-CHERI/cheritest) started failing. It turns out
> this commit accidentally changed add.s into a subtract.
>
>
Alex, all you said sounds very probable to me. I currently don't have any
dev system, at hand, but as soon as I get it, I will confirm/disconfirm
(but again, most likely confirm) this problem, and than we'll give you
green lifght for your patch.

Yours,
Aleksandar




> Signed-off-by: Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
> ---
>  target/mips/fpu_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
> index 7a3a61cab3..56beda49d8 100644
> --- a/target/mips/fpu_helper.c
> +++ b/target/mips/fpu_helper.c
> @@ -1221,7 +1221,7 @@ uint32_t helper_float_add_s(CPUMIPSState *env,
>  {
>      uint32_t wt2;
>
> -    wt2 = float32_sub(fst0, fst1, &env->active_fpu.fp_status);
> +    wt2 = float32_add(fst0, fst1, &env->active_fpu.fp_status);
>      update_fcr31(env, GETPC());
>      return wt2;
>  }
> --
> 2.27.0
>
>
Philippe Mathieu-Daudé July 3, 2020, 6:40 p.m. UTC | #2 
Hi Alex,

On 7/3/20 6:15 PM, Alex Richardson wrote:
> After merging latest QEMU upstream into our CHERI fork, I noticed that
> some of the FPU tests in our MIPS baremetal testsuite

I understand by baremetal your soft core implementation running on
a FPGA, right?

> (https://github.com/CTSRD-CHERI/cheritest) started failing. It turns out
> this commit accidentally changed add.s into a subtract.

Fixes: 1ace099f2a ("target/mips: fpu: Demacro ADD.<D|S|PS>")
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks for your quick fix!

Phil.

> Signed-off-by: Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
> ---
>  target/mips/fpu_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
> index 7a3a61cab3..56beda49d8 100644
> --- a/target/mips/fpu_helper.c
> +++ b/target/mips/fpu_helper.c
> @@ -1221,7 +1221,7 @@ uint32_t helper_float_add_s(CPUMIPSState *env,
>  {
>      uint32_t wt2;
>  
> -    wt2 = float32_sub(fst0, fst1, &env->active_fpu.fp_status);
> +    wt2 = float32_add(fst0, fst1, &env->active_fpu.fp_status);
>      update_fcr31(env, GETPC());
>      return wt2;
>  }
>
Alex Richardson July 6, 2020, 10:22 a.m. UTC | #3 
Hi Philippe,

On Fri, 3 Jul 2020 at 19:40, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Alex,
>
> On 7/3/20 6:15 PM, Alex Richardson wrote:
> > After merging latest QEMU upstream into our CHERI fork, I noticed that
> > some of the FPU tests in our MIPS baremetal testsuite
>
> I understand by baremetal your soft core implementation running on
> a FPGA, right?
>
By baremetal I mean small test binaries running without a host OS.
The test suite was originally written for the CHERI FPGAs, but it also
supports various MIPS simulators, our executable formal model written
in sail and our fork of QEMU (https://github.com/CTSRD-CHERI/qemu).
Unfortunately it cannot be run with upstream QEMU as it requires a
special MTC0 instruction to dump register values in a textual format
to the logfile.

> > (https://github.com/CTSRD-CHERI/cheritest) started failing. It turns out
> > this commit accidentally changed add.s into a subtract.
>
> Fixes: 1ace099f2a ("target/mips: fpu: Demacro ADD.<D|S|PS>")
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Thanks for your quick fix!
>
> Phil.
>
> > Signed-off-by: Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
> > ---
> >  target/mips/fpu_helper.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
> > index 7a3a61cab3..56beda49d8 100644
> > --- a/target/mips/fpu_helper.c
> > +++ b/target/mips/fpu_helper.c
> > @@ -1221,7 +1221,7 @@ uint32_t helper_float_add_s(CPUMIPSState *env,
> >  {
> >      uint32_t wt2;
> >
> > -    wt2 = float32_sub(fst0, fst1, &env->active_fpu.fp_status);
> > +    wt2 = float32_add(fst0, fst1, &env->active_fpu.fp_status);
> >      update_fcr31(env, GETPC());
> >      return wt2;
> >  }
> >
>
Aleksandar Markovic July 7, 2020, 4:26 p.m. UTC | #4 
On Fri, Jul 3, 2020 at 6:33 PM Alex Richardson
<Alexander.Richardson@cl.cam.ac.uk> wrote:
>
> After merging latest QEMU upstream into our CHERI fork, I noticed that
> some of the FPU tests in our MIPS baremetal testsuite
> (https://github.com/CTSRD-CHERI/cheritest) started failing. It turns out
> this commit accidentally changed add.s into a subtract.
>
> Signed-off-by: Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
> ---

Applied to MIPS + TCG Continuous Benchmarking queue.

>  target/mips/fpu_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
> index 7a3a61cab3..56beda49d8 100644
> --- a/target/mips/fpu_helper.c
> +++ b/target/mips/fpu_helper.c
> @@ -1221,7 +1221,7 @@ uint32_t helper_float_add_s(CPUMIPSState *env,
>  {
>      uint32_t wt2;
>
> -    wt2 = float32_sub(fst0, fst1, &env->active_fpu.fp_status);
> +    wt2 = float32_add(fst0, fst1, &env->active_fpu.fp_status);
>      update_fcr31(env, GETPC());
>      return wt2;
>  }
> --
> 2.27.0
>
>
Philippe Mathieu-Daudé July 9, 2020, 1:38 p.m. UTC | #5 
Hi Aleksandar,

On 7/7/20 6:26 PM, Aleksandar Markovic wrote:
> On Fri, Jul 3, 2020 at 6:33 PM Alex Richardson
> <Alexander.Richardson@cl.cam.ac.uk> wrote:
>>
>> After merging latest QEMU upstream into our CHERI fork, I noticed that
>> some of the FPU tests in our MIPS baremetal testsuite
>> (https://github.com/CTSRD-CHERI/cheritest) started failing. It turns out
>> this commit accidentally changed add.s into a subtract.
>>
>> Signed-off-by: Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
>> ---
> 
> Applied to MIPS + TCG Continuous Benchmarking queue.

If you don't mind I'll include this patch for the mips pull request I
plan to send before hard freeze (on the list). I'm keeping your S-o-b.

> 
>>  target/mips/fpu_helper.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
>> index 7a3a61cab3..56beda49d8 100644
>> --- a/target/mips/fpu_helper.c
>> +++ b/target/mips/fpu_helper.c
>> @@ -1221,7 +1221,7 @@ uint32_t helper_float_add_s(CPUMIPSState *env,
>>  {
>>      uint32_t wt2;
>>
>> -    wt2 = float32_sub(fst0, fst1, &env->active_fpu.fp_status);
>> +    wt2 = float32_add(fst0, fst1, &env->active_fpu.fp_status);
>>      update_fcr31(env, GETPC());
>>      return wt2;
>>  }
>> --
>> 2.27.0
>>
>>
>
diff mbox series

Patch

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index 7a3a61cab3..56beda49d8 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1221,7 +1221,7 @@  uint32_t helper_float_add_s(CPUMIPSState *env,
 {
     uint32_t wt2;
 
-    wt2 = float32_sub(fst0, fst1, &env->active_fpu.fp_status);
+    wt2 = float32_add(fst0, fst1, &env->active_fpu.fp_status);
     update_fcr31(env, GETPC());
     return wt2;
 }