Re: [Qemu-devel] [Qemu-ppc] [PATCH 14/15] target-ppc: Use tcg_gen_extract_*

[David Gibson]

On Wed, Oct 26, 2016 at 08:38:06AM -0700, Richard Henderson wrote:
> On 10/25/2016 07:59 PM, David Gibson wrote:
> > On Sat, Oct 15, 2016 at 08:37:49PM -0700, Richard Henderson wrote:
> >> Use the new primitives for RDWINM and RLDICL.
> >>
> >> Cc: address@hidden
> >> Signed-off-by: Richard Henderson <address@hidden>
> >> ---
> >>  target-ppc/translate.c | 9 ++++-----
> >>  1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> >> index bfc1301..724d95c 100644
> >> --- a/target-ppc/translate.c
> >> +++ b/target-ppc/translate.c
> >> @@ -1977,9 +1977,8 @@ static void gen_rlwinm(DisasContext *ctx)
> >>      if (mb == 0 && me == (31 - sh)) {
> >>          tcg_gen_shli_tl(t_ra, t_rs, sh);
> >>          tcg_gen_ext32u_tl(t_ra, t_ra);
> >> -    } else if (sh != 0 && me == 31 && sh == (32 - mb)) {
> >> -        tcg_gen_ext32u_tl(t_ra, t_rs);
> >> -        tcg_gen_shri_tl(t_ra, t_ra, mb);
> >> +    } else if (me == 31 && (me - mb + 1) + sh <= 32) {
> > 
> > I'm having trouble figuring out what the second part of this condition
> > is supposed to be checking for, and it seems like it's too
> > restrictive.
> > 
> > For example, everything except the LSB of a word would be:
> >     rlwnim rT,rA,31,1,31
> > which would fail the test, but it should be fine to implement that
> > with an extract op.
> 
> It was confusing to me too, which is why I rearranged this in the v2 of this
> patchset.  To which thread you also responded yesterday, so...

Ah, sorry.  Because I missed it originally, I only had your ping, not
the actual v2 series in my inbox.  When I went back through my archive
to find it, I accidentally picked up v1 instead of v2.

> 
> Anyway, in v2 this looks like
> 
>     if (sh != 0 && len > 0 && me == (31 - sh)) {
>         tcg_gen_deposit_z_tl(t_ra, t_rs, sh, len);
>     } else if (me == 31 && rsh + len <= 32) {
>         tcg_gen_extract_tl(t_ra, t_rs, rsh, len);
>     } else {
> 
> Basically, we're trying to match those combinations of rotate+mask that can be
> implemented with shifts instead of real rotations.  That is, the mask doesn't
> follow the rotate around the end of the word.

Ok, that looks correct, the change frm sh to rsh is the fix, I think.

-- 
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