* [Qemu-devel] [PATCH] target/ppc: bcdsub fix sign when result is zero @ 2018-07-16 19:03 Yasmin Beatriz 2018-07-18 2:22 ` David Gibson 0 siblings, 1 reply; 3+ messages in thread From: Yasmin Beatriz @ 2018-07-16 19:03 UTC (permalink / raw) To: qemu-ppc; +Cc: qemu-devel, david, rth 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 <yasmins@linux.ibm.com> --- 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); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] target/ppc: bcdsub fix sign when result is zero 2018-07-16 19:03 [Qemu-devel] [PATCH] target/ppc: bcdsub fix sign when result is zero Yasmin Beatriz @ 2018-07-18 2:22 ` David Gibson 2018-07-18 12:48 ` Yasmin Beatriz 0 siblings, 1 reply; 3+ messages in thread From: David Gibson @ 2018-07-18 2:22 UTC (permalink / raw) To: Yasmin Beatriz; +Cc: qemu-ppc, qemu-devel, rth [-- Attachment #1: Type: text/plain, Size: 1762 bytes --] 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 <yasmins@linux.ibm.com> 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(&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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] target/ppc: bcdsub fix sign when result is zero 2018-07-18 2:22 ` David Gibson @ 2018-07-18 12:48 ` Yasmin Beatriz 0 siblings, 0 replies; 3+ messages in thread From: Yasmin Beatriz @ 2018-07-18 12:48 UTC (permalink / raw) To: David Gibson; +Cc: qemu-ppc, qemu-devel, rth 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 <yasmins@linux.ibm.com> > > 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-07-18 12:48 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-07-16 19:03 [Qemu-devel] [PATCH] target/ppc: bcdsub fix sign when result is zero Yasmin Beatriz 2018-07-18 2:22 ` David Gibson 2018-07-18 12:48 ` Yasmin Beatriz
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).