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

[Matheus K. Ferst]

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>