Re: [PATCH v3 25/30] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI

[Richard Henderson]

On 4/30/21 7:05 AM, Matheus K. Ferst wrote:
> > +ADDI            000001 10 0--.-- ..................     \
> > +                001110 ..... ..... ................     @PLS_D
>
> I'm not sure about this. It's a bit surprising to find ADDI here, and the 
> comment that explains why is likely to be ignored after the big copyright header.

You could move the comment closer, and replicate, e.g.

ADDI .... \
     .... @PLS_D # PADDI


> I'd prefer to keep a trans_PADDI like
>
>  > static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a)
>  > {
>  >     if(!resolve_PLS_D(ctx, a)) {
>  >         return false;
>  >     }
>  >     return trans_ADDI(ctx, a);
>  > }

But in this case ADDI probably doesn't use PLS_D.  You could use

static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a)
{
    arg_D d;
    if (!resolve_PLS_D(ctx, &d, a)) {
        return false;
    }
    return trans_ADDI(ctx, &d);
}

making sure to use int64_t in the offset for arg_D.

> It's the middle way between v2 and v3. trans_ADDI code is reused, it'll 
> probably be optimized as a tail call, and resolve_PLS_D is not called when it's 
> not needed.

My version won't tail-call, because of the escaping local storage, but I don't 
see how you can avoid it.


r~