Re: [PATCH v2 11/15] target/ppc: Move D/DS/X-form integer loads to decodetree

[Matheus K. Ferst]

On 27/04/2021 14:16, Luis Pires wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
>
> These are all connected by macros in the legacy decoding.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
>   target/ppc/insn32.decode                   |  37 ++++
>   target/ppc/translate.c                     | 136 ++-------------
>   target/ppc/translate/fixedpoint-impl.c.inc | 188 +++++++++++++++++++++
>   3 files changed, 238 insertions(+), 123 deletions(-)
>
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index 878d2f2f66..bf39ce5c15 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -20,6 +20,43 @@
>   &D              rt ra si
>   @D              ...... rt:5 ra:5 si:s16                 &D
>   
> +%ds_si          2:s14  !function=times_4
> +@DS             ...... rt:5 ra:5 .............. ..      &D si=%ds_si
> +
> +&X              rt ra rb
> +@X              ...... rt:5 ra:5 rb:5 .......... .      &X
> +

This is a bit problematic, the instruction form isn't enough to decide its
fields. Eg. setb is X-form, but the fields are rt:5 bfa:3, setbc is also 
X-form
and the fields are rt:5 ba:5. In fact, for the X-form, there is a whole 
page of
field designations in PowerISA v3.1.

I would break this into three cases:
 - Some forms have single field designations. Eg. B, XLS, XX4;
 - Others have multiple designations but are just alternative names for the
   fields. Eg. DQ, DS, M;
 - And there are forms with multiple designations, with a variable 
number of
   fields that may overlap each other. Eg. X, XFX, XX2.

The first is a non-issue, just use the form name as done here. The 
second seems
tractable, we could pick one field name for each part of the insn and 
still use
the form name as the identifier for args_def/fmt_def. The last case will 
likely
require multiple fmt_defs for each form, in which case we would need to 
come up
with a pattern to name them.

Looking at what Binutils did when they added Power10 support, it seems 
that the
insn form is just a hint for opcode positions, and fields are specified 
for each
insn. The sad part of this kind of approach is that it would leave us 
with, eg.
arg_LBZX and arg_LBZUX instead of a single arg_X, making it harder to put
multiple insns under the same implementation.

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>