On 30/04/2021 11:31, Richard Henderson wrote:
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
If we keep this naming, IMHO moving the comment closer looks better.
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.
We'd keep trans_ADDI with the same signature to avoid creating an arg_D
on the stack. Patch 4 added type specification, maybe we can define an
arg_D within arg_PLD_D? I'll play a bit to see if it works.
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~
I haven't been able to test it properly yet, but at least on godbolt it
seems that the compiler prefers to inline over tail call, so maybe
that's not a problem.
Thanks,
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>
