[RFC PATCH] target/ppc: Move add and subf type fixed-point arithmetic instructions to decodetree

Chinmay Rath posted 1 patch 2 weeks, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240209113532.580983-1-rathc@linux.ibm.com
Maintainers: Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>
There is a newer version of this series
target/ppc/insn32.decode                   |  26 ++++
target/ppc/translate.c                     | 136 ---------------------
target/ppc/translate/fixedpoint-impl.c.inc |  69 +++++++++++
3 files changed, 95 insertions(+), 136 deletions(-)
[RFC PATCH] target/ppc: Move add and subf type fixed-point arithmetic instructions to decodetree
Posted by Chinmay Rath 2 weeks, 1 day ago
This patch moves the below instructions to decodetree specification:

        {add, subf}[c,e,me,ze][o][.]       : XO-form
        addic[.], subfic                   : D-form
        addex                              : Z23-form

This patch introduces XO form instructions into decode tree specification, for
which all the four variations([o][.]) have been handled with a single pattern.
The changes were verified by validating that the tcg ops generated by those
instructions remain the same, which were captured with the '-d in_asm,op' flag.

Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
---
 target/ppc/insn32.decode                   |  26 ++++
 target/ppc/translate.c                     | 136 ---------------------
 target/ppc/translate/fixedpoint-impl.c.inc |  69 +++++++++++
 3 files changed, 95 insertions(+), 136 deletions(-)

diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 4fcf3af8d0..ddaa47210a 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -187,6 +187,12 @@
 &X_a            ra
 @X_a            ...... ra:3 .. ..... ..... .......... .         &X_a
 
+&XO             rt ra rb oe:bool rc:bool
+@XO             ...... rt:5 ra:5 rb:5 oe:1 ......... rc:1       &XO
+
+&XO_ta          rt ra oe:bool rc:bool
+@XO_ta          ...... rt:5 ra:5 ..... oe:1 ......... rc:1      &XO_ta
+
 %xx_xt          0:1 21:5
 %xx_xb          1:1 11:5
 %xx_xa          2:1 16:5
@@ -239,6 +245,9 @@
 &Z23_tab        frt fra frb rmc rc:bool
 @Z23_tab        ...... frt:5 fra:5 frb:5 rmc:2 ........ rc:1    &Z23_tab
 
+&Z23_tab_cy     rt ra rb cy
+@Z23_tab_cy     ...... rt:5 ra:5 rb:5 cy:2 ........ .           &Z23_tab_cy
+
 %z23_frtp       22:4 !function=times_2
 %z23_frap       17:4 !function=times_2
 %z23_frbp       12:4 !function=times_2
@@ -322,10 +331,27 @@ CMPLI           001010 ... - . ..... ................   @D_bfu
 
 ### Fixed-Point Arithmetic Instructions
 
+ADD             011111 ..... ..... ..... . 100001010 .  @XO
+ADDC            011111 ..... ..... ..... . 000001010 .  @XO
+ADDE            011111 ..... ..... ..... . 010001010 .  @XO
+ADDEX           011111 ..... ..... ..... .. 10101010 -  @Z23_tab_cy
+
 ADDI            001110 ..... ..... ................     @D
 ADDIS           001111 ..... ..... ................     @D
+ADDIC           001100 ..... ..... ................     @D
+ADDIC_          001101 ..... ..... ................     @D
 
 ADDPCIS         010011 ..... ..... .......... 00010 .   @DX
+ADDME           011111 ..... ..... ----- . 011101010 .  @XO_ta
+ADDZE           011111 ..... ..... ----- . 011001010 .  @XO_ta
+
+SUBF            011111 ..... ..... ..... . 000101000 .  @XO
+SUBFIC          001000 ..... ..... ................     @D
+SUBFC           011111 ..... ..... ..... . 000001000 .  @XO
+SUBFE           011111 ..... ..... ..... . 010001000 .  @XO
+
+SUBFME          011111 ..... ..... ----- . 011101000 .  @XO_ta
+SUBFZE          011111 ..... ..... ----- . 011001000 .  @XO_ta
 
 ## Fixed-Point Logical Instructions
 
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 049f636927..51dc1e79cc 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1697,61 +1697,6 @@ static inline void gen_op_arith_add(DisasContext *ctx, TCGv ret, TCGv arg1,
         tcg_gen_mov_tl(ret, t0);
     }
 }
