[PATCH 04/12] target/mips: Convert mips16e decr_and_load/store() macros to functions

Philippe Mathieu-Daudé posted 12 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH 04/12] target/mips: Convert mips16e decr_and_load/store() macros to functions
Posted by Philippe Mathieu-Daudé 2 months, 3 weeks ago
Functions are easier to rework than macros. Besides,
there is no gain here in inlining these.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/tcg/mips16e_translate.c.inc | 101 +++++++++++++-----------
 1 file changed, 53 insertions(+), 48 deletions(-)

diff --git a/target/mips/tcg/mips16e_translate.c.inc b/target/mips/tcg/mips16e_translate.c.inc
index 5cffe0e412..31bc14f9ca 100644
--- a/target/mips/tcg/mips16e_translate.c.inc
+++ b/target/mips/tcg/mips16e_translate.c.inc
@@ -122,11 +122,23 @@ enum {
 
 static int xlat(int r)
 {
-  static int map[] = { 16, 17, 2, 3, 4, 5, 6, 7 };
+  static const int map[] = { 16, 17, 2, 3, 4, 5, 6, 7 };
 
   return map[r];
 }
 
+static void decr_and_store(DisasContext *ctx, unsigned regidx, TCGv t0)
+{
+    TCGv t1 = tcg_temp_new();
+    TCGv t2 = tcg_temp_new();
+
+    tcg_gen_movi_tl(t2, -4);
+    gen_op_addr_add(ctx, t0, t0, t2);
+    gen_load_gpr(t1, regidx);
+    tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEUL |
+                       ctx->default_tcg_memop_mask);
+}
+
 static void gen_mips16_save(DisasContext *ctx,
                             int xsregs, int aregs,
                             int do_ra, int do_s0, int do_s1,
@@ -196,46 +208,38 @@ static void gen_mips16_save(DisasContext *ctx,
 
     gen_load_gpr(t0, 29);
 
-#define DECR_AND_STORE(reg) do {                                 \
-        tcg_gen_movi_tl(t2, -4);                                 \
-        gen_op_addr_add(ctx, t0, t0, t2);                        \
-        gen_load_gpr(t1, reg);                                   \
-        tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEUL |       \
-                           ctx->default_tcg_memop_mask);         \
-    } while (0)
-
     if (do_ra) {
-        DECR_AND_STORE(31);
+        decr_and_store(ctx, 31, t0);
     }
 
     switch (xsregs) {
     case 7:
-        DECR_AND_STORE(30);
+        decr_and_store(ctx, 30, t0);
         /* Fall through */
     case 6:
-        DECR_AND_STORE(23);
+        decr_and_store(ctx, 23, t0);
         /* Fall through */
     case 5:
-        DECR_AND_STORE(22);
+        decr_and_store(ctx, 22, t0);
         /* Fall through */
     case 4:
-        DECR_AND_STORE(21);
+        decr_and_store(ctx, 21, t0);
         /* Fall through */
     case 3:
-        DECR_AND_STORE(20);
+        decr_and_store(ctx, 20, t0);
         /* Fall through */
     case 2:
-        DECR_AND_STORE(19);
+        decr_and_store(ctx, 19, t0);
         /* Fall through */
     case 1:
-        DECR_AND_STORE(18);
+        decr_and_store(ctx, 18, t0);
     }
 
     if (do_s1) {
-        DECR_AND_STORE(17);
+        decr_and_store(ctx, 17, t0);
     }
     if (do_s0) {
-        DECR_AND_STORE(16);
+        decr_and_store(ctx, 16, t0);
     }
 
     switch (aregs) {
@@ -270,23 +274,34 @@ static void gen_mips16_save(DisasContext *ctx,
     }
 
     if (astatic > 0) {
-        DECR_AND_STORE(7);
+        decr_and_store(ctx, 7, t0);
         if (astatic > 1) {
-            DECR_AND_STORE(6);
+        decr_and_store(ctx, 6, t0);
             if (astatic > 2) {
-                DECR_AND_STORE(5);
+                decr_and_store(ctx, 5, t0);
                 if (astatic > 3) {
-                    DECR_AND_STORE(4);
+                    decr_and_store(ctx, 4, t0);
                 }
             }
         }
     }
-#undef DECR_AND_STORE
 
     tcg_gen_movi_tl(t2, -framesize);
     gen_op_addr_add(ctx, cpu_gpr[29], cpu_gpr[29], t2);
 }
 
