David Gibson <da...@gibson.dropbear.id.au> writes: > [ Unknown signature status ] > On Fri, Jul 22, 2016 at 10:59:18AM +0530, Nikunj A Dadhania wrote: >> David Gibson <da...@gibson.dropbear.id.au> writes: >> >> > [ Unknown signature status ] >> > On Mon, Jul 18, 2016 at 10:35:09PM +0530, Nikunj A Dadhania wrote: >> >> Adding following instructions: >> >> >> >> moduw: Modulo Unsigned Word >> >> modsw: Modulo Signed Word >> >> >> >> Signed-off-by: Nikunj A Dadhania <nik...@linux.vnet.ibm.com> >> > >> > As rth has already mentioned this many branches probably means this >> > wants a helper. >> > >> >> --- >> >> target-ppc/translate.c | 48 >> >> ++++++++++++++++++++++++++++++++++++++++++++++++ >> >> 1 file changed, 48 insertions(+) >> >> >> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c >> >> index d44f7af..487dd94 100644 >> >> --- a/target-ppc/translate.c >> >> +++ b/target-ppc/translate.c >> >> @@ -1178,6 +1178,52 @@ GEN_DIVE(divde, divde, 0); >> >> GEN_DIVE(divdeo, divde, 1); >> >> #endif >> >> >> >> +static inline void gen_op_arith_modw(DisasContext *ctx, TCGv ret, TCGv >> >> arg1, >> >> + TCGv arg2, int sign) >> >> +{ >> >> + TCGLabel *l1 = gen_new_label(); >> >> + TCGLabel *l2 = gen_new_label(); >> >> + TCGv_i32 t0 = tcg_temp_local_new_i32(); >> >> + TCGv_i32 t1 = tcg_temp_local_new_i32(); >> >> + TCGv_i32 t2 = tcg_temp_local_new_i32(); >> >> + >> >> + tcg_gen_trunc_tl_i32(t0, arg1); >> >> + tcg_gen_trunc_tl_i32(t1, arg2); >> >> + tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1); >> >> Result for: >> <anything> % 0 and ... >> >> >> + if (sign) { >> >> + TCGLabel *l3 = gen_new_label(); >> >> + tcg_gen_brcondi_i32(TCG_COND_NE, t1, -1, l3); >> >> + tcg_gen_brcondi_i32(TCG_COND_EQ, t0, INT32_MIN, l1); >> >> + gen_set_label(l3); >> > >> > It's not really clear to be what the logic above is doing. >> >> ... For signed case >> 0x8000_0000 % -1 >> >> Is undefined, addressing those cases. > > Do you mean the tcg operations have undefined results or that the ppc > instructions have undefined results?
TCG side, I haven't tried. > If the latter, then why do you care about those cases? Thats how divd is implemented as well, i didn't want to break that. I am looking at doing both div and mod as helpers. >> >> + tcg_gen_rem_i32(t2, t0, t1); >> >> + } else { >> >> + tcg_gen_remu_i32(t2, t0, t1); >> >> + } >> >> + tcg_gen_br(l2); >> >> + gen_set_label(l1); >> >> + if (sign) { >> >> + tcg_gen_sari_i32(t2, t0, 31); >> > >> > AFAICT this sets t2 to either 0 or -1 depending on the sign of t0, >> > which seems like an odd thing to do. >> >> Extending the sign later ... > > Right, so after sign extension you have a 64-bit 0 or -1. Still not > seeing what that 0 or -1 result is useful for. Oh ok, i got why you got confused. I am re-writing all of it though, but for understanding: if (divisor == 0) goto l1; if (signed) { if (divisor == -1 && dividend == INT_MIN) goto l1; compute_signed_rem(t2, t0, t1); } else { compute_unsigned_rem(t2, t0, t1); } goto l2; /* jump to setting extending result and return */ l1: /* in case of invalid input set values */ if (signed) t2 = -1 or 0; else t2 = 0; l2: set (ret, t2) Regards Nikunj