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>
---
 target/hexagon/translate.h |  3 +++
 target/hexagon/genptr.c    | 14 ++++++++---
 target/hexagon/translate.c | 51 ++++++++++++++++++++++++++------------
 3 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h
index 0a7f37d5842..2fca157553b 100644
--- 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];
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 1ebc7471409..dc9ba8a0060 100644
--- 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 28a171bf623..22f0ff5b891 100644
--- 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,
-- 
2.34.1

On Wed, Jul 15, 2026 at 12:57 PM Brian Cain <brian.cain@oss.qualcomm.com>
wrote:

> 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>
> ---
>  target/hexagon/translate.h |  3 +++
>  target/hexagon/genptr.c    | 14 ++++++++---
>  target/hexagon/translate.c | 51 ++++++++++++++++++++++++++------------
>  3 files changed, 49 insertions(+), 19 deletions(-)
>
>
> @@ -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");
>

This only lives within the context of a single packet, so it shouldn't be
global.

Put it into DisasContext and make it a tmp similar to new_value.

On 7/19/2026 4:31 PM, Taylor Simpson wrote:
>
>
> On Wed, Jul 15, 2026 at 12:57 PM Brian Cain 
> <brian.cain@oss.qualcomm.com> wrote:
>
>     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>
>     ---
>      target/hexagon/translate.h |  3 +++
>      target/hexagon/genptr.c    | 14 ++++++++---
>      target/hexagon/translate.c | 51
>     ++++++++++++++++++++++++++------------
>      3 files changed, 49 insertions(+), 19 deletions(-)
>
>
>     @@ -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");
>
>
> This only lives within the context of a single packet, so it shouldn't 
> be global.
>
> Put it into DisasContext and make it a tmp similar to new_value.


Taylor - this patch was included for the sake of testing the l2vic 
series, but landed recently - 
https://lore.kernel.org/qemu-devel/20260720100107.GA816167@fedora/

So I'll send a new independent series with updates suggested for 
"target/hexagon: fix PC not advancing for non-COF TB-ending packets" and 
"target/hexagon: initialize deferred sreg-write temps to current value".

-Brian