* Re: [PATCH] Fix MIPS add.s after 1ace099f2acb952eaaef0ba7725879949a7e4406
2020-07-03 16:15 [PATCH] Fix MIPS add.s after 1ace099f2acb952eaaef0ba7725879949a7e4406 Alex Richardson
@ 2020-07-03 16:33 ` Aleksandar Markovic
2020-07-03 18:40 ` Philippe Mathieu-Daudé
2020-07-07 16:26 ` Aleksandar Markovic
2 siblings, 0 replies; 6+ messages in thread
From: Aleksandar Markovic @ 2020-07-03 16:33 UTC (permalink / raw)
To: Alex Richardson
Cc: qemu-trivial@nongnu.org, Aleksandar Rikalo,
open list:All patches CC here, Aurelien Jarno
[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]
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
>
>
[-- Attachment #2: Type: text/html, Size: 1926 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Fix MIPS add.s after 1ace099f2acb952eaaef0ba7725879949a7e4406
2020-07-03 16:15 [PATCH] Fix MIPS add.s after 1ace099f2acb952eaaef0ba7725879949a7e4406 Alex Richardson
2020-07-03 16:33 ` Aleksandar Markovic
@ 2020-07-03 18:40 ` Philippe Mathieu-Daudé
2020-07-06 10:22 ` Alexander Richardson
2020-07-07 16:26 ` Aleksandar Markovic
2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-03 18:40 UTC (permalink / raw)
To: Alex Richardson, Aleksandar Markovic
Cc: qemu-trivial, Aleksandar Rikalo, open list:All patches CC here,
Aurelien Jarno
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;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Fix MIPS add.s after 1ace099f2acb952eaaef0ba7725879949a7e4406
2020-07-03 18:40 ` Philippe Mathieu-Daudé
@ 2020-07-06 10:22 ` Alexander Richardson
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Richardson @ 2020-07-06 10:22 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-trivial, Aleksandar Markovic, Aleksandar Rikalo,
Aurelien Jarno, open list:All patches CC here
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;
> > }
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix MIPS add.s after 1ace099f2acb952eaaef0ba7725879949a7e4406
2020-07-03 16:15 [PATCH] Fix MIPS add.s after 1ace099f2acb952eaaef0ba7725879949a7e4406 Alex Richardson
2020-07-03 16:33 ` Aleksandar Markovic
2020-07-03 18:40 ` Philippe Mathieu-Daudé
@ 2020-07-07 16:26 ` Aleksandar Markovic
2020-07-09 13:38 ` Philippe Mathieu-Daudé
2 siblings, 1 reply; 6+ messages in thread
From: Aleksandar Markovic @ 2020-07-07 16:26 UTC (permalink / raw)
To: Alex Richardson
Cc: QEMU Trivial, Aleksandar Markovic, Aleksandar Rikalo,
Aurelien Jarno, open list:All patches CC here
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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Fix MIPS add.s after 1ace099f2acb952eaaef0ba7725879949a7e4406
2020-07-07 16:26 ` Aleksandar Markovic
@ 2020-07-09 13:38 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-09 13:38 UTC (permalink / raw)
To: Aleksandar Markovic, Alex Richardson
Cc: QEMU Trivial, Aleksandar Markovic, Aleksandar Rikalo,
Aurelien Jarno, open list:All patches CC here
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
>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread