Re: [PATCH v4 28/31] target/ppc: Implement setbc/setbcr/stnbc/setnbcr instructions

["Matheus K. Ferst" <matheus.ferst@eldorado.org.br>]

On 13/05/2021 08:01, Richard Henderson wrote:
> On 5/12/21 1:54 PM, matheus.ferst@eldorado.org.br wrote:
>> +static bool do_set_bool_cond(DisasContext *ctx, arg_X_bi *a, bool 
>> neg, bool rev)
>> +{
>> +    REQUIRE_INSNS_FLAGS2(ctx, ISA310);
>> +    uint32_t mask = 0x08 >> (a->bi & 0x03);
>> +    TCGv temp = tcg_temp_new();
>> +
>> +    tcg_gen_extu_i32_tl(temp, cpu_crf[a->bi >> 2]);
>> +    tcg_gen_andi_tl(temp, temp, mask);
>> +    tcg_gen_movcond_tl(a->r?TCG_COND_EQ:TCG_COND_NE, cpu_gpr[a->rt], 
>> temp,
>> +                       tcg_constant_tl(0), tcg_constant_tl(a->n?-1:1),
>> +                       tcg_constant_tl(0));
> 
> Mind the spacing around ?:.
> 

Fixed.

> Did you forget to update a->r and a->n to "neg" and "rev"?
> It sure looks like this doesn't compile...
> 

I messed up when rebasing, the change is in the next patch. I'll fix 
that too.

> I guess this is fine with movcond, but perhaps slightly better with
> 
>    tcg_gen_setcondi_tl(cond, rt, temp, 0);
>    if (neg) {
>      tcg_gen_neg_tl(rt, rt);
>    }
> 
> TCG isn't the most optimizing of compilers...
> 
> 
> r~

And also looks cleaner, I'll apply that too.

-- 
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>