-/* Add functions with two operands */
-#define GEN_INT_ARITH_ADD(name, opc3, ca, add_ca, compute_ca, compute_ov)     \
-static void glue(gen_, name)(DisasContext *ctx)                               \
-{                                                                             \
-    gen_op_arith_add(ctx, cpu_gpr[rD(ctx->opcode)],                           \
-                     cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],      \
-                     ca, glue(ca, 32),                                        \
-                     add_ca, compute_ca, compute_ov, Rc(ctx->opcode));        \
-}
-/* Add functions with one operand and one immediate */
-#define GEN_INT_ARITH_ADD_CONST(name, opc3, const_val, ca,                    \
-                                add_ca, compute_ca, compute_ov)               \
-static void glue(gen_, name)(DisasContext *ctx)                               \
-{                                                                             \
-    TCGv t0 = tcg_constant_tl(const_val);                                     \
-    gen_op_arith_add(ctx, cpu_gpr[rD(ctx->opcode)],                           \
-                     cpu_gpr[rA(ctx->opcode)], t0,                            \
-                     ca, glue(ca, 32),                                        \
-                     add_ca, compute_ca, compute_ov, Rc(ctx->opcode));        \
-}
-
-/* add  add.  addo  addo. */
-GEN_INT_ARITH_ADD(add, 0x08, cpu_ca, 0, 0, 0)
-GEN_INT_ARITH_ADD(addo, 0x18, cpu_ca, 0, 0, 1)
-/* addc  addc.  addco  addco. */
-GEN_INT_ARITH_ADD(addc, 0x00, cpu_ca, 0, 1, 0)
-GEN_INT_ARITH_ADD(addco, 0x10, cpu_ca, 0, 1, 1)
-/* adde  adde.  addeo  addeo. */
-GEN_INT_ARITH_ADD(adde, 0x04, cpu_ca, 1, 1, 0)
-GEN_INT_ARITH_ADD(addeo, 0x14, cpu_ca, 1, 1, 1)
-/* addme  addme.  addmeo  addmeo.  */
-GEN_INT_ARITH_ADD_CONST(addme, 0x07, -1LL, cpu_ca, 1, 1, 0)
-GEN_INT_ARITH_ADD_CONST(addmeo, 0x17, -1LL, cpu_ca, 1, 1, 1)
-/* addex */
-GEN_INT_ARITH_ADD(addex, 0x05, cpu_ov, 1, 1, 0);
-/* addze  addze.  addzeo  addzeo.*/
-GEN_INT_ARITH_ADD_CONST(addze, 0x06, 0, cpu_ca, 1, 1, 0)
-GEN_INT_ARITH_ADD_CONST(addzeo, 0x16, 0, cpu_ca, 1, 1, 1)
-/* addic  addic.*/
-static inline void gen_op_addic(DisasContext *ctx, bool compute_rc0)
-{
-    TCGv c = tcg_constant_tl(SIMM(ctx->opcode));
-    gen_op_arith_add(ctx, cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
-                     c, cpu_ca, cpu_ca32, 0, 1, 0, compute_rc0);
-}
-
-static void gen_addic(DisasContext *ctx)
-{
-    gen_op_addic(ctx, 0);
-}
-
-static void gen_addic_(DisasContext *ctx)
-{
-    gen_op_addic(ctx, 1);
-}
 
 static inline void gen_op_arith_divw(DisasContext *ctx, TCGv ret, TCGv arg1,
                                      TCGv arg2, int sign, int compute_ov)
@@ -2172,47 +2117,6 @@ static inline void gen_op_arith_subf(DisasContext *ctx, TCGv ret, TCGv arg1,
         tcg_gen_mov_tl(ret, t0);
     }
 }
-/* Sub functions with Two operands functions */
-#define GEN_INT_ARITH_SUBF(name, opc3, add_ca, compute_ca, compute_ov)        \
-static void glue(gen_, name)(DisasContext *ctx)                               \
-{                                                                             \
-    gen_op_arith_subf(ctx, cpu_gpr[rD(ctx->opcode)],                          \
-                      cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],     \
-                      add_ca, compute_ca, compute_ov, Rc(ctx->opcode));       \
-}
-/* Sub functions with one operand and one immediate */
-#define GEN_INT_ARITH_SUBF_CONST(name, opc3, const_val,                       \
-                                add_ca, compute_ca, compute_ov)               \
-static void glue(gen_, name)(DisasContext *ctx)                               \
-{                                                                             \
-    TCGv t0 = tcg_constant_tl(const_val);                                     \
-    gen_op_arith_subf(ctx, cpu_gpr[rD(ctx->opcode)],                          \
-                      cpu_gpr[rA(ctx->opcode)], t0,                           \
-                      add_ca, compute_ca, compute_ov, Rc(ctx->opcode));       \
-}
-/* subf  subf.  subfo  subfo. */
-GEN_INT_ARITH_SUBF(subf, 0x01, 0, 0, 0)
-GEN_INT_ARITH_SUBF(subfo, 0x11, 0, 0, 1)
-/* subfc  subfc.  subfco  subfco. */
-GEN_INT_ARITH_SUBF(subfc, 0x00, 0, 1, 0)
-GEN_INT_ARITH_SUBF(subfco, 0x10, 0, 1, 1)
-/* subfe  subfe.  subfeo  subfo. */
-GEN_INT_ARITH_SUBF(subfe, 0x04, 1, 1, 0)
-GEN_INT_ARITH_SUBF(subfeo, 0x14, 1, 1, 1)
-/* subfme  subfme.  subfmeo  subfmeo.  */
-GEN_INT_ARITH_SUBF_CONST(subfme, 0x07, -1LL, 1, 1, 0)
-GEN_INT_ARITH_SUBF_CONST(subfmeo, 0x17, -1LL, 1, 1, 1)
-/* subfze  subfze.  subfzeo  subfzeo.*/
-GEN_INT_ARITH_SUBF_CONST(subfze, 0x06, 0, 1, 1, 0)
-GEN_INT_ARITH_SUBF_CONST(subfzeo, 0x16, 0, 1, 1, 1)
-
-/* subfic */
-static void gen_subfic(DisasContext *ctx)
-{
-    TCGv c = tcg_constant_tl(SIMM(ctx->opcode));
-    gen_op_arith_subf(ctx, cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
-                      c, 0, 1, 0, 0);
-}
 
 /* neg neg. nego nego. */
 static inline void gen_op_arith_neg(DisasContext *ctx, bool compute_ov)
