target/hexagon: fix PC not advancing for non-COF TB-ending packets

Add hex_next_PC, a global mirroring CPUHexagonState::next_PC,
and ctx->need_next_pc, so that gen_write_new_pc_addr() can write the
branch target through hex_next_PC instead of hex_gpr[HEX_REG_PC]
when a later unconditional write of PC is expected. gen_end_tb()
then commits hex_next_PC into hex_gpr[HEX_REG_PC] at the end of the
packet.

Previously, non-COF instructions that still end a TB did not advance the
PC, since next_PC's value was never written back into the PC register.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>

diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index f48a27bc09b85c88781165d446cf821dbea107b7..3f310377097c75396e10758938f06f9a8537138f 100644 (file)
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -613,14 +613,22 @@ static void gen_write_new_pc_addr(DisasContext *ctx, TCGv addr,
         tcg_gen_brcondi_tl(cond, pred, 1, pred_false);
     }
 
+    /*
+     * If gen_end_tb() will unconditionally overwrite PC with hex_next_PC
+     * (because this packet has a predicated COF that may not execute),
+     * write the branch target there instead of directly into the PC
+     * global, or the overwrite in gen_end_tb() would clobber it.
+     */
+    TCGv pc_wr = ctx->need_next_pc ? hex_next_PC : hex_gpr[HEX_REG_PC];
+
     if (ctx->pkt.pkt_has_multi_cof) {
         /* If there are multiple branches in a packet, ignore the second one */
-        tcg_gen_movcond_tl(TCG_COND_NE, hex_gpr[HEX_REG_PC],
+        tcg_gen_movcond_tl(TCG_COND_NE, pc_wr,
                            ctx->branch_taken, tcg_constant_tl(0),
-                           hex_gpr[HEX_REG_PC], addr);
+                           pc_wr, addr);
         tcg_gen_movi_tl(ctx->branch_taken, 1);
     } else {
-        tcg_gen_mov_tl(hex_gpr[HEX_REG_PC], addr);
+        tcg_gen_mov_tl(pc_wr, addr);
     }
 
     if (cond != TCG_COND_ALWAYS) {

diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 115b6912553749902fd74d7ca145cab4e01bddd6..199b4f8c2eca2ed3abfd3ad11b59fdb081bc2bd3 100644 (file)
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -54,6 +54,7 @@ static const AnalyzeInsn opcode_analyze[XX_LAST_OPCODE] = {
 TCGv hex_gpr[TOTAL_PER_THREAD_REGS];
 TCGv hex_pred[NUM_PREGS];
 TCGv hex_slot_cancelled;
+TCGv hex_next_PC;
 TCGv hex_new_value_usr;
 TCGv hex_store_addr[STORES_MAX];
 TCGv_i32 hex_store_width[STORES_MAX];
@@ -184,10 +185,16 @@ static void gen_goto_tb(DisasContext *ctx, unsigned tb_slot_idx,
     }
 }
 
+static bool need_next_PC(DisasContext *ctx);
+
 static void gen_end_tb(DisasContext *ctx)
 {
     gen_exec_counters(ctx);
 
+    if (ctx->need_next_pc) {
+        tcg_gen_mov_tl(hex_gpr[HEX_REG_PC], hex_next_PC);
+    }
+
     if (ctx->branch_cond != TCG_COND_NEVER) {
         if (ctx->branch_cond != TCG_COND_ALWAYS) {
             TCGLabel *skip = gen_new_label();
@@ -391,17 +398,25 @@ static bool pkt_ends_tb(Packet *pkt)
 
 static bool need_next_PC(DisasContext *ctx)
 {
-    /* Check for conditional control flow or HW loop end */
-    for (int i = 0; i < ctx->pkt.num_insns; i++) {
-        uint16_t opcode = ctx->pkt.insn[i].opcode;
-        if (GET_ATTRIB(opcode, A_CONDEXEC) && GET_ATTRIB(opcode, A_COF)) {
-            return true;
-        }
-        if (GET_ATTRIB(opcode, A_HWLOOP0_END) ||
-            GET_ATTRIB(opcode, A_HWLOOP1_END)) {
-            return true;
+    Packet *pkt = &ctx->pkt;
+    if (pkt->pkt_has_cof || ctx->pkt_ends_tb) {
+        for (int i = 0; i < pkt->num_insns; i++) {
+            uint16_t opcode = pkt->insn[i].opcode;
+            if ((GET_ATTRIB(opcode, A_CONDEXEC) && GET_ATTRIB(opcode, A_COF)) ||
+                GET_ATTRIB(opcode, A_HWLOOP0_END) ||
+                GET_ATTRIB(opcode, A_HWLOOP1_END)) {
+                return true;
+            }
         }
     }
+    /*
+     * We end the TB on some instructions that do not change the flow (for
+     * other reasons). In these cases, we must set pc too, as the insn won't
+     * do it themselves.
+     */
+    if (ctx->pkt_ends_tb && !check_for_attrib(pkt, A_COF)) {
+        return true;
+    }
     return false;
 }
 
@@ -637,12 +652,14 @@ static void gen_start_packet(DisasContext *ctx)
     ctx->branch_taken = NULL;
     if (ctx->pkt.pkt_has_cof) {
         ctx->branch_taken = tcg_temp_new();
-        if (ctx->pkt.pkt_has_multi_cof) {
-            tcg_gen_movi_tl(ctx->branch_taken, 0);
-        }
-        if (need_next_PC(ctx)) {
-            tcg_gen_movi_tl(hex_gpr[HEX_REG_PC], next_PC);
-        }
+    }
+    if (ctx->pkt.pkt_has_multi_cof) {
+        tcg_gen_movi_tl(ctx->branch_taken, 0);
+    }
+    ctx->pkt_ends_tb = pkt_ends_tb(&ctx->pkt);
+    ctx->need_next_pc = need_next_PC(ctx);
+    if (ctx->need_next_pc) {
+        tcg_gen_movi_tl(hex_next_PC, next_PC);
     }
 
     /* Preload the predicated registers into get_result_gpr(ctx, i) */
@@ -1142,7 +1159,7 @@ static void gen_commit_packet(DisasContext *ctx)
         ctx->pkt.vhist_insn->generate(ctx);
     }
 
-    if (pkt_ends_tb(&ctx->pkt) || ctx->base.is_jmp == DISAS_NORETURN) {
+    if (ctx->pkt_ends_tb || ctx->base.is_jmp == DISAS_NORETURN) {
         gen_end_tb(ctx);
     }
 }
@@ -1327,6 +1344,8 @@ void hexagon_translate_init(void)
     }
     hex_new_value_usr = tcg_global_mem_new(tcg_env,
         offsetof(CPUHexagonState, new_value_usr), "new_value_usr");
+    hex_next_PC = tcg_global_mem_new(tcg_env,
+        offsetof(CPUHexagonState, next_PC), "next_PC");
 
     for (i = 0; i < NUM_PREGS; i++) {
         hex_pred[i] = tcg_global_mem_new(tcg_env,

diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h
index 0a7f37d58429aa194e268a1e5692b72748d8abec..2fca157553b6f122585b800a5db62ea6cdb005be 100644 (file)
--- a/target/hexagon/translate.h
+++ b/target/hexagon/translate.h
@@ -40,6 +40,7 @@ typedef struct DisasContext {
     int reg_log_idx;
     DECLARE_BITMAP(regs_written, TOTAL_PER_THREAD_REGS);
     DECLARE_BITMAP(predicated_regs, TOTAL_PER_THREAD_REGS);
+    bool pkt_ends_tb;
     bool implicit_usr_write;
 #ifndef CONFIG_USER_ONLY
     int greg_log[GREG_WRITES_MAX];
@@ -75,6 +76,7 @@ typedef struct DisasContext {
     DECLARE_BITMAP(insn_qregs_read, NUM_QREGS);
     bool pre_commit;
     bool need_commit;
+    bool need_next_pc;
     TCGCond branch_cond;
     target_ulong branch_dest;
     bool is_tight_loop;
@@ -310,6 +312,7 @@ extern TCGv hex_gpr[TOTAL_PER_THREAD_REGS];
 extern TCGv hex_pred[NUM_PREGS];
 extern TCGv hex_slot_cancelled;
 extern TCGv hex_new_value_usr;
+extern TCGv hex_next_PC;
 extern TCGv hex_store_addr[STORES_MAX];
 extern TCGv_i32 hex_store_width[STORES_MAX];
 extern TCGv hex_store_val32[STORES_MAX];