diff mbox series

[17/21] Hexagon (target/hexagon) Move new_value to DisasContext

Message ID 20230426004234.1319401-8-tsimpson@quicinc.com (mailing list archive)
State New, archived
Headers show
Series Hexagon (target/hexagon) short-circuit and move to DisasContext | expand

Commit Message

Taylor Simpson April 26, 2023, 12:42 a.m. UTC 
The new_value array in the CPUHexagonState is only used for bookkeeping
within the translation of a packet.  With recent changes that eliminate
the need to free TCGv variables, these make more sense to be transient
and kept in DisasContext.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
 target/hexagon/cpu.h       |  1 -
 target/hexagon/translate.h |  2 +-
 target/hexagon/genptr.c    |  6 +++++-
 target/hexagon/translate.c | 14 +++-----------
 4 files changed, 9 insertions(+), 14 deletions(-)

Comments

Richard Henderson April 27, 2023, 11:01 a.m. UTC | #1 
On 4/26/23 01:42, Taylor Simpson wrote:
> +    for (i = 0; i < TOTAL_PER_THREAD_REGS; i++) {
> +        ctx->new_value[i] = NULL;
> +    }

Perhaps

   memset(ctx->new_value, 0, sizeof(ctx->new_value));

Though probably the compiler would make that transformation.
Or perhaps

-    DisasContext ctx;
+    DisasContext ctx = { 0 };

in gen_intermediate_code, and eliminate other 0 init in gen_start_packet?

But it's not wrong as-is, so,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Richard Henderson April 27, 2023, 11:03 a.m. UTC | #2 
On 4/27/23 12:01, Richard Henderson wrote:
> On 4/26/23 01:42, Taylor Simpson wrote:
>> +    for (i = 0; i < TOTAL_PER_THREAD_REGS; i++) {
>> +        ctx->new_value[i] = NULL;
>> +    }
> 
> Perhaps
> 
>    memset(ctx->new_value, 0, sizeof(ctx->new_value));
> 
> Though probably the compiler would make that transformation.
> Or perhaps
> 
> -    DisasContext ctx;
> +    DisasContext ctx = { 0 };
> 
> in gen_intermediate_code, and eliminate other 0 init in gen_start_packet?

Duh, start packet is called more than once per TB.


r~
diff mbox series

Patch

diff --git a/target/hexagon/cpu.h b/target/hexagon/cpu.h
index 3687f2caa2..22aba20be2 100644
--- a/target/hexagon/cpu.h
+++ b/target/hexagon/cpu.h
@@ -85,7 +85,6 @@  typedef struct CPUArchState {
     target_ulong stack_start;
 
     uint8_t slot_cancelled;
-    target_ulong new_value[TOTAL_PER_THREAD_REGS];
     target_ulong new_value_usr;
 
     /*
diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h
index 4c17433a6f..6dde487566 100644
--- a/target/hexagon/translate.h
+++ b/target/hexagon/translate.h
@@ -69,6 +69,7 @@  typedef struct DisasContext {
     bool need_pkt_has_store_s1;
     bool short_circuit;
     bool has_hvx_helper;
+    TCGv new_value[TOTAL_PER_THREAD_REGS];
 } DisasContext;
 
 static inline void ctx_log_pred_write(DisasContext *ctx, int pnum)
@@ -190,7 +191,6 @@  extern TCGv hex_pred[NUM_PREGS];
 extern TCGv hex_this_PC;
 extern TCGv hex_slot_cancelled;
 extern TCGv hex_branch_taken;
-extern TCGv hex_new_value[TOTAL_PER_THREAD_REGS];
 extern TCGv hex_new_value_usr;
 extern TCGv hex_reg_written[TOTAL_PER_THREAD_REGS];
 extern TCGv hex_new_pred_value[NUM_PREGS];
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index e6327425cd..47d472f586 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -74,7 +74,11 @@  TCGv get_result_gpr(DisasContext *ctx, int rnum)
         if (rnum == HEX_REG_USR) {
             return hex_new_value_usr;
         } else {
-            return hex_new_value[rnum];
+            if (ctx->new_value[rnum] == NULL) {
+                ctx->new_value[rnum] = tcg_temp_new();
+                tcg_gen_movi_tl(ctx->new_value[rnum], 0);
+            }
+            return ctx->new_value[rnum];
         }
     } else {
         return hex_gpr[rnum];
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 0afb3a0993..3d6b22a577 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -44,7 +44,6 @@  TCGv hex_pred[NUM_PREGS];
 TCGv hex_this_PC;
 TCGv hex_slot_cancelled;
 TCGv hex_branch_taken;
-TCGv hex_new_value[TOTAL_PER_THREAD_REGS];
 TCGv hex_new_value_usr;
 TCGv hex_reg_written[TOTAL_PER_THREAD_REGS];
 TCGv hex_new_pred_value[NUM_PREGS];
@@ -513,6 +512,9 @@  static void gen_start_packet(DisasContext *ctx)
     }
     ctx->s1_store_processed = false;
     ctx->pre_commit = true;
+    for (i = 0; i < TOTAL_PER_THREAD_REGS; i++) {
+        ctx->new_value[i] = NULL;
+    }
 
     analyze_packet(ctx);
 
@@ -1156,7 +1158,6 @@  void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
 }
 
 #define NAME_LEN               64
-static char new_value_names[TOTAL_PER_THREAD_REGS][NAME_LEN];
 static char reg_written_names[TOTAL_PER_THREAD_REGS][NAME_LEN];
 static char new_pred_value_names[NUM_PREGS][NAME_LEN];
 static char store_addr_names[STORES_MAX][NAME_LEN];
@@ -1178,15 +1179,6 @@  void hexagon_translate_init(void)
             offsetof(CPUHexagonState, gpr[i]),
             hexagon_regnames[i]);
 
-        if (i == HEX_REG_USR) {
-            hex_new_value[i] = NULL;
-        } else {
-            snprintf(new_value_names[i], NAME_LEN, "new_%s", hexagon_regnames[i]);
-            hex_new_value[i] = tcg_global_mem_new(cpu_env,
-                offsetof(CPUHexagonState, new_value[i]),
-                new_value_names[i]);
-        }
-
         if (HEX_DEBUG) {
             snprintf(reg_written_names[i], NAME_LEN, "reg_written_%s",
                      hexagon_regnames[i]);