@@ -6486,8 +6390,6 @@ GEN_HANDLER_E(cmpeqb, 0x1F, 0x00, 0x07, 0x00600000, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER_E(cmpb, 0x1F, 0x1C, 0x0F, 0x00000001, PPC_NONE, PPC2_ISA205),
 GEN_HANDLER_E(cmprb, 0x1F, 0x00, 0x06, 0x00400001, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER(isel, 0x1F, 0x0F, 0xFF, 0x00000001, PPC_ISEL),
-GEN_HANDLER(addic, 0x0C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
-GEN_HANDLER2(addic_, "addic.", 0x0D, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(mulhw, 0x1F, 0x0B, 0x02, 0x00000400, PPC_INTEGER),
 GEN_HANDLER(mulhwu, 0x1F, 0x0B, 0x00, 0x00000400, PPC_INTEGER),
 GEN_HANDLER(mullw, 0x1F, 0x0B, 0x07, 0x00000000, PPC_INTEGER),
@@ -6498,7 +6400,6 @@ GEN_HANDLER(mulld, 0x1F, 0x09, 0x07, 0x00000000, PPC_64B),
 #endif
 GEN_HANDLER(neg, 0x1F, 0x08, 0x03, 0x0000F800, PPC_INTEGER),
 GEN_HANDLER(nego, 0x1F, 0x08, 0x13, 0x0000F800, PPC_INTEGER),
-GEN_HANDLER(subfic, 0x08, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER2(andi_, "andi.", 0x1C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER2(andis_, "andis.", 0x1D, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(cntlzw, 0x1F, 0x1A, 0x00, 0x00000000, PPC_INTEGER),
@@ -6709,25 +6610,6 @@ GEN_HANDLER_E(maddhd_maddhdu, 0x04, 0x18, 0xFF, 0x00000000, PPC_NONE,
 GEN_HANDLER_E(maddld, 0x04, 0x19, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300),
 #endif
 
-#undef GEN_INT_ARITH_ADD
-#undef GEN_INT_ARITH_ADD_CONST
-#define GEN_INT_ARITH_ADD(name, opc3, add_ca, compute_ca, compute_ov)         \
-GEN_HANDLER(name, 0x1F, 0x0A, opc3, 0x00000000, PPC_INTEGER),
-#define GEN_INT_ARITH_ADD_CONST(name, opc3, const_val,                        \
-                                add_ca, compute_ca, compute_ov)               \
-GEN_HANDLER(name, 0x1F, 0x0A, opc3, 0x0000F800, PPC_INTEGER),
-GEN_INT_ARITH_ADD(add, 0x08, 0, 0, 0)
-GEN_INT_ARITH_ADD(addo, 0x18, 0, 0, 1)
-GEN_INT_ARITH_ADD(addc, 0x00, 0, 1, 0)
-GEN_INT_ARITH_ADD(addco, 0x10, 0, 1, 1)
-GEN_INT_ARITH_ADD(adde, 0x04, 1, 1, 0)
-GEN_INT_ARITH_ADD(addeo, 0x14, 1, 1, 1)
-GEN_INT_ARITH_ADD_CONST(addme, 0x07, -1LL, 1, 1, 0)
-GEN_INT_ARITH_ADD_CONST(addmeo, 0x17, -1LL, 1, 1, 1)
-GEN_HANDLER_E(addex, 0x1F, 0x0A, 0x05, 0x00000000, PPC_NONE, PPC2_ISA300),
-GEN_INT_ARITH_ADD_CONST(addze, 0x06, 0, 1, 1, 0)
-GEN_INT_ARITH_ADD_CONST(addzeo, 0x16, 0, 1, 1, 1)
-
 #undef GEN_INT_ARITH_DIVW
 #define GEN_INT_ARITH_DIVW(name, opc3, sign, compute_ov)                      \
 GEN_HANDLER(name, 0x1F, 0x0B, opc3, 0x00000000, PPC_INTEGER)
@@ -6766,24 +6648,6 @@ GEN_INT_ARITH_MUL_HELPER(mulhd, 0x02),
 GEN_INT_ARITH_MUL_HELPER(mulldo, 0x17),
 #endif
 
-#undef GEN_INT_ARITH_SUBF
-#undef GEN_INT_ARITH_SUBF_CONST
-#define GEN_INT_ARITH_SUBF(name, opc3, add_ca, compute_ca, compute_ov)        \
-GEN_HANDLER(name, 0x1F, 0x08, opc3, 0x00000000, PPC_INTEGER),
-#define GEN_INT_ARITH_SUBF_CONST(name, opc3, const_val,                       \
-                                add_ca, compute_ca, compute_ov)               \
-GEN_HANDLER(name, 0x1F, 0x08, opc3, 0x0000F800, PPC_INTEGER),
-GEN_INT_ARITH_SUBF(subf, 0x01, 0, 0, 0)
-GEN_INT_ARITH_SUBF(subfo, 0x11, 0, 0, 1)
-GEN_INT_ARITH_SUBF(subfc, 0x00, 0, 1, 0)
-GEN_INT_ARITH_SUBF(subfco, 0x10, 0, 1, 1)
-GEN_INT_ARITH_SUBF(subfe, 0x04, 1, 1, 0)
-GEN_INT_ARITH_SUBF(subfeo, 0x14, 1, 1, 1)
-GEN_INT_ARITH_SUBF_CONST(subfme, 0x07, -1LL, 1, 1, 0)
-GEN_INT_ARITH_SUBF_CONST(subfmeo, 0x17, -1LL, 1, 1, 1)
-GEN_INT_ARITH_SUBF_CONST(subfze, 0x06, 0, 1, 1, 0)
-GEN_INT_ARITH_SUBF_CONST(subfzeo, 0x16, 0, 1, 1, 1)
-
 #undef GEN_LOGICAL1
 #undef GEN_LOGICAL2
 #define GEN_LOGICAL2(name, tcg_op, opc, type)                                 \
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
index 51c6fa7330..12374b78cf 100644
--- a/target/ppc/translate/fixedpoint-impl.c.inc
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -325,6 +325,75 @@ static bool trans_ADDPCIS(DisasContext *ctx, arg_DX *a)
     return true;
 }
 
+static bool trans_ADDEX(DisasContext *ctx, arg_Z23_tab_cy *a)
+{
+    gen_op_arith_add(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], cpu_gpr[a->rb],
+                     cpu_ov, cpu_ov32, true, true, false, false);
+    return true;
+}
+
+static bool do_add_D(DisasContext *ctx, arg_D *a, bool add_ca, bool compute_ca,
+                     bool compute_ov, bool compute_rc0)
+{
+    gen_op_arith_add(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra],
+                     tcg_constant_tl(a->si), cpu_ca, cpu_ca32,
+                     add_ca, compute_ca, compute_ov, compute_rc0);
+    return true;
+}
+
+static bool do_add_XO(DisasContext *ctx, arg_XO *a, bool add_ca,
+                      bool compute_ca)
+{
+    gen_op_arith_add(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], cpu_gpr[a->rb],
+                     cpu_ca, cpu_ca32, add_ca, compute_ca, a->oe, a->rc);
+    return true;
+}
+
+static bool do_add_const_XO(DisasContext *ctx, arg_XO_ta *a, TCGv const_val,
+                            bool add_ca, bool compute_ca)
+{
+    gen_op_arith_add(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], const_val,
+                     cpu_ca, cpu_ca32, add_ca, compute_ca, a->oe, a->rc);
+    return true;
+}
+
+TRANS(ADD, do_add_XO, false, false);
+TRANS(ADDC, do_add_XO, false, true);
+TRANS(ADDE, do_add_XO, true, true);
+TRANS(ADDME, do_add_const_XO, tcg_constant_tl(-1LL), true, true);
+TRANS(ADDZE, do_add_const_XO, tcg_constant_tl(0), true, true);
+TRANS(ADDIC, do_add_D, false, true, false, false);
+TRANS(ADDIC_, do_add_D, false, true, false, true);
+
+static bool trans_SUBFIC(DisasContext *ctx, arg_D *a)
+{
+    gen_op_arith_subf(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra],
+                      tcg_constant_tl(a->si), false, true, false, false);
+    return true;
+}
+
+static bool do_subf_XO(DisasContext *ctx, arg_XO *a, bool add_ca,
+                       bool compute_ca)
+{
+    gen_op_arith_subf(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], cpu_gpr[a->rb],
+                      add_ca, compute_ca, a->oe, a->rc);
+    return true;
+}
+
+static bool do_subf_const_XO(DisasContext *ctx, arg_XO_ta *a, TCGv const_val,
+                             bool add_ca, bool compute_ca)
+{
+    gen_op_arith_subf(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], const_val,
+                      add_ca, compute_ca, a->oe, a->rc);
+    return true;
+}
+
+TRANS(SUBF, do_subf_XO, false, false)
+TRANS(SUBFC, do_subf_XO, false, true)
+TRANS(SUBFE, do_subf_XO, true, true)
+TRANS(SUBFME, do_subf_const_XO, tcg_constant_tl(-1LL), true, true)
+TRANS(SUBFZE, do_subf_const_XO, tcg_constant_tl(0), true, true)
+
 static bool trans_INVALID(DisasContext *ctx, arg_INVALID *a)
 {
     gen_invalid(ctx);
-- 
2.39.3
Re: [RFC PATCH] target/ppc: Move add and subf type fixed-point arithmetic instructions to decodetree
Posted by Richard Henderson 1 week, 5 days ago
On 2/9/24 01:35, Chinmay Rath wrote:
> +&Z23_tab_cy     rt ra rb cy
> +@Z23_tab_cy     ...... rt:5 ra:5 rb:5 cy:2 ........ .           &Z23_tab_cy
...
> +ADDEX           011111 ..... ..... ..... .. 10101010 -  @Z23_tab_cy
...
> +static bool trans_ADDEX(DisasContext *ctx, arg_Z23_tab_cy *a)
> +{
> +    gen_op_arith_add(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], cpu_gpr[a->rb],
> +                     cpu_ov, cpu_ov32, true, true, false, false);
> +    return true;
> +}

