* [Qemu-devel] [PATCH] target-ppc: fix nego and subf*o instructions
@ 2013-03-30 23:54 Aurelien Jarno
2013-03-31 23:19 ` Richard Henderson
0 siblings, 1 reply; 6+ messages in thread
From: Aurelien Jarno @ 2013-03-30 23:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Graf, Aurelien Jarno, Richard Henderson
The overflow computation of nego and subf*o instructions has been broken
in commit ffe30937. This patch fixes it.
With this change the PPC emulation passes the Gwenole Beauchesne
testsuite again.
Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
target-ppc/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 5e741d1..062493a 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -749,7 +749,7 @@ static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
tcg_gen_xor_tl(cpu_ov, arg0, arg1);
tcg_gen_xor_tl(t0, arg1, arg2);
if (sub) {
- tcg_gen_and_tl(cpu_ov, cpu_ov, t0);
+ tcg_gen_andc_tl(cpu_ov, t0, cpu_ov);
} else {
tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] target-ppc: fix nego and subf*o instructions
2013-03-30 23:54 [Qemu-devel] [PATCH] target-ppc: fix nego and subf*o instructions Aurelien Jarno
@ 2013-03-31 23:19 ` Richard Henderson
2013-03-31 23:50 ` Peter Maydell
2013-04-01 0:22 ` Aurelien Jarno
0 siblings, 2 replies; 6+ messages in thread
From: Richard Henderson @ 2013-03-31 23:19 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel, Alexander Graf
On 2013-03-30 16:54, Aurelien Jarno wrote:
> The overflow computation of nego and subf*o instructions has been broken
> in commit ffe30937. This patch fixes it.
>
> With this change the PPC emulation passes the Gwenole Beauchesne
> testsuite again.
>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-ppc/translate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 5e741d1..062493a 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -749,7 +749,7 @@ static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
> tcg_gen_xor_tl(cpu_ov, arg0, arg1);
> tcg_gen_xor_tl(t0, arg1, arg2);
> if (sub) {
> - tcg_gen_and_tl(cpu_ov, cpu_ov, t0);
> + tcg_gen_andc_tl(cpu_ov, t0, cpu_ov);
> } else {
> tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
> }
I'm a bit confused. This is the exact same algorithm that's used on ARM and
i386. And as far as I can determine, all three platforms have the same
definition of "overflow".
r~
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] target-ppc: fix nego and subf*o instructions
2013-03-31 23:19 ` Richard Henderson
@ 2013-03-31 23:50 ` Peter Maydell
2013-04-01 0:31 ` Aurelien Jarno
2013-04-01 1:15 ` Richard Henderson
2013-04-01 0:22 ` Aurelien Jarno
1 sibling, 2 replies; 6+ messages in thread
From: Peter Maydell @ 2013-03-31 23:50 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, Aurelien Jarno, Alexander Graf
On 1 April 2013 00:19, Richard Henderson <rth@twiddle.net> wrote:
> On 2013-03-30 16:54, Aurelien Jarno wrote:
>>
>> The overflow computation of nego and subf*o instructions has been broken
>> in commit ffe30937. This patch fixes it.
>>
>> With this change the PPC emulation passes the Gwenole Beauchesne
>> testsuite again.
>>
>> Cc: Alexander Graf <agraf@suse.de>
>> Cc: Richard Henderson <rth@twiddle.net>
>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>> ---
>> target-ppc/translate.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index 5e741d1..062493a 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -749,7 +749,7 @@ static inline void
>> gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
>> tcg_gen_xor_tl(cpu_ov, arg0, arg1);
>> tcg_gen_xor_tl(t0, arg1, arg2);
>> if (sub) {
>> - tcg_gen_and_tl(cpu_ov, cpu_ov, t0);
>> + tcg_gen_andc_tl(cpu_ov, t0, cpu_ov);
>> } else {
>> tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
>> }
>
>
> I'm a bit confused. This is the exact same algorithm that's used on ARM and
> i386. And as far as I can determine, all three platforms have the same
> definition of "overflow".
I think it's not quite the same as ARM because the two arguments
to subtract are reversed for PPC (ie PPC is 'subtract from', not
'subtract'). I think that means you want 'xor_tl(cpu_ov, arg0, arg2)'
for your first xor, maybe? (untested)
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] target-ppc: fix nego and subf*o instructions
2013-03-31 23:50 ` Peter Maydell
@ 2013-04-01 0:31 ` Aurelien Jarno
2013-04-01 1:15 ` Richard Henderson
1 sibling, 0 replies; 6+ messages in thread
From: Aurelien Jarno @ 2013-04-01 0:31 UTC (permalink / raw)
To: Peter Maydell; +Cc: Alexander Graf, qemu-devel, Richard Henderson
On Mon, Apr 01, 2013 at 12:50:58AM +0100, Peter Maydell wrote:
> On 1 April 2013 00:19, Richard Henderson <rth@twiddle.net> wrote:
> > On 2013-03-30 16:54, Aurelien Jarno wrote:
> >>
> >> The overflow computation of nego and subf*o instructions has been broken
> >> in commit ffe30937. This patch fixes it.
> >>
> >> With this change the PPC emulation passes the Gwenole Beauchesne
> >> testsuite again.
> >>
> >> Cc: Alexander Graf <agraf@suse.de>
> >> Cc: Richard Henderson <rth@twiddle.net>
> >> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >> ---
> >> target-ppc/translate.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> >> index 5e741d1..062493a 100644
> >> --- a/target-ppc/translate.c
> >> +++ b/target-ppc/translate.c
> >> @@ -749,7 +749,7 @@ static inline void
> >> gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
> >> tcg_gen_xor_tl(cpu_ov, arg0, arg1);
> >> tcg_gen_xor_tl(t0, arg1, arg2);
> >> if (sub) {
> >> - tcg_gen_and_tl(cpu_ov, cpu_ov, t0);
> >> + tcg_gen_andc_tl(cpu_ov, t0, cpu_ov);
> >> } else {
> >> tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
> >> }
> >
> >
> > I'm a bit confused. This is the exact same algorithm that's used on ARM and
> > i386. And as far as I can determine, all three platforms have the same
> > definition of "overflow".
>
> I think it's not quite the same as ARM because the two arguments
> to subtract are reversed for PPC (ie PPC is 'subtract from', not
> 'subtract'). I think that means you want 'xor_tl(cpu_ov, arg0, arg2)'
> for your first xor, maybe? (untested)
Indeed, this way of doing also works, and is also cleaner. I'll send a
new version of the patch. Thanks.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] target-ppc: fix nego and subf*o instructions
2013-03-31 23:50 ` Peter Maydell
2013-04-01 0:31 ` Aurelien Jarno
@ 2013-04-01 1:15 ` Richard Henderson
1 sibling, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2013-04-01 1:15 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, Aurelien Jarno, Alexander Graf
On 03/31/2013 04:50 PM, Peter Maydell wrote:
>> > I'm a bit confused. This is the exact same algorithm that's used on ARM and
>> > i386. And as far as I can determine, all three platforms have the same
>> > definition of "overflow".
> I think it's not quite the same as ARM because the two arguments
> to subtract are reversed for PPC (ie PPC is 'subtract from', not
> 'subtract'). I think that means you want 'xor_tl(cpu_ov, arg0, arg2)'
> for your first xor, maybe? (untested)
Oh, duh. Of course.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: fix nego and subf*o instructions
2013-03-31 23:19 ` Richard Henderson
2013-03-31 23:50 ` Peter Maydell
@ 2013-04-01 0:22 ` Aurelien Jarno
1 sibling, 0 replies; 6+ messages in thread
From: Aurelien Jarno @ 2013-04-01 0:22 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, Alexander Graf
On Sun, Mar 31, 2013 at 04:19:45PM -0700, Richard Henderson wrote:
> On 2013-03-30 16:54, Aurelien Jarno wrote:
> >The overflow computation of nego and subf*o instructions has been broken
> >in commit ffe30937. This patch fixes it.
> >
> >With this change the PPC emulation passes the Gwenole Beauchesne
> >testsuite again.
> >
> >Cc: Alexander Graf <agraf@suse.de>
> >Cc: Richard Henderson <rth@twiddle.net>
> >Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >---
> > target-ppc/translate.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> >index 5e741d1..062493a 100644
> >--- a/target-ppc/translate.c
> >+++ b/target-ppc/translate.c
> >@@ -749,7 +749,7 @@ static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
> > tcg_gen_xor_tl(cpu_ov, arg0, arg1);
> > tcg_gen_xor_tl(t0, arg1, arg2);
> > if (sub) {
> >- tcg_gen_and_tl(cpu_ov, cpu_ov, t0);
> >+ tcg_gen_andc_tl(cpu_ov, t0, cpu_ov);
> > } else {
> > tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
> > }
>
> I'm a bit confused. This is the exact same algorithm that's used on
> ARM and i386. And as far as I can determine, all three platforms
> have the same definition of "overflow".
>
I based my patch on the previous version of the code, which swaps
*both* the brcond conditions testing the results of the xor. As far I
understand the brcond are replaced by the and/andc in your patch.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-01 1:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-30 23:54 [Qemu-devel] [PATCH] target-ppc: fix nego and subf*o instructions Aurelien Jarno
2013-03-31 23:19 ` Richard Henderson
2013-03-31 23:50 ` Peter Maydell
2013-04-01 0:31 ` Aurelien Jarno
2013-04-01 1:15 ` Richard Henderson
2013-04-01 0:22 ` Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).