Hexagon (target/hexagon) analyze all reads before writes

I noticed that analyze_packet is marking the implicit pred reads after
marking all the writes.  However, the semantics of the instrucion and
packet are to do all the reads, then do the operation, then do all the
writes.

Here is the old code
static void analyze_packet(DisasContext *ctx)
{
    Packet *pkt = ctx->pkt;
    ctx->read_after_write = false;
    ctx->has_hvx_overlap = false;
    for (int i = 0; i < pkt->num_insns; i++) {
        Insn *insn = &pkt->insn[i];
        ctx->insn = insn;
        if (opcode_analyze[insn->opcode]) {
            opcode_analyze[insn->opcode](ctx);
        }
        mark_implicit_reg_writes(ctx);
        mark_implicit_pred_writes(ctx);
        mark_implicit_pred_reads(ctx);
    }

    ctx->need_commit = need_commit(ctx);
}

Recall that opcode_analyze[insn->opcode](ctx) will mark all the
explicit reads then all the explicit writes.

To properly handle the semantics, we'll create two new functions
    mark_implicit_reads
    mark_implicit_writes
Then we change gen_analyze_funcs.py to add a call to the former
after all the explicit reads and a call to the latter after all
the explicit_writes.

The reason this is an RFC patch is I can't find any instructions
where this distinction makes a difference in ctx->need_commit which
determines if the packet commit can be short-circuited.  However, this
could change in the future if the architecture introduces an
instruction with an implicit read of a register that is also written
(either implicit or explicit).  Then, anlayze_packet would detect
a read-after-write, and the packet would not short-circuit.  The
execution would be correct, but the performance would not be optimal.

Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
Reviewed-by: Brian Cain <brian.cain@oss.qualcomm.com>
Tested-by: Brian Cain <brian.cain@oss.qualcomm.com>
Message-Id: <20250325021440.81386-1-ltaylorsimpson@gmail.com>
Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>

diff --git a/target/hexagon/gen_analyze_funcs.py b/target/hexagon/gen_analyze_funcs.py
index 3ac7cc2cfe577e7a3c2781dce6bb4fe0b2b0b376..fdefd5b4b361698da78e19b87c20036b23b0bd1f 100755 (executable)
--- a/target/hexagon/gen_analyze_funcs.py
+++ b/target/hexagon/gen_analyze_funcs.py
@@ -67,6 +67,8 @@ def gen_analyze_func(f, tag, regs, imms):
         if reg.is_read():
             reg.analyze_read(f, regno)
 
+    f.write("    mark_implicit_reads(ctx);\n")
+
     ## Analyze the register writes
     for regno, register in enumerate(regs):
         reg_type, reg_id = register
@@ -74,6 +76,8 @@ def gen_analyze_func(f, tag, regs, imms):
         if reg.is_written():
             reg.analyze_write(f, tag, regno)
 
+    f.write("    mark_implicit_writes(ctx);\n")
+
     f.write("}\n\n")
 
 

diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index f3240953b596bd5e7a9c57476c84cb202f373376..3762faec4d817124f01320f80533136468d396d2 100644 (file)
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -37,6 +37,10 @@
 #include "exec/helper-info.c.inc"
 #undef  HELPER_H
 
+/* Forward declarations referenced in analyze_funcs_generated.c.inc */
+static void mark_implicit_reads(DisasContext *ctx);
+static void mark_implicit_writes(DisasContext *ctx);
+
 #include "analyze_funcs_generated.c.inc"
 
 typedef void (*AnalyzeInsn)(DisasContext *ctx);
@@ -377,6 +381,17 @@ static void mark_implicit_pred_reads(DisasContext *ctx)
     mark_implicit_pred_read(ctx, A_IMPLICIT_READS_P3, 3);
 }
 
+static void mark_implicit_reads(DisasContext *ctx)
+{
+    mark_implicit_pred_reads(ctx);
+}
+
+static void mark_implicit_writes(DisasContext *ctx)
+{
+    mark_implicit_reg_writes(ctx);
+    mark_implicit_pred_writes(ctx);
+}
+
 static void analyze_packet(DisasContext *ctx)
 {
     Packet *pkt = ctx->pkt;
@@ -388,9 +403,6 @@ static void analyze_packet(DisasContext *ctx)
         if (opcode_analyze[insn->opcode]) {
             opcode_analyze[insn->opcode](ctx);
         }
-        mark_implicit_reg_writes(ctx);
-        mark_implicit_pred_writes(ctx);
-        mark_implicit_pred_reads(ctx);
     }
 
     ctx->need_commit = need_commit(ctx);