Re: [Qemu-devel] fix evsrwu and others for powerpcspe

Re: [Qemu-devel] fix evsrwu and others for powerpcspe

[Michael Tokarev]

26.12.2017 14:29, info@mobile-stream.com wrote:
> GEN_SPEOP_ARITH2 declares non-local temporaries then calls gen_op_evsrwu/evsrws/evslw which generates branch insn.

I'm Cc'ing qemu-devel@ for review, since I don't know much about how ppc works.

Thanks,

/mjt

> --- target/ppc/translate/spe-impl.inc.c.orig
> +++ target/ppc/translate/spe-impl.inc.c
> @@ -158,8 +158,8 @@
>          gen_exception(ctx, POWERPC_EXCP_SPEU);                                \
>          return;                                                               \
>      }                                                                         \
> -    t0 = tcg_temp_new_i32();                                                  \
> -    t1 = tcg_temp_new_i32();                                                  \
> +    t0 = tcg_temp_local_new_i32();                                                  \
> +    t1 = tcg_temp_local_new_i32();                                                  \
>                                                                                \
>      tcg_gen_trunc_tl_i32(t0, cpu_gpr[rA(ctx->opcode)]);                       \
>      tcg_gen_trunc_tl_i32(t1, cpu_gpr[rB(ctx->opcode)]);                       \
> 
> 

Re: [Qemu-devel] fix evsrwu and others for powerpcspe

[Thomas Huth]

On 10.02.2018 08:25, Michael Tokarev wrote:
> 26.12.2017 14:29, info@mobile-stream.com wrote:
>> GEN_SPEOP_ARITH2 declares non-local temporaries then calls gen_op_evsrwu/evsrws/evslw which generates branch insn.
> 
> I'm Cc'ing qemu-devel@ for review, since I don't know much about how ppc works.

For PPC related stuff, please also always send a CC: to
qemu-ppc@nongnu.org (or even better, use the scripts/get_maintainer.pl
script to get the CC: list right).

Also, your patch is missing a proper "Signed-off-by" line. Please have a
look at https://wiki.qemu.org/Contribute/SubmitAPatch for details.

 Thanks,
  Thomas

> 
>> --- target/ppc/translate/spe-impl.inc.c.orig
>> +++ target/ppc/translate/spe-impl.inc.c
>> @@ -158,8 +158,8 @@
>>          gen_exception(ctx, POWERPC_EXCP_SPEU);                                \
>>          return;                                                               \
>>      }                                                                         \
>> -    t0 = tcg_temp_new_i32();                                                  \
>> -    t1 = tcg_temp_new_i32();                                                  \
>> +    t0 = tcg_temp_local_new_i32();                                                  \
>> +    t1 = tcg_temp_local_new_i32();                                                  \
>>                                                                                \
>>      tcg_gen_trunc_tl_i32(t0, cpu_gpr[rA(ctx->opcode)]);                       \
>>      tcg_gen_trunc_tl_i32(t1, cpu_gpr[rB(ctx->opcode)]);                       \
>>
>>
> 
> 

Re: [Qemu-devel] fix evsrwu and others for powerpcspe

[Richard Henderson]

On 02/09/2018 11:25 PM, Michael Tokarev wrote:
> 26.12.2017 14:29, info@mobile-stream.com wrote:
>> GEN_SPEOP_ARITH2 declares non-local temporaries then calls gen_op_evsrwu/evsrws/evslw which generates branch insn.
> 
> I'm Cc'ing qemu-devel@ for review, since I don't know much about how ppc works.
> 
> Thanks,
> 
> /mjt
> 
>> --- target/ppc/translate/spe-impl.inc.c.orig
>> +++ target/ppc/translate/spe-impl.inc.c
>> @@ -158,8 +158,8 @@
>>          gen_exception(ctx, POWERPC_EXCP_SPEU);                                \
>>          return;                                                               \
>>      }                                                                         \
>> -    t0 = tcg_temp_new_i32();                                                  \
>> -    t1 = tcg_temp_new_i32();                                                  \
>> +    t0 = tcg_temp_local_new_i32();                                                  \
>> +    t1 = tcg_temp_local_new_i32();                                                  \

While that's one possible solution, it would be better to rewrite the helpers
not to use branches.  E.g.

    tcg_gen_andi_i32(t0, arg2, 0x1f);
    tcg_gen_andi_i32(t1, arg2, 0x20);
    tcg_gen_shr_i32(ret, arg1, t0);
    tcg_gen_movi_i32(t0, 0);
    tcg_gen_movcond_i32(TCG_COND_NE, ret, t1, t0, t0, ret);


r~