[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [RFC v1 05/13] target-ppc: add modulo word operations
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [RFC v1 05/13] target-ppc: add modulo word operations |
Date: |
Fri, 22 Jul 2016 16:09:48 +1000 |
User-agent: |
Mutt/1.6.2 (2016-07-01) |
On Fri, Jul 22, 2016 at 10:59:18AM +0530, Nikunj A Dadhania wrote:
> David Gibson <address@hidden> 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 <address@hidden>
> >
> > 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? If the latter, then why do you
care about those cases?
> >> + 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.
>
> >> + } else {
> >> + tcg_gen_movi_i32(t2, 0);
> >> + }
> >> + gen_set_label(l2);
> >> + tcg_gen_extu_i32_tl(ret, t2);
>
> ... Here.
>
> Regards
> Nikunj
>
--
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
[Qemu-ppc] [RFC v1 06/13] target-ppc: add modulo dword operations, Nikunj A Dadhania, 2016/07/18
[Qemu-ppc] [RFC v1 04/13] target-ppc: add cmprb instruction, Nikunj A Dadhania, 2016/07/18
[Qemu-ppc] [RFC v1 07/13] target-ppc: add cnttzd[.] instruction, Nikunj A Dadhania, 2016/07/18
[Qemu-ppc] [RFC v1 08/13] target-ppc: add cnttzw[.] instruction, Nikunj A Dadhania, 2016/07/18