Re: [RFC 2/3] target/ppc: Add IBM PPE42 special instructions

[Richard Henderson <richard.henderson@linaro.org>]

On 8/15/25 08:37, Glenn Miles wrote:
> Adds the following instructions exclusively for
> IBM PPE42 processors:
> 
>    LSKU
>    LCXU
>    STSKU
>    STCXU
>    LVD
>    LVDU
>    LVDX
>    STVD
>    STVDU
>    STVDX
>    SLVD
>    SRVD
>    CMPWBC
>    CMPLWBC
>    CMPWIBC
>    BNBWI
>    BNBW
>    CLRBWIBC
>    CLRWBC
>    DCBQ
>    RLDICL
>    RLDICR
>    RLDIMI
> 
> A PPE42 GCC compiler is available here:
> https://github.com/open-power/ppe42-gcc
> 
> For more information on the PPE42 processors please visit:
> https://wiki.raptorcs.com/w/images/a/a3/PPE_42X_Core_Users_Manual.pdf
> 
> Signed-off-by: Glenn Miles <milesg@linux.ibm.com>
> ---
>   target/ppc/helper_regs.c            |  15 +-
>   target/ppc/insn32.decode            |  66 ++-
>   target/ppc/translate.c              |  29 +-
>   target/ppc/translate/ppe-impl.c.inc | 805 ++++++++++++++++++++++++++++
>   4 files changed, 898 insertions(+), 17 deletions(-)
>   create mode 100644 target/ppc/translate/ppe-impl.c.inc

Oof.  This is really too much at once -- the patch ought to be split.

> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index e53fd2840d..8beb588a2a 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -58,6 +58,10 @@
>   %ds_rtp         22:4   !function=times_2
>   @DS_rtp         ...... ....0 ra:5 .............. ..             &D rt=%ds_rtp si=%ds_si
>   
> +%dd_si          3:s13

Note that the format describes this as shifted left by 3.  I see you multiply by 8 in 
trans_LSKU, but it's generally easier to have the immediates match the format description.

> +&DD             rt ra si:int64_t

Why are you forcing si to int64_t?  It's a 16-bit signed immediate.

> +static bool trans_LCXU(DisasContext *ctx, arg_LCXU *a)
> +{
> +#if defined(TARGET_PPC64)
> +    return false;
> +#else
> +    int i;
> +    TCGv base, EA;
> +    TCGv lo, hi;
> +    TCGv_i64 t8;
> +    const uint8_t vd_list[] = {9, 7, 5, 3, 0};

We're trying to move away from having separate binaries for different cpu models.  While 
target/ppc/ needs quite a bit of work for that to be a reality, let's try to avoid adding 
more code that needs cleaning up.

> +
> +    if (unlikely(!is_ppe(ctx))) {
> +        return false;
> +    }

This test should be sufficient to disable the insn for ppc64, without the ifdef.


r~