CY != 0 is reserved.

While you could diagnose this in trans_ADDEX, it seems cleaner to simply match 00 in the 
CY field until a future ISA defines something else.  All that is required is a comment in 
the decodetree entry.

# Z23-form, with CY=0; all other values for CY are reserved.
# This works out the same as X-form.
ADDEX    011111 ..... ..... ..... 00 10101010 -   @X


r~
Re: [RFC PATCH] target/ppc: Move add and subf type fixed-point arithmetic instructions to decodetree
Posted by Chinmay Rath 1 week, 4 days ago
Hi Richard,

On 2/13/24 03:51, Richard Henderson wrote:
> On 2/9/24 01:35, Chinmay Rath wrote:
>> +&Z23_tab_cy     rt ra rb cy
>> +@Z23_tab_cy     ...... rt:5 ra:5 rb:5 cy:2 ........ . &Z23_tab_cy
> ...
>> +ADDEX           011111 ..... ..... ..... .. 10101010 -  @Z23_tab_cy
> ...
>> +static bool trans_ADDEX(DisasContext *ctx, arg_Z23_tab_cy *a)
>> +{
>> +    gen_op_arith_add(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], 
>> cpu_gpr[a->rb],
>> +                     cpu_ov, cpu_ov32, true, true, false, false);
>> +    return true;
>> +}
>
> CY != 0 is reserved.
>
> While you could diagnose this in trans_ADDEX, it seems cleaner to 
> simply match 00 in the CY field until a future ISA defines something 
> else.  All that is required is a comment in the decodetree entry.
>
> # Z23-form, with CY=0; all other values for CY are reserved.
> # This works out the same as X-form.
> ADDEX    011111 ..... ..... ..... 00 10101010 -   @X
>
Thanks for your review comments.

