Re: [Qemu-devel] [PATCH] target/ppc: bcdsub fix sign when result is zero

[Yasmin Beatriz]

On Wed, Jul 18, 2018 at 12:22:02PM +1000, David Gibson wrote:
> On Mon, Jul 16, 2018 at 07:03:12PM +0000, Yasmin Beatriz wrote:
> > When the result of bcdsub is equal to zero, the result sign may be
> > set to negative in some cases, and this does not follow the Power ISA
> > specifications as to decimal integer arithmetic instructions.
> > 
> > Signed-off-by: Yasmin Beatriz <address@hidden>
> 
> The fix looks correct, but since this has been broken forever and
> we're in hard freeze I'm going to apply for 3.1, not 3.0.
> 
> Fwiw, I think the surrounding logic can probably be simplified a
> little as well - I think this is now the only case with a zero result,
> so I think the 'zero' local can probably be removed.

You are right. I am working on this and will send a new patch very soon.
Thank you for the note.
 
> > ---
> >  target/ppc/int_helper.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
> > index 03d37da..fa18e6e 100644
> > --- a/target/ppc/int_helper.c
> > +++ b/target/ppc/int_helper.c
> > @@ -2747,6 +2747,9 @@ uint32_t helper_bcdadd(ppc_avr_t *r,  ppc_avr_t *a, 
> > ppc_avr_t *b, uint32_t ps)
> >              result.u8[BCD_DIG_BYTE(0)] = bcd_preferred_sgn(sgna, ps);
> >              zero = bcd_sub_mag(&result, a, b, &invalid, &overflow);
> >              cr = (sgna > 0) ? CRF_GT : CRF_LT;
> > +        } else if (bcd_cmp_mag(a, b) == 0) {
> > +            result.u8[BCD_DIG_BYTE(0)] = bcd_preferred_sgn(0, ps);
> > +            zero = bcd_sub_mag(&result, b, a, &invalid, &overflow);
> >          } else {
> >              result.u8[BCD_DIG_BYTE(0)] = bcd_preferred_sgn(sgnb, ps);
> >              zero = bcd_sub_mag(&result, b, a, &invalid, &overflow);
> 
> -- 
> David Gibson                  | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au        | minimalist, thank you.  NOT _the_ 
> _other_
>                               | _way_ _around_!
> http://www.ozlabs.org/~dgibson