| Message ID | 1364687686-6006-1-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
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~
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
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.
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.
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~
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); }
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(-)