Re: [Qemu-devel] [PATCH] target-ppc: fix nego and subf*o instructions

[Aurelien Jarno <aurelien@aurel32.net>]

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