Re: [Qemu-devel] [PATCH 1/6] target-ppc: add vmul10[u, eu, cu, ecu]q instructions

[Rajalakshmi Srinivasaraghavan <raji@linux.vnet.ibm.com>]

On 09/28/2016 10:12 PM, Richard Henderson wrote:
> On 09/27/2016 10:45 PM, Rajalakshmi Srinivasaraghavan wrote:
>> +    val = tcg_const_i64(10);                                            \
> Rename this "ten" for clarity?
>
>> +    z = tcg_const_i64(0);                                               \
>> +                                                                        \
>> +    if (add_cin) {                                                      \
>> +        tcg_gen_andi_i64(cin, cpu_avrl[rB(ctx->opcode)], 0xF);          \
>> +        tcg_gen_movcond_i64(TCG_COND_LTU, cin, cin, val, cin, z);       \
> What is the purpose of this movcond?  The docs specifically say that values
> greater than 9 are undefined.
>
>> +    } else {                                                            \
>> +        tcg_gen_movi_i64(cin, 0);                                       \
>> +    }                                                                   \
>> +                                                                        \
>> +    tcg_gen_mulu2_i64(t0, t1, cpu_avrl[rA(ctx->opcode)], val);          \
>> +    tcg_gen_add2_i64(cpu_avrl[rD(ctx->opcode)], t2, t0, z, cin, z);     \
>> +    tcg_gen_add2_i64(t2, t0, t1, z, t2, z);                             \
> This two additions are unused if !add_cin, and the second appears to be
> mergable with the first -- don't use so many z's.  I think this simplifies to
>
>    if (add_cin) {
>       tcg_gen_mulu2_i64(t0, t1, cpu_avrl[rA(ctx->opcode)], ten);
>       tcg_gen_andi_i64(t2, cpu_avrl[rB(ctx->opcode)], 0xF);
>       tcg_gen_add2_i64(cpu_avrl[rD(ctx->opcode)], t2, t0, t1, t2, z);
>    } else {
>       tcg_gen_mulu2_i64(cpu_avrl[rD(ctx->opcode)], t2,
>                         cpu_avrl[rA(ctx->opcode)], ten);
>    }
>
>> +    tcg_gen_mulu2_i64(t0, t1, cpu_avrh[rA(ctx->opcode)], val);          \
>> +    tcg_gen_add2_i64(cpu_avrh[rD(ctx->opcode)], t2, t0, z, t2, z);      \
>> +                                                                        \
>> +    if (ret_carry) {                                                    \
>> +        tcg_gen_add2_i64(cpu_avrl[rD(ctx->opcode)], t0, t1, z, t2, z);  \
>> +        tcg_gen_movi_i64(cpu_avrh[rD(ctx->opcode)], 0);                 \
> Likewise simplifies to
>
>    if (ret_carry) {
>       tcg_gen_mulu2_i64(t0, t1, cpu_avrh[rA(ctx->opcode)], ten);
>       tcg_gen_add2_i64(t0, cpu_avrl[rD(ctx->opcode)], t0, t1, t2, z);
>       tcg_gen_movi_i64(cpu_avrh[rD(ctx->opcode)], 0);
>    } else {
>       tcg_gen_mul_i64(t0, cpu_avrh[rA(ctx->opcode)], ten);
>       tcg_gen_add_i64(cpu_avrh[rD(ctx->opcode)], t0, t2);
>    }
>
Will check and send updated patch.
> r~
>
>

-- 
Thanks
Rajalakshmi S