Re: [Qemu-devel] [PATCH] target/ppc: bcdsub fix sign when result is zero
On Wed, Jul 18, 2018 at 12:22:02PM +1000, David Gibson wrote:
> On Mon, Jul 16, 2018 at 07:03:12PM +, 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
>
> 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(, a, b, , );
> > 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(, b, a, , );
> > } else {
> > result.u8[BCD_DIG_BYTE(0)] = bcd_preferred_sgn(sgnb, ps);
> > zero = bcd_sub_mag(, b, a, , );
>
> --
> 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
Re: [Qemu-devel] [PATCH] target/ppc: bcdsub fix sign when result is zero
On Mon, Jul 16, 2018 at 07:03:12PM +, 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
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.
> ---
> 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(, a, b, , );
> 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(, b, a, , );
> } else {
> result.u8[BCD_DIG_BYTE(0)] = bcd_preferred_sgn(sgnb, ps);
> zero = bcd_sub_mag(, b, a, , );
--
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
signature.asc
Description: PGP signature
