Re: [Qemu-devel] [PATCH 4/8] target/ppc: Optimize emulation of vgbbd instruction

[Richard Henderson <richard.henderson@linaro.org>]

On 6/6/19 5:15 AM, Stefan Brankovic wrote:
> Optimize altivec instruction vgbbd (Vector Gather Bits by Bytes by Doubleword)
> All ith bits (i in range 1 to 8) of each byte of doubleword element in
> source register are concatenated and placed into ith byte of appropriate
> doubleword element in destination register.
> 
> Following solution is done for every doubleword element of source register
> (placed in shifted variable):
> We gather bits in 2x8 iterations.
> In first iteration bit 1 of byte 1, bit 2 of byte 2,... bit 8 of byte 8 are
> in their final spots so we just and avr with mask. For every next iteration,
> we have to shift right both shifted(7 places) and mask(8 places), so we get
> bit 1 of byte 2, bit 2 of byte 3.. bit 7 of byte 8 in right places so we and
> shifted with new value of mask... After first 8 iteration(first for loop) we
> have all first bits in their final place all second bits but second bit from
> eight byte in their place,... only 1 eight bit from eight byte is in it's
> place), so we and result1 with mask1 to save those bits that are at right
> place and save them in result1. In second loop we do all operations
> symetrical, so we get other half of bits on their final spots, and save
> result in result2. Or of result1 and result2 is placed in appropriate
> doubleword element of vD. We repeat this 2 times.
> 
> Signed-off-by: Stefan Brankovic <stefan.brankovic@rt-rk.com>
> ---
>  target/ppc/translate/vmx-impl.inc.c | 99 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 98 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/translate/vmx-impl.inc.c b/target/ppc/translate/vmx-impl.inc.c
> index 87f69dc..010f337 100644
> --- a/target/ppc/translate/vmx-impl.inc.c
> +++ b/target/ppc/translate/vmx-impl.inc.c
> @@ -780,6 +780,103 @@ static void trans_vsr(DisasContext *ctx)
>      tcg_temp_free_i64(tmp);
>  }
>  
> +/*
> + * vgbbd VRT,VRB - Vector Gather Bits by Bytes by Doubleword
> + *
> + * All ith bits (i in range 1 to 8) of each byte of doubleword element in source
> + * register are concatenated and placed into ith byte of appropriate doubleword
> + * element in destination register.
> + *
> + * Following solution is done for every doubleword element of source register
> + * (placed in shifted variable):
> + * We gather bits in 2x8 iterations.
> + * In first iteration bit 1 of byte 1, bit 2 of byte 2,... bit 8 of byte 8 are
> + * in their final spots so we just and avr with mask. For every next iteration,
> + * we have to shift right both shifted(7 places) and mask(8 places), so we get
> + * bit 1 of byte 2, bit 2 of byte 3.. bit 7 of byte 8 in right places so we and
> + * shifted with new value of mask... After first 8 iteration(first for loop) we
> + * have all first bits in their final place all second bits but second bit from
> + * eight byte in their place,... only 1 eight bit from eight byte is in it's
> + * place), so we and result1 with mask1 to save those bits that are at right
> + * place and save them in result1. In second loop we do all operations
> + * symetrical, so we get other half of bits on their final spots, and save
> + * result in result2. Or of result1 and result2 is placed in appropriate
> + * doubleword element of vD. We repeat this 2 times.
> + */
> +static void trans_vgbbd(DisasContext *ctx)
> +{
> +    int VT = rD(ctx->opcode);
> +    int VB = rB(ctx->opcode);
> +    TCGv_i64 tmp = tcg_temp_new_i64();
> +    TCGv_i64 avr = tcg_temp_new_i64();
> +    TCGv_i64 shifted = tcg_temp_new_i64();
> +    TCGv_i64 result1 = tcg_temp_new_i64();
> +    TCGv_i64 result2 = tcg_temp_new_i64();
> +    uint64_t mask = 0x8040201008040201ULL;
> +    uint64_t mask1 = 0x80c0e0f0f8fcfeffULL;
> +    uint64_t mask2 = 0x7f3f1f0f07030100ULL;
> +    int i;
> +
> +    get_avr64(avr, VB, true);
> +    tcg_gen_movi_i64(result1, 0x0ULL);
> +    tcg_gen_mov_i64(shifted, avr);
> +    for (i = 0; i < 8; i++) {
> +        tcg_gen_andi_i64(tmp, shifted, mask);
> +        tcg_gen_or_i64(result1, result1, tmp);
> +
> +        tcg_gen_shri_i64(shifted, shifted, 7);
> +        mask = mask >> 8;
> +    }
> +    tcg_gen_andi_i64(result1, result1, mask1);

This masking appears to be redundant with the masking within the loop.

> +
> +    mask = 0x8040201008040201ULL;
> +    tcg_gen_movi_i64(result2, 0x0ULL);
> +    for (i = 0; i < 8; i++) {
> +        tcg_gen_andi_i64(tmp, avr, mask);
> +        tcg_gen_or_i64(result2, result2, tmp);
> +
> +        tcg_gen_shli_i64(avr, avr, 7);
> +        mask = mask << 8;
> +    }
> +    tcg_gen_andi_i64(result2, result2, mask2);

Similarly.

Also, the first iteration of the second loop is redundant with the first
iteration of the first loop.

I will also note that these are large constants, not easily constructable.
Therefore it would be best to avoid needing to construct them twice.  You can
do this by processing the two doublewords simultaneously.  e.g.

	TCGv_i64 avr[2], out[2], tmp, tcg_mask;

	identity_mask = 0x8040201008040201ull;
	tcg_gen_movi_i64(tcg_mask, identity_mask);
	for (j = 0; j < 2; j++) {
	    get_avr(avr[j], VB, j);
	    tcg_gen_and_i64(out[j], avr[j], tcg_mask);
	}
	for (i = 1; i < 8; i++) {
	    tcg_gen_movi_i64(tcg_mask, identity_mask >> (i * 8);
	    for (j = 0; j < 2; j++) {
	        tcg_gen_shri_i64(tmp, avr[j], i * 7);
	        tcg_gen_and_i64(tmp, tmp, tcg_mask);
	        tcg_gen_or_i64(out[j], out[j], tmp);
	    }
	}
	for (i = 1; i < 8; i++) {
	    tcg_gen_movi_i64(tcg_mask, identity_mask << (i * 8));
	    for (j = 0; j < 2; j++) {
	        tcg_gen_shli_i64(tmp, avr[j], i * 7);
	        tcg_gen_and_i64(tmp, tmp, tcg_mask);
	        tcg_gen_or_i64(out[j], out[j], tmp);
	    }
	}
	for (j = 0; j < 2; j++) {
	    set_avr(VT, out[j], j);
	}

This should produce the same results with fewer operations.


r~