Re: [Qemu-ppc] [PATCH v1 3/3] target-ppc: implement xxbr[qdwh] instruction

[David Gibson]

On Thu, Oct 13, 2016 at 01:14:35PM -0500, Richard Henderson wrote:
> On 10/12/2016 07:21 PM, David Gibson wrote:
> > > +static void gen_bswap32x4(TCGv_i64 outh, TCGv_i64 outl,
> > > +                          TCGv_i64 inh, TCGv_i64 inl)
> > > +{
> > > +    TCGv_i64 hi = tcg_temp_new_i64();
> > > +    TCGv_i64 lo = tcg_temp_new_i64();
> > > +
> > > +    tcg_gen_bswap64_i64(hi, inh);
> > > +    tcg_gen_bswap64_i64(lo, inl);
> > > +    tcg_gen_shri_i64(outh, hi, 32);
> > > +    tcg_gen_deposit_i64(outh, outh, hi, 32, 32);
> > > +    tcg_gen_shri_i64(outl, lo, 32);
> > > +    tcg_gen_deposit_i64(outl, outl, lo, 32, 32);
> > > +
> > > +    tcg_temp_free_i64(hi);
> > > +    tcg_temp_free_i64(lo);
> > > +}
> > 
> > Is there actually any advantage to having this 128-bit operation
> > working on two 64-bit "register"s, as opposed to having a bswap32x2
> > that operates on a single 64-bit register amd calling it twice?
> 
> For this one, no particular advantage.
> 
> > > +    gen_bswap16x8(xth, xtl, xbh, xbl);
> > 
> > Likewise for the 16x8 version, I guess, although that would mean
> > changing the existing users.
> 
> For this one, we have to build a 64-bit constant, 0x00ff00ff00ff00ff.  On
> some hosts that's up to 6 insns.  Being about to reuse that for both swaps
> is useful.

Ah, good point.

> > > +    tcg_gen_bswap64_i64(t0, xbl);
> > > +    tcg_gen_bswap64_i64(xtl, xbh);
> > > +    tcg_gen_bswap64_i64(xth, t0);
> > 
> > This looks wrong.  You swap xbl as you move it to t0, then swap it
> > again as you put it back into xth.  So it looks like you'll translate
> >            0011223344556677 8899AABBCCDDEEFF
> > to
> >            8899AABBCCDDEEFF 7766554433221100
> > whereas it should become
> >        FFEEDDCCBBAA9977 7766554433221100
> 
> Indeed, the third line should be a move, not a swap.
> 
> 
> r~
> 

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