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 <ltaylorsimp...@gmail.com>
---
target/hexagon/translate.c | 18 +++++++++++++++---
target/hexagon/gen_analyze_funcs.py | 4 ++++
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index fe7858703c..5271c4e022 100644
--- 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);
@@ -378,6 +382,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;
@@ -389,9 +404,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);
diff --git a/target/hexagon/gen_analyze_funcs.py
b/target/hexagon/gen_analyze_funcs.py
index 3ac7cc2cfe..fdefd5b4b3 100755
--- 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")
--
2.43.0