+static void decr_and_load(DisasContext *ctx, unsigned regidx, TCGv t0)
+{
+    TCGv t1 = tcg_temp_new();
+    TCGv t2 = tcg_temp_new();
+
+    tcg_gen_movi_tl(t2, -4);
+    gen_op_addr_add(ctx, t0, t0, t2);
+    tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_TESL |
+                       ctx->default_tcg_memop_mask);
+    gen_store_gpr(t1, regidx);
+}
+
 static void gen_mips16_restore(DisasContext *ctx,
                                int xsregs, int aregs,
                                int do_ra, int do_s0, int do_s1,
@@ -294,52 +309,43 @@ static void gen_mips16_restore(DisasContext *ctx,
 {
     int astatic;
     TCGv t0 = tcg_temp_new();
-    TCGv t1 = tcg_temp_new();
     TCGv t2 = tcg_temp_new();
 
     tcg_gen_movi_tl(t2, framesize);
     gen_op_addr_add(ctx, t0, cpu_gpr[29], t2);
 
-#define DECR_AND_LOAD(reg) do {                            \
-        tcg_gen_movi_tl(t2, -4);                           \
-        gen_op_addr_add(ctx, t0, t0, t2);                  \
-        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_TESL | \
-                           ctx->default_tcg_memop_mask);   \
-        gen_store_gpr(t1, reg);                            \
-    } while (0)
-
     if (do_ra) {
-        DECR_AND_LOAD(31);
+        decr_and_load(ctx, 31, t0);
     }
 
     switch (xsregs) {
     case 7:
-        DECR_AND_LOAD(30);
+        decr_and_load(ctx, 30, t0);
         /* Fall through */
     case 6:
-        DECR_AND_LOAD(23);
+        decr_and_load(ctx, 23, t0);
         /* Fall through */
     case 5:
-        DECR_AND_LOAD(22);
+        decr_and_load(ctx, 22, t0);
         /* Fall through */
     case 4:
-        DECR_AND_LOAD(21);
+        decr_and_load(ctx, 21, t0);
         /* Fall through */
     case 3:
-        DECR_AND_LOAD(20);
+        decr_and_load(ctx, 20, t0);
         /* Fall through */
     case 2:
-        DECR_AND_LOAD(19);
+        decr_and_load(ctx, 19, t0);
         /* Fall through */
     case 1:
-        DECR_AND_LOAD(18);
+        decr_and_load(ctx, 18, t0);
     }
 
     if (do_s1) {
-        DECR_AND_LOAD(17);
+        decr_and_load(ctx, 17, t0);
     }
     if (do_s0) {
-        DECR_AND_LOAD(16);
+        decr_and_load(ctx, 16, t0);
     }
 
     switch (aregs) {
@@ -374,18 +380,17 @@ static void gen_mips16_restore(DisasContext *ctx,
     }
 
     if (astatic > 0) {
-        DECR_AND_LOAD(7);
+        decr_and_load(ctx, 7, t0);
         if (astatic > 1) {
-            DECR_AND_LOAD(6);
+            decr_and_load(ctx, 6, t0);
             if (astatic > 2) {
-                DECR_AND_LOAD(5);
+                decr_and_load(ctx, 5, t0);
                 if (astatic > 3) {
-                    DECR_AND_LOAD(4);
+                    decr_and_load(ctx, 4, t0);
                 }
             }
         }
     }
-#undef DECR_AND_LOAD
 
     tcg_gen_movi_tl(t2, framesize);
     gen_op_addr_add(ctx, cpu_gpr[29], cpu_gpr[29], t2);
-- 
2.45.2


Re: [PATCH 04/12] target/mips: Convert mips16e decr_and_load/store() macros to functions
Posted by Richard Henderson 2 months, 2 weeks ago
On 9/30/24 02:10, Philippe Mathieu-Daudé wrote:
> Functions are easier to rework than macros. Besides,
> there is no gain here in inlining these.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/mips/tcg/mips16e_translate.c.inc | 101 +++++++++++++-----------
>   1 file changed, 53 insertions(+), 48 deletions(-)
> 
> diff --git a/target/mips/tcg/mips16e_translate.c.inc b/target/mips/tcg/mips16e_translate.c.inc
> index 5cffe0e412..31bc14f9ca 100644
> --- a/target/mips/tcg/mips16e_translate.c.inc
> +++ b/target/mips/tcg/mips16e_translate.c.inc
> @@ -122,11 +122,23 @@ enum {
>   
>   static int xlat(int r)
>   {
> -  static int map[] = { 16, 17, 2, 3, 4, 5, 6, 7 };
> +  static const int map[] = { 16, 17, 2, 3, 4, 5, 6, 7 };
>   
>     return map[r];
>   }
>   
> +static void decr_and_store(DisasContext *ctx, unsigned regidx, TCGv t0)
> +{
> +    TCGv t1 = tcg_temp_new();
> +    TCGv t2 = tcg_temp_new();
> +
> +    tcg_gen_movi_tl(t2, -4);
> +    gen_op_addr_add(ctx, t0, t0, t2);

Code movement, so:
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

However, for cleanup, pass tcg_constant_tl(-4) instead of moving -4 into t2.


r~