| Message ID | 1487585521-19445-8-git-send-email-nikunj@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote: > For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV > and OV32 are set to 1. > > For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are > set to 1. > > Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as > nego was the last user. > > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > --- > target/ppc/translate.c | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index 9fa3b5a..0168e1c 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -1473,14 +1473,6 @@ static void gen_subfic(DisasContext *ctx) > } > > /* neg neg. nego nego. */ > -static inline void gen_op_arith_neg(DisasContext *ctx, bool compute_ov) > -{ > - TCGv zero = tcg_const_tl(0); > - gen_op_arith_subf(ctx, cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)], > - zero, 0, 0, compute_ov, Rc(ctx->opcode)); > - tcg_temp_free(zero); > -} > - > static void gen_neg(DisasContext *ctx) > { > tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]); > @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx) > > static void gen_nego(DisasContext *ctx) > { > - gen_op_arith_neg(ctx, 1); > + TCGv t0 = tcg_temp_new(); > + TCGv zero = tcg_const_tl(0); > + > + if (NARROW_MODE(ctx)) { > + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN); > + } else { > + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN); > + } > + > + tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero); > + tcg_gen_mov_tl(cpu_ov32, cpu_ov); > + tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]); > + tcg_temp_free(t0); > + tcg_temp_free(zero); > } Again, you're forgetting "nego.". Don't try to simplify from gen_op_arith_subf by hand. r~
Richard Henderson <rth@twiddle.net> writes: > On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote: >> For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV >> and OV32 are set to 1. >> >> For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are >> set to 1. >> >> Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as >> nego was the last user. >> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx) >> >> static void gen_nego(DisasContext *ctx) >> { >> - gen_op_arith_neg(ctx, 1); >> + TCGv t0 = tcg_temp_new(); >> + TCGv zero = tcg_const_tl(0); >> + >> + if (NARROW_MODE(ctx)) { >> + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN); >> + } else { >> + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN); >> + } >> + >> + tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero); >> + tcg_gen_mov_tl(cpu_ov32, cpu_ov); >> + tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]); >> + tcg_temp_free(t0); >> + tcg_temp_free(zero); >> } > > Again, you're forgetting "nego.". Don't try to simplify from gen_op_arith_subf > by hand. The reason of the simplification was the interpretation of ov and ov32. I will add a code to compute the Rc. Regards Nikunj
On 02/21/2017 08:26 PM, Nikunj A Dadhania wrote: > Richard Henderson <rth@twiddle.net> writes: > >> On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote: >>> For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV >>> and OV32 are set to 1. >>> >>> For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are >>> set to 1. >>> >>> Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as >>> nego was the last user. >>> >>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> >> @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx) >>> >>> static void gen_nego(DisasContext *ctx) >>> { >>> - gen_op_arith_neg(ctx, 1); >>> + TCGv t0 = tcg_temp_new(); >>> + TCGv zero = tcg_const_tl(0); >>> + >>> + if (NARROW_MODE(ctx)) { >>> + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN); >>> + } else { >>> + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN); >>> + } >>> + >>> + tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero); >>> + tcg_gen_mov_tl(cpu_ov32, cpu_ov); >>> + tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]); >>> + tcg_temp_free(t0); >>> + tcg_temp_free(zero); >>> } >> >> Again, you're forgetting "nego.". Don't try to simplify from gen_op_arith_subf >> by hand. > > The reason of the simplification was the interpretation of ov and ov32. > I will add a code to compute the Rc. Why do you believe that the computation for overflow is different between neg and subf? r~
Richard Henderson <rth@twiddle.net> writes: > On 02/21/2017 08:26 PM, Nikunj A Dadhania wrote: >> Richard Henderson <rth@twiddle.net> writes: >> >>> On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote: >>>> For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV >>>> and OV32 are set to 1. >>>> >>>> For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are >>>> set to 1. >>>> >>>> Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as >>>> nego was the last user. >>>> >>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> >>> @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx) >>>> >>>> static void gen_nego(DisasContext *ctx) >>>> { >>>> - gen_op_arith_neg(ctx, 1); >>>> + TCGv t0 = tcg_temp_new(); >>>> + TCGv zero = tcg_const_tl(0); >>>> + >>>> + if (NARROW_MODE(ctx)) { >>>> + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN); >>>> + } else { >>>> + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN); >>>> + } >>>> + >>>> + tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero); >>>> + tcg_gen_mov_tl(cpu_ov32, cpu_ov); >>>> + tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]); >>>> + tcg_temp_free(t0); >>>> + tcg_temp_free(zero); >>>> } >>> >>> Again, you're forgetting "nego.". Don't try to simplify from gen_op_arith_subf >>> by hand. >> >> The reason of the simplification was the interpretation of ov and ov32. >> I will add a code to compute the Rc. > > Why do you believe that the computation for overflow is different between neg > and subf? For example in 64-bit mode, if we negate INT64_MIN(nego[.]), both OV and OV32 has to be set to 1 according to ISA 3.0. If I use subf, only OV is set to 1. Regards Nikunj
On 02/22/2017 02:53 PM, Nikunj A Dadhania wrote: > Richard Henderson <rth@twiddle.net> writes: > >> On 02/21/2017 08:26 PM, Nikunj A Dadhania wrote: >>> Richard Henderson <rth@twiddle.net> writes: >>> >>>> On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote: >>>>> For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV >>>>> and OV32 are set to 1. >>>>> >>>>> For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are >>>>> set to 1. >>>>> >>>>> Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as >>>>> nego was the last user. >>>>> >>>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> >>>> @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx) >>>>> >>>>> static void gen_nego(DisasContext *ctx) >>>>> { >>>>> - gen_op_arith_neg(ctx, 1); >>>>> + TCGv t0 = tcg_temp_new(); >>>>> + TCGv zero = tcg_const_tl(0); >>>>> + >>>>> + if (NARROW_MODE(ctx)) { >>>>> + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN); >>>>> + } else { >>>>> + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN); >>>>> + } >>>>> + >>>>> + tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero); >>>>> + tcg_gen_mov_tl(cpu_ov32, cpu_ov); >>>>> + tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]); >>>>> + tcg_temp_free(t0); >>>>> + tcg_temp_free(zero); >>>>> } >>>> >>>> Again, you're forgetting "nego.". Don't try to simplify from gen_op_arith_subf >>>> by hand. >>> >>> The reason of the simplification was the interpretation of ov and ov32. >>> I will add a code to compute the Rc. >> >> Why do you believe that the computation for overflow is different between neg >> and subf? > > For example in 64-bit mode, if we negate INT64_MIN(nego[.]), both OV and > OV32 has to be set to 1 according to ISA 3.0. If I use subf, only OV is > set to 1. What an odd corner case for OV32 wrt nego. But you're right that's what the manual says. I wonder why the hardware folk designed the chip that way. It seems broken. You might want to confirm with the hardware folk that this isn't a bug in the manual. r~
Richard Henderson <rth@twiddle.net> writes: > On 02/22/2017 02:53 PM, Nikunj A Dadhania wrote: >> Richard Henderson <rth@twiddle.net> writes: >> >>> On 02/21/2017 08:26 PM, Nikunj A Dadhania wrote: >>>> Richard Henderson <rth@twiddle.net> writes: >>>> >>>>> On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote: >>>>>> For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV >>>>>> and OV32 are set to 1. >>>>>> >>>>>> For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are >>>>>> set to 1. >>>>>> >>>>>> Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as >>>>>> nego was the last user. >>>>>> >>>>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> >>>>> @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx) >>>>>> >>>>>> static void gen_nego(DisasContext *ctx) >>>>>> { >>>>>> - gen_op_arith_neg(ctx, 1); >>>>>> + TCGv t0 = tcg_temp_new(); >>>>>> + TCGv zero = tcg_const_tl(0); >>>>>> + >>>>>> + if (NARROW_MODE(ctx)) { >>>>>> + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN); >>>>>> + } else { >>>>>> + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN); >>>>>> + } >>>>>> + >>>>>> + tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero); >>>>>> + tcg_gen_mov_tl(cpu_ov32, cpu_ov); >>>>>> + tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]); >>>>>> + tcg_temp_free(t0); >>>>>> + tcg_temp_free(zero); >>>>>> } >>>>> >>>>> Again, you're forgetting "nego.". Don't try to simplify from gen_op_arith_subf >>>>> by hand. >>>> >>>> The reason of the simplification was the interpretation of ov and ov32. >>>> I will add a code to compute the Rc. >>> >>> Why do you believe that the computation for overflow is different between neg >>> and subf? >> >> For example in 64-bit mode, if we negate INT64_MIN(nego[.]), both OV and >> OV32 has to be set to 1 according to ISA 3.0. If I use subf, only OV is >> set to 1. > > What an odd corner case for OV32 wrt nego. But you're right that's what the > manual says. I wonder why the hardware folk designed the chip that way. It > seems broken. > > You might want to confirm with the hardware folk that this isn't a bug in the > manual. Sure, moreover I figured out that the hardware simulator isnt following the ISA3.0 behaviour (setting OV and OV32 both) Regards Nikunj
diff --git a/target/ppc/translate.c b/target/ppc/translate.c index 9fa3b5a..0168e1c 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -1473,14 +1473,6 @@ static void gen_subfic(DisasContext *ctx) } /* neg neg. nego nego. */ -static inline void gen_op_arith_neg(DisasContext *ctx, bool compute_ov) -{ - TCGv zero = tcg_const_tl(0); - gen_op_arith_subf(ctx, cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)], - zero, 0, 0, compute_ov, Rc(ctx->opcode)); - tcg_temp_free(zero); -} - static void gen_neg(DisasContext *ctx) { tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]); @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx) static void gen_nego(DisasContext *ctx) { - gen_op_arith_neg(ctx, 1); + TCGv t0 = tcg_temp_new(); + TCGv zero = tcg_const_tl(0); + + if (NARROW_MODE(ctx)) { + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN); + } else { + tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN); + } + + tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero); + tcg_gen_mov_tl(cpu_ov32, cpu_ov); + tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]); + tcg_temp_free(t0); + tcg_temp_free(zero); } /*** Integer logical ***/
For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV and OV32 are set to 1. For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are set to 1. Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as nego was the last user. Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> --- target/ppc/translate.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)