Re: [PATCH v2 09/15] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI

[Matheus K. Ferst]

On 27/04/2021 14:16, Luis Pires wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/ppc/insn32.decode                   |  8 +++++
>   target/ppc/insn64.decode                   | 14 ++++++++
>   target/ppc/translate.c                     | 29 ---------------
>   target/ppc/translate/fixedpoint-impl.c.inc | 42 ++++++++++++++++++++++
>   4 files changed, 64 insertions(+), 29 deletions(-)
>
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index b175441209..878d2f2f66 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -16,3 +16,11 @@
>   # You should have received a copy of the GNU Lesser General Public
>   # License along with this library; if not, see <http://www.gnu.org/licenses/>.
>   #
> +
> +&D              rt ra si
> +@D              ...... rt:5 ra:5 si:s16                 &D
> +
> +### Fixed-Point Arithmetic Instructions
> +
> +ADDI            001110 ..... ..... ................     @D
> +ADDIS           001111 ..... ..... ................     @D
> diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
> index 9fc45d0614..68ed2cbff8 100644
> --- a/target/ppc/insn64.decode
> +++ b/target/ppc/insn64.decode
> @@ -16,3 +16,17 @@
>   # You should have received a copy of the GNU Lesser General Public
>   # License along with this library; if not, see <http://www.gnu.org/licenses/>.
>   #
> +
> +# Format MLS:D and 8LS:D
> +&PLS_D          rt ra si r
> +
> +%pls_si         32:s18 0:16
> +
> +@PLS_D          ...... .. ... r:1 .. .................. \
> +                ...... rt:5 ra:5 ................       \
> +                &PLS_D si=%pls_si
> +
> +### Fixed-Point Arithmetic Instructions
> +
> +PADDI           000001 10 0--.-- ..................     \
> +                001110 ..... ..... ................     @PLS_D

<snip>

> +
> +static bool trans_ADDI(DisasContext *ctx, arg_D *a)
> +{
> +    if (a->ra) {
> +        tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], a->si);
> +    } else {
> +        tcg_gen_movi_tl(cpu_gpr[a->rt], a->si);
> +    }
> +    return true;
> +}
> +
> +static bool trans_ADDIS(DisasContext *ctx, arg_D *a)
> +{
> +    a->si <<= 16;
> +    return trans_ADDI(ctx, a);
> +}
> +
> +static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a)
> +{
> +    if (!resolve_PLS_D(ctx, a)) {
> +        return false;
> +    }
> +    if (a->ra) {
> +        tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], a->si);
> +    } else {
> +        tcg_gen_movi_tl(cpu_gpr[a->rt], a->si);
> +    }
> +    return true;
> +}

In our first attempt, we did some efforts to keep prefixed instructions 
type 0b10 and 0b11 under the same implementation as their word-size 
counterpart, i.e. trans_ADDI and trans_PADDI had the same signature and 
just forwarded their arguments to a third method that does the real 
work. Is this kind of approach desirable? We initially achieved this by 
using const_elt to set r=0 for addi, which is not particularly nice, but 
we can look for other solutions.

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>