I shall update as suggested in v2.

Regards,

Chinmay

>
> r~

Re: [RFC PATCH] target/ppc: Move add and subf type fixed-point arithmetic instructions to decodetree
Posted by Harsh Prateek Bora 1 week, 5 days ago
+ Daniel, Cedric, Richard - for review comments, if any.

On 2/9/24 17:05, Chinmay Rath wrote:
> This patch moves the below instructions to decodetree specification:
> 
>          {add, subf}[c,e,me,ze][o][.]       : XO-form
>          addic[.], subfic                   : D-form
>          addex                              : Z23-form
> 
> This patch introduces XO form instructions into decode tree specification, for
> which all the four variations([o][.]) have been handled with a single pattern.
> The changes were verified by validating that the tcg ops generated by those
> instructions remain the same, which were captured with the '-d in_asm,op' flag.
> 
> Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
> ---
>   target/ppc/insn32.decode                   |  26 ++++
>   target/ppc/translate.c                     | 136 ---------------------
>   target/ppc/translate/fixedpoint-impl.c.inc |  69 +++++++++++
>   3 files changed, 95 insertions(+), 136 deletions(-)
> 
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index 4fcf3af8d0..ddaa47210a 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -187,6 +187,12 @@
>   &X_a            ra
>   @X_a            ...... ra:3 .. ..... ..... .......... .         &X_a
>   
> +&XO             rt ra rb oe:bool rc:bool
> +@XO             ...... rt:5 ra:5 rb:5 oe:1 ......... rc:1       &XO
> +
> +&XO_ta          rt ra oe:bool rc:bool
> +@XO_ta          ...... rt:5 ra:5 ..... oe:1 ......... rc:1      &XO_ta
> +
>   %xx_xt          0:1 21:5
>   %xx_xb          1:1 11:5
>   %xx_xa          2:1 16:5
> @@ -239,6 +245,9 @@
>   &Z23_tab        frt fra frb rmc rc:bool
>   @Z23_tab        ...... frt:5 fra:5 frb:5 rmc:2 ........ rc:1    &Z23_tab
>   
> +&Z23_tab_cy     rt ra rb cy
> +@Z23_tab_cy     ...... rt:5 ra:5 rb:5 cy:2 ........ .           &Z23_tab_cy
> +
>   %z23_frtp       22:4 !function=times_2
>   %z23_frap       17:4 !function=times_2
>   %z23_frbp       12:4 !function=times_2
> @@ -322,10 +331,27 @@ CMPLI           001010 ... - . ..... ................   @D_bfu
>   
>   ### Fixed-Point Arithmetic Instructions
>   
> +ADD             011111 ..... ..... ..... . 100001010 .  @XO
> +ADDC            011111 ..... ..... ..... . 000001010 .  @XO
> +ADDE            011111 ..... ..... ..... . 010001010 .  @XO
> +ADDEX           011111 ..... ..... ..... .. 10101010 -  @Z23_tab_cy
> +
>   ADDI            001110 ..... ..... ................     @D
>   ADDIS           001111 ..... ..... ................     @D
> +ADDIC           001100 ..... ..... ................     @D
> +ADDIC_          001101 ..... ..... ................     @D
>   
>   ADDPCIS         010011 ..... ..... .......... 00010 .   @DX
> +ADDME           011111 ..... ..... ----- . 011101010 .  @XO_ta
> +ADDZE           011111 ..... ..... ----- . 011001010 .  @XO_ta
> +
> +SUBF            011111 ..... ..... ..... . 000101000 .  @XO
> +SUBFIC          001000 ..... ..... ................     @D
> +SUBFC           011111 ..... ..... ..... . 000001000 .  @XO
> +SUBFE           011111 ..... ..... ..... . 010001000 .  @XO
> +
> +SUBFME          011111 ..... ..... ----- . 011101000 .  @XO_ta
> +SUBFZE          011111 ..... ..... ----- . 011001000 .  @XO_ta
>   
>   ## Fixed-Point Logical Instructions
>   
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 049f636927..51dc1e79cc 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -1697,61 +1697,6 @@ static inline void gen_op_arith_add(DisasContext *ctx, TCGv ret, TCGv arg1,
>           tcg_gen_mov_tl(ret, t0);
>       }
>   }
> -/* Add functions with two operands */
> -#define GEN_INT_ARITH_ADD(name, opc3, ca, add_ca, compute_ca, compute_ov)     \
> -static void glue(gen_, name)(DisasContext *ctx)                               \
> -{                                                                             \
> -    gen_op_arith_add(ctx, cpu_gpr[rD(ctx->opcode)],                           \
> -                     cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],      \
> -                     ca, glue(ca, 32),                                        \
> -                     add_ca, compute_ca, compute_ov, Rc(ctx->opcode));        \
> -}
> -/* Add functions with one operand and one immediate */
> -#define GEN_INT_ARITH_ADD_CONST(name, opc3, const_val, ca,                    \
> -                                add_ca, compute_ca, compute_ov)               \
> -static void glue(gen_, name)(DisasContext *ctx)                               \
> -{                                                                             \
> -    TCGv t0 = tcg_constant_tl(const_val);                                     \
> -    gen_op_arith_add(ctx, cpu_gpr[rD(ctx->opcode)],                           \
> -                     cpu_gpr[rA(ctx->opcode)], t0,                            \
> -                     ca, glue(ca, 32),                                        \
> -                     add_ca, compute_ca, compute_ov, Rc(ctx->opcode));        \
> -}
> -
> -/* add  add.  addo  addo. */
> -GEN_INT_ARITH_ADD(add, 0x08, cpu_ca, 0, 0, 0)
> -GEN_INT_ARITH_ADD(addo, 0x18, cpu_ca, 0, 0, 1)
> -/* addc  addc.  addco  addco. */
> -GEN_INT_ARITH_ADD(addc, 0x00, cpu_ca, 0, 1, 0)
> -GEN_INT_ARITH_ADD(addco, 0x10, cpu_ca, 0, 1, 1)
> -/* adde  adde.  addeo  addeo. */
> -GEN_INT_ARITH_ADD(adde, 0x04, cpu_ca, 1, 1, 0)
> -GEN_INT_ARITH_ADD(addeo, 0x14, cpu_ca, 1, 1, 1)
> -/* addme  addme.  addmeo  addmeo.  */
> -GEN_INT_ARITH_ADD_CONST(addme, 0x07, -1LL, cpu_ca, 1, 1, 0)
> -GEN_INT_ARITH_ADD_CONST(addmeo, 0x17, -1LL, cpu_ca, 1, 1, 1)
> -/* addex */
> -GEN_INT_ARITH_ADD(addex, 0x05, cpu_ov, 1, 1, 0);
> -/* addze  addze.  addzeo  addzeo.*/
> -GEN_INT_ARITH_ADD_CONST(addze, 0x06, 0, cpu_ca, 1, 1, 0)
> -GEN_INT_ARITH_ADD_CONST(addzeo, 0x16, 0, cpu_ca, 1, 1, 1)
> -/* addic  addic.*/
> -static inline void gen_op_addic(DisasContext *ctx, bool compute_rc0)
> -{
> -    TCGv c = tcg_constant_tl(SIMM(ctx->opcode));
> -    gen_op_arith_add(ctx, cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
> -                     c, cpu_ca, cpu_ca32, 0, 1, 0, compute_rc0);
> -}
> -
> -static void gen_addic(DisasContext *ctx)
> -{
> -    gen_op_addic(ctx, 0);
> -}
> -
> -static void gen_addic_(DisasContext *ctx)
> -{
> -    gen_op_addic(ctx, 1);
> -}
>   
>   static inline void gen_op_arith_divw(DisasContext *ctx, TCGv ret, TCGv arg1,
>                                        TCGv arg2, int sign, int compute_ov)
> @@ -2172,47 +2117,6 @@ static inline void gen_op_arith_subf(DisasContext *ctx, TCGv ret, TCGv arg1,
>           tcg_gen_mov_tl(ret, t0);
>       }
>   }
> -/* Sub functions with Two operands functions */
> -#define GEN_INT_ARITH_SUBF(name, opc3, add_ca, compute_ca, compute_ov)        \
> -static void glue(gen_, name)(DisasContext *ctx)                               \
> -{                                                                             \
> -    gen_op_arith_subf(ctx, cpu_gpr[rD(ctx->opcode)],                          \
> -                      cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],     \
> -                      add_ca, compute_ca, compute_ov, Rc(ctx->opcode));       \
> -}
> -/* Sub functions with one operand and one immediate */
> -#define GEN_INT_ARITH_SUBF_CONST(name, opc3, const_val,                       \
> -                                add_ca, compute_ca, compute_ov)               \
> -static void glue(gen_, name)(DisasContext *ctx)                               \
> -{                                                                             \
> -    TCGv t0 = tcg_constant_tl(const_val);                                     \
> -    gen_op_arith_subf(ctx, cpu_gpr[rD(ctx->opcode)],                          \
> -                      cpu_gpr[rA(ctx->opcode)], t0,                           \
> -                      add_ca, compute_ca, compute_ov, Rc(ctx->opcode));       \
> -}
> -/* subf  subf.  subfo  subfo. */
> -GEN_INT_ARITH_SUBF(subf, 0x01, 0, 0, 0)
> -GEN_INT_ARITH_SUBF(subfo, 0x11, 0, 0, 1)
> -/* subfc  subfc.  subfco  subfco. */
> -GEN_INT_ARITH_SUBF(subfc, 0x00, 0, 1, 0)
> -GEN_INT_ARITH_SUBF(subfco, 0x10, 0, 1, 1)
> -/* subfe  subfe.  subfeo  subfo. */
> -GEN_INT_ARITH_SUBF(subfe, 0x04, 1, 1, 0)
> -GEN_INT_ARITH_SUBF(subfeo, 0x14, 1, 1, 1)
> -/* subfme  subfme.  subfmeo  subfmeo.  */
> -GEN_INT_ARITH_SUBF_CONST(subfme, 0x07, -1LL, 1, 1, 0)
> -GEN_INT_ARITH_SUBF_CONST(subfmeo, 0x17, -1LL, 1, 1, 1)
> -/* subfze  subfze.  subfzeo  subfzeo.*/
> -GEN_INT_ARITH_SUBF_CONST(subfze, 0x06, 0, 1, 1, 0)
> -GEN_INT_ARITH_SUBF_CONST(subfzeo, 0x16, 0, 1, 1, 1)
> -
> -/* subfic */
> -static void gen_subfic(DisasContext *ctx)
> -{
> -    TCGv c = tcg_constant_tl(SIMM(ctx->opcode));
> -    gen_op_arith_subf(ctx, cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
> -                      c, 0, 1, 0, 0);
> -}
>   
>   /* neg neg. nego nego. */
>   static inline void gen_op_arith_neg(DisasContext *ctx, bool compute_ov)
> @@ -6486,8 +6390,6 @@ GEN_HANDLER_E(cmpeqb, 0x1F, 0x00, 0x07, 0x00600000, PPC_NONE, PPC2_ISA300),
>   GEN_HANDLER_E(cmpb, 0x1F, 0x1C, 0x0F, 0x00000001, PPC_NONE, PPC2_ISA205),
>   GEN_HANDLER_E(cmprb, 0x1F, 0x00, 0x06, 0x00400001, PPC_NONE, PPC2_ISA300),
>   GEN_HANDLER(isel, 0x1F, 0x0F, 0xFF, 0x00000001, PPC_ISEL),
> -GEN_HANDLER(addic, 0x0C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
> -GEN_HANDLER2(addic_, "addic.", 0x0D, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
>   GEN_HANDLER(mulhw, 0x1F, 0x0B, 0x02, 0x00000400, PPC_INTEGER),
>   GEN_HANDLER(mulhwu, 0x1F, 0x0B, 0x00, 0x00000400, PPC_INTEGER),
>   GEN_HANDLER(mullw, 0x1F, 0x0B, 0x07, 0x00000000, PPC_INTEGER),
> @@ -6498,7 +6400,6 @@ GEN_HANDLER(mulld, 0x1F, 0x09, 0x07, 0x00000000, PPC_64B),
>   #endif
>   GEN_HANDLER(neg, 0x1F, 0x08, 0x03, 0x0000F800, PPC_INTEGER),
>   GEN_HANDLER(nego, 0x1F, 0x08, 0x13, 0x0000F800, PPC_INTEGER),
> -GEN_HANDLER(subfic, 0x08, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
>   GEN_HANDLER2(andi_, "andi.", 0x1C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
>   GEN_HANDLER2(andis_, "andis.", 0x1D, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
>   GEN_HANDLER(cntlzw, 0x1F, 0x1A, 0x00, 0x00000000, PPC_INTEGER),
> @@ -6709,25 +6610,6 @@ GEN_HANDLER_E(maddhd_maddhdu, 0x04, 0x18, 0xFF, 0x00000000, PPC_NONE,
>   GEN_HANDLER_E(maddld, 0x04, 0x19, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300),
>   #endif
>   
> -#undef GEN_INT_ARITH_ADD
> -#undef GEN_INT_ARITH_ADD_CONST
> -#define GEN_INT_ARITH_ADD(name, opc3, add_ca, compute_ca, compute_ov)         \
> -GEN_HANDLER(name, 0x1F, 0x0A, opc3, 0x00000000, PPC_INTEGER),
> -#define GEN_INT_ARITH_ADD_CONST(name, opc3, const_val,                        \
> -                                add_ca, compute_ca, compute_ov)               \
> -GEN_HANDLER(name, 0x1F, 0x0A, opc3, 0x0000F800, PPC_INTEGER),
> -GEN_INT_ARITH_ADD(add, 0x08, 0, 0, 0)
> -GEN_INT_ARITH_ADD(addo, 0x18, 0, 0, 1)
> -GEN_INT_ARITH_ADD(addc, 0x00, 0, 1, 0)
> -GEN_INT_ARITH_ADD(addco, 0x10, 0, 1, 1)
> -GEN_INT_ARITH_ADD(adde, 0x04, 1, 1, 0)
> -GEN_INT_ARITH_ADD(addeo, 0x14, 1, 1, 1)
> -GEN_INT_ARITH_ADD_CONST(addme, 0x07, -1LL, 1, 1, 0)
> -GEN_INT_ARITH_ADD_CONST(addmeo, 0x17, -1LL, 1, 1, 1)
> -GEN_HANDLER_E(addex, 0x1F, 0x0A, 0x05, 0x00000000, PPC_NONE, PPC2_ISA300),
> -GEN_INT_ARITH_ADD_CONST(addze, 0x06, 0, 1, 1, 0)
> -GEN_INT_ARITH_ADD_CONST(addzeo, 0x16, 0, 1, 1, 1)
> -
>   #undef GEN_INT_ARITH_DIVW
>   #define GEN_INT_ARITH_DIVW(name, opc3, sign, compute_ov)                      \
>   GEN_HANDLER(name, 0x1F, 0x0B, opc3, 0x00000000, PPC_INTEGER)
> @@ -6766,24 +6648,6 @@ GEN_INT_ARITH_MUL_HELPER(mulhd, 0x02),
>   GEN_INT_ARITH_MUL_HELPER(mulldo, 0x17),
>   #endif
>   
> -#undef GEN_INT_ARITH_SUBF
> -#undef GEN_INT_ARITH_SUBF_CONST
> -#define GEN_INT_ARITH_SUBF(name, opc3, add_ca, compute_ca, compute_ov)        \
> -GEN_HANDLER(name, 0x1F, 0x08, opc3, 0x00000000, PPC_INTEGER),
> -#define GEN_INT_ARITH_SUBF_CONST(name, opc3, const_val,                       \
> -                                add_ca, compute_ca, compute_ov)               \
> -GEN_HANDLER(name, 0x1F, 0x08, opc3, 0x0000F800, PPC_INTEGER),
> -GEN_INT_ARITH_SUBF(subf, 0x01, 0, 0, 0)
> -GEN_INT_ARITH_SUBF(subfo, 0x11, 0, 0, 1)
> -GEN_INT_ARITH_SUBF(subfc, 0x00, 0, 1, 0)
> -GEN_INT_ARITH_SUBF(subfco, 0x10, 0, 1, 1)
> -GEN_INT_ARITH_SUBF(subfe, 0x04, 1, 1, 0)
> -GEN_INT_ARITH_SUBF(subfeo, 0x14, 1, 1, 1)
> -GEN_INT_ARITH_SUBF_CONST(subfme, 0x07, -1LL, 1, 1, 0)
> -GEN_INT_ARITH_SUBF_CONST(subfmeo, 0x17, -1LL, 1, 1, 1)
> -GEN_INT_ARITH_SUBF_CONST(subfze, 0x06, 0, 1, 1, 0)
> -GEN_INT_ARITH_SUBF_CONST(subfzeo, 0x16, 0, 1, 1, 1)
> -
>   #undef GEN_LOGICAL1
>   #undef GEN_LOGICAL2
>   #define GEN_LOGICAL2(name, tcg_op, opc, type)                                 \
> diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
> index 51c6fa7330..12374b78cf 100644
> --- a/target/ppc/translate/fixedpoint-impl.c.inc
> +++ b/target/ppc/translate/fixedpoint-impl.c.inc
> @@ -325,6 +325,75 @@ static bool trans_ADDPCIS(DisasContext *ctx, arg_DX *a)
>       return true;
>   }
>   
> +static bool trans_ADDEX(DisasContext *ctx, arg_Z23_tab_cy *a)
> +{
> +    gen_op_arith_add(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], cpu_gpr[a->rb],
> +                     cpu_ov, cpu_ov32, true, true, false, false);
> +    return true;
> +}
> +
> +static bool do_add_D(DisasContext *ctx, arg_D *a, bool add_ca, bool compute_ca,
> +                     bool compute_ov, bool compute_rc0)
> +{
> +    gen_op_arith_add(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra],
> +                     tcg_constant_tl(a->si), cpu_ca, cpu_ca32,
> +                     add_ca, compute_ca, compute_ov, compute_rc0);
> +    return true;
> +}
> +
> +static bool do_add_XO(DisasContext *ctx, arg_XO *a, bool add_ca,
> +                      bool compute_ca)
> +{
> +    gen_op_arith_add(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], cpu_gpr[a->rb],
> +                     cpu_ca, cpu_ca32, add_ca, compute_ca, a->oe, a->rc);
> +    return true;
> +}
> +
> +static bool do_add_const_XO(DisasContext *ctx, arg_XO_ta *a, TCGv const_val,
> +                            bool add_ca, bool compute_ca)
> +{
> +    gen_op_arith_add(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], const_val,
> +                     cpu_ca, cpu_ca32, add_ca, compute_ca, a->oe, a->rc);
> +    return true;
> +}
> +
> +TRANS(ADD, do_add_XO, false, false);
> +TRANS(ADDC, do_add_XO, false, true);
> +TRANS(ADDE, do_add_XO, true, true);
> +TRANS(ADDME, do_add_const_XO, tcg_constant_tl(-1LL), true, true);
> +TRANS(ADDZE, do_add_const_XO, tcg_constant_tl(0), true, true);
> +TRANS(ADDIC, do_add_D, false, true, false, false);
> +TRANS(ADDIC_, do_add_D, false, true, false, true);
> +
> +static bool trans_SUBFIC(DisasContext *ctx, arg_D *a)
> +{
> +    gen_op_arith_subf(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra],
> +                      tcg_constant_tl(a->si), false, true, false, false);
> +    return true;
> +}
> +
> +static bool do_subf_XO(DisasContext *ctx, arg_XO *a, bool add_ca,
> +                       bool compute_ca)
> +{
> +    gen_op_arith_subf(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], cpu_gpr[a->rb],
> +                      add_ca, compute_ca, a->oe, a->rc);
> +    return true;
> +}
> +
> +static bool do_subf_const_XO(DisasContext *ctx, arg_XO_ta *a, TCGv const_val,
> +                             bool add_ca, bool compute_ca)
> +{
> +    gen_op_arith_subf(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], const_val,
> +                      add_ca, compute_ca, a->oe, a->rc);
> +    return true;
> +}
> +
> +TRANS(SUBF, do_subf_XO, false, false)
> +TRANS(SUBFC, do_subf_XO, false, true)
> +TRANS(SUBFE, do_subf_XO, true, true)
> +TRANS(SUBFME, do_subf_const_XO, tcg_constant_tl(-1LL), true, true)
> +TRANS(SUBFZE, do_subf_const_XO, tcg_constant_tl(0), true, true)
> +
>   static bool trans_INVALID(DisasContext *ctx, arg_INVALID *a)
>   {
>       gen_invalid(ctx);