| Message ID | 20240201103340.119081-4-ltaylorsimpson@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Hexagon (target/hexagon) Enable more short-circuit packets | expand |
> -----Original Message----- > From: Taylor Simpson <ltaylorsimpson@gmail.com> > Sent: Thursday, February 1, 2024 4:34 AM > To: qemu-devel@nongnu.org > Cc: Brian Cain <bcain@quicinc.com>; Matheus Bernardino (QUIC) > <quic_mathbern@quicinc.com>; Sid Manning <sidneym@quicinc.com>; > Marco Liebel (QUIC) <quic_mliebel@quicinc.com>; > richard.henderson@linaro.org; philmd@linaro.org; ale@rev.ng; anjo@rev.ng; > ltaylorsimpson@gmail.com > Subject: [PATCH v2 3/3] Hexagon (target/hexagon) Enable more short-circuit > packets (HVX) > > WARNING: This email originated from outside of Qualcomm. Please be wary > of any links or attachments, and do not enable macros. > > Look for read-after-write instead of overlap of reads and writes > > HVX instructions with helpers have pass-by-reference semantics, so > we check for overlaps of reads and writes within the same instruction. > > Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com> > --- > target/hexagon/translate.h | 88 +++++++++++++++++++++++------ > target/hexagon/translate.c | 58 ++----------------- > target/hexagon/gen_analyze_funcs.py | 19 ++++--- > target/hexagon/hex_common.py | 45 ++++++++++----- > 4 files changed, 115 insertions(+), 95 deletions(-) > > diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h > index d5e7f49ad8..00cc2bcd63 100644 > --- a/target/hexagon/translate.h > +++ b/target/hexagon/translate.h > @@ -50,23 +50,27 @@ typedef struct DisasContext { > int tmp_vregs_num[VECTOR_TEMPS_MAX]; > int vreg_log[NUM_VREGS]; > int vreg_log_idx; > + DECLARE_BITMAP(vregs_written, NUM_VREGS); > + DECLARE_BITMAP(insn_vregs_written, NUM_VREGS); > DECLARE_BITMAP(vregs_updated_tmp, NUM_VREGS); > DECLARE_BITMAP(vregs_updated, NUM_VREGS); > DECLARE_BITMAP(vregs_select, NUM_VREGS); > DECLARE_BITMAP(predicated_future_vregs, NUM_VREGS); > DECLARE_BITMAP(predicated_tmp_vregs, NUM_VREGS); > - DECLARE_BITMAP(vregs_read, NUM_VREGS); > + DECLARE_BITMAP(insn_vregs_read, NUM_VREGS); > int qreg_log[NUM_QREGS]; > int qreg_log_idx; > - DECLARE_BITMAP(qregs_read, NUM_QREGS); > + DECLARE_BITMAP(qregs_written, NUM_QREGS); > + DECLARE_BITMAP(insn_qregs_written, NUM_QREGS); > + DECLARE_BITMAP(insn_qregs_read, NUM_QREGS); > bool pre_commit; > bool need_commit; > TCGCond branch_cond; > target_ulong branch_dest; > bool is_tight_loop; > bool short_circuit; > - bool has_hvx_helper; > bool read_after_write; > + bool has_hvx_overlap; > TCGv new_value[TOTAL_PER_THREAD_REGS]; > TCGv new_pred_value[NUM_PREGS]; > TCGv pred_written; > @@ -146,10 +150,25 @@ intptr_t ctx_future_vreg_off(DisasContext *ctx, int > regnum, > intptr_t ctx_tmp_vreg_off(DisasContext *ctx, int regnum, > int num, bool alloc_ok); > > +static inline void ctx_start_hvx_insn(DisasContext *ctx) > +{ > + bitmap_zero(ctx->insn_vregs_written, NUM_VREGS); > + bitmap_zero(ctx->insn_vregs_read, NUM_VREGS); > + bitmap_zero(ctx->insn_qregs_written, NUM_QREGS); > + bitmap_zero(ctx->insn_qregs_read, NUM_QREGS); > +} > + > static inline void ctx_log_vreg_write(DisasContext *ctx, > int rnum, VRegWriteType type, > - bool is_predicated) > + bool is_predicated, bool has_helper) > { > + if (has_helper) { > + set_bit(rnum, ctx->insn_vregs_written); > + if (test_bit(rnum, ctx->insn_vregs_read)) { > + ctx->has_hvx_overlap = true; > + } > + } > + set_bit(rnum, ctx->vregs_written); > if (type != EXT_TMP) { > if (!test_bit(rnum, ctx->vregs_updated)) { > ctx->vreg_log[ctx->vreg_log_idx] = rnum; > @@ -175,42 +194,77 @@ static inline void ctx_log_vreg_write(DisasContext > *ctx, > > static inline void ctx_log_vreg_write_pair(DisasContext *ctx, > int rnum, VRegWriteType type, > - bool is_predicated) > + bool is_predicated, bool has_helper) > { > - ctx_log_vreg_write(ctx, rnum ^ 0, type, is_predicated); > - ctx_log_vreg_write(ctx, rnum ^ 1, type, is_predicated); > + ctx_log_vreg_write(ctx, rnum ^ 0, type, is_predicated, has_helper); > + ctx_log_vreg_write(ctx, rnum ^ 1, type, is_predicated, has_helper); > } > > -static inline void ctx_log_vreg_read(DisasContext *ctx, int rnum) > +static inline void ctx_log_vreg_read(DisasContext *ctx, int rnum, > + bool has_helper) > { > - set_bit(rnum, ctx->vregs_read); > + if (has_helper) { > + set_bit(rnum, ctx->insn_vregs_read); > + if (test_bit(rnum, ctx->insn_vregs_written)) { > + ctx->has_hvx_overlap = true; > + } > + } > + if (test_bit(rnum, ctx->vregs_written)) { > + ctx->read_after_write = true; > + } > } > > -static inline void ctx_log_vreg_read_new(DisasContext *ctx, int rnum) > +static inline void ctx_log_vreg_read_new(DisasContext *ctx, int rnum, > + bool has_helper) > { > g_assert(is_gather_store_insn(ctx) || > test_bit(rnum, ctx->vregs_updated) || > test_bit(rnum, ctx->vregs_select) || > test_bit(rnum, ctx->vregs_updated_tmp)); > - set_bit(rnum, ctx->vregs_read); > + if (has_helper) { > + set_bit(rnum, ctx->insn_vregs_read); > + if (test_bit(rnum, ctx->insn_vregs_written)) { > + ctx->has_hvx_overlap = true; > + } > + } > + if (is_gather_store_insn(ctx)) { > + ctx->read_after_write = true; > + } > } > > -static inline void ctx_log_vreg_read_pair(DisasContext *ctx, int rnum) > +static inline void ctx_log_vreg_read_pair(DisasContext *ctx, int rnum, > + bool has_helper) > { > - ctx_log_vreg_read(ctx, rnum ^ 0); > - ctx_log_vreg_read(ctx, rnum ^ 1); > + ctx_log_vreg_read(ctx, rnum ^ 0, has_helper); > + ctx_log_vreg_read(ctx, rnum ^ 1, has_helper); > } > > static inline void ctx_log_qreg_write(DisasContext *ctx, > - int rnum) > + int rnum, bool has_helper) > { > + if (has_helper) { > + set_bit(rnum, ctx->insn_qregs_written); > + if (test_bit(rnum, ctx->insn_qregs_read)) { > + ctx->has_hvx_overlap = true; > + } > + } > + set_bit(rnum, ctx->qregs_written); > ctx->qreg_log[ctx->qreg_log_idx] = rnum; > ctx->qreg_log_idx++; > } > > -static inline void ctx_log_qreg_read(DisasContext *ctx, int qnum) > +static inline void ctx_log_qreg_read(DisasContext *ctx, > + int qnum, bool has_helper) > { > - set_bit(qnum, ctx->qregs_read); > + if (has_helper) { > + set_bit(qnum, ctx->insn_qregs_read); > + if (test_bit(qnum, ctx->insn_qregs_written)) { > + ctx->has_hvx_overlap = true; > + } > + } > + if (test_bit(qnum, ctx->qregs_written)) { > + ctx->read_after_write = true; > + } > } > > extern TCGv hex_gpr[TOTAL_PER_THREAD_REGS]; > diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c > index 751ca71790..ed4b4acd1d 100644 > --- a/target/hexagon/translate.c > +++ b/target/hexagon/translate.c > @@ -378,60 +378,10 @@ static bool need_commit(DisasContext *ctx) > return true; > } > > - if (pkt->num_insns == 1) { > - if (pkt->pkt_has_hvx) { > - /* > - * The HVX instructions with generated helpers use > - * pass-by-reference, so they need the read/write overlap > - * check below. > - * The HVX instructions with overrides are OK. > - */ > - if (!ctx->has_hvx_helper) { > - return false; > - } > - } else { > - return false; > - } > - } > - > - if (ctx->read_after_write) { > + if (ctx->read_after_write || ctx->has_hvx_overlap) { > return true; > } > > - /* Check for overlap between HVX reads and writes */ > - for (int i = 0; i < ctx->vreg_log_idx; i++) { > - int vnum = ctx->vreg_log[i]; > - if (test_bit(vnum, ctx->vregs_read)) { > - return true; > - } > - } > - if (!bitmap_empty(ctx->vregs_updated_tmp, NUM_VREGS)) { > - int i = find_first_bit(ctx->vregs_updated_tmp, NUM_VREGS); > - while (i < NUM_VREGS) { > - if (test_bit(i, ctx->vregs_read)) { > - return true; > - } > - i = find_next_bit(ctx->vregs_updated_tmp, NUM_VREGS, i + 1); > - } > - } > - if (!bitmap_empty(ctx->vregs_select, NUM_VREGS)) { > - int i = find_first_bit(ctx->vregs_select, NUM_VREGS); > - while (i < NUM_VREGS) { > - if (test_bit(i, ctx->vregs_read)) { > - return true; > - } > - i = find_next_bit(ctx->vregs_select, NUM_VREGS, i + 1); > - } > - } > - > - /* Check for overlap between HVX predicate reads and writes */ > - for (int i = 0; i < ctx->qreg_log_idx; i++) { > - int qnum = ctx->qreg_log[i]; > - if (test_bit(qnum, ctx->qregs_read)) { > - return true; > - } > - } > - > return false; > } > > @@ -453,8 +403,8 @@ static void mark_implicit_pred_reads(DisasContext > *ctx) > static void analyze_packet(DisasContext *ctx) > { > Packet *pkt = ctx->pkt; > - ctx->has_hvx_helper = false; > 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; > @@ -485,13 +435,13 @@ static void gen_start_packet(DisasContext *ctx) > ctx->future_vregs_idx = 0; > ctx->tmp_vregs_idx = 0; > ctx->vreg_log_idx = 0; > + bitmap_zero(ctx->vregs_written, NUM_VREGS); > bitmap_zero(ctx->vregs_updated_tmp, NUM_VREGS); > bitmap_zero(ctx->vregs_updated, NUM_VREGS); > bitmap_zero(ctx->vregs_select, NUM_VREGS); > bitmap_zero(ctx->predicated_future_vregs, NUM_VREGS); > bitmap_zero(ctx->predicated_tmp_vregs, NUM_VREGS); > - bitmap_zero(ctx->vregs_read, NUM_VREGS); > - bitmap_zero(ctx->qregs_read, NUM_QREGS); > + bitmap_zero(ctx->qregs_written, NUM_QREGS); > ctx->qreg_log_idx = 0; > for (i = 0; i < STORES_MAX; i++) { > ctx->store_width[i] = 0; > diff --git a/target/hexagon/gen_analyze_funcs.py > b/target/hexagon/gen_analyze_funcs.py > index 890e6a3a95..81e1d9cfa3 100755 > --- a/target/hexagon/gen_analyze_funcs.py > +++ b/target/hexagon/gen_analyze_funcs.py > @@ -43,6 +43,16 @@ def gen_analyze_func(f, tag, regs, imms): > f.write("{\n") > > f.write(" Insn *insn G_GNUC_UNUSED = ctx->insn;\n") > + if (hex_common.is_hvx_insn(tag)): > + if hex_common.has_hvx_helper(tag): > + f.write( > + " const bool G_GNUC_UNUSED insn_has_hvx_helper = true;\n" > + ) > + f.write(" ctx_start_hvx_insn(ctx);\n") > + else: > + f.write( > + " const bool G_GNUC_UNUSED insn_has_hvx_helper = false;\n" > + ) > > ## Declare all the registers > for regno, register in enumerate(regs): > @@ -64,15 +74,6 @@ def gen_analyze_func(f, tag, regs, imms): > if reg.is_written(): > reg.analyze_write(f, tag, regno) > > - has_generated_helper = not hex_common.skip_qemu_helper( > - tag > - ) and not hex_common.is_idef_parser_enabled(tag) > - > - ## Mark HVX instructions with generated helpers > - if (has_generated_helper and > - "A_CVI" in hex_common.attribdict[tag]): > - f.write(" ctx->has_hvx_helper = true;\n") > - > f.write("}\n\n") > > > diff --git a/target/hexagon/hex_common.py > b/target/hexagon/hex_common.py > index 33801e4bd7..9e7f613e3c 100755 > --- a/target/hexagon/hex_common.py > +++ b/target/hexagon/hex_common.py > @@ -241,6 +241,16 @@ def is_idef_parser_enabled(tag): > return tag in idef_parser_enabled > > > +def is_hvx_insn(tag): > + return "A_CVI" in attribdict[tag] > + > + > +def has_hvx_helper(tag): > + return (is_hvx_insn(tag) and > + not skip_qemu_helper(tag) and > + not is_idef_parser_enabled(tag)) > + > + > def imm_name(immlett): > return f"{immlett}iV" > > @@ -704,7 +714,8 @@ def analyze_write(self, f, tag, regno): > newv = hvx_newv(tag) > predicated = "true" if is_predicated(tag) else "false" > f.write(code_fmt(f"""\ > - ctx_log_vreg_write(ctx, {self.reg_num}, {newv}, {predicated}); > + ctx_log_vreg_write(ctx, {self.reg_num}, {newv}, {predicated}, > + insn_has_hvx_helper); > """)) > > class VRegSource(Register, Hvx, OldSource): > @@ -724,7 +735,7 @@ def helper_hvx_desc(self, f): > """)) > def analyze_read(self, f, regno): > f.write(code_fmt(f"""\ > - ctx_log_vreg_read(ctx, {self.reg_num}); > + ctx_log_vreg_read(ctx, {self.reg_num}, insn_has_hvx_helper); > """)) > > class VRegNewSource(Register, Hvx, NewSource): > @@ -741,7 +752,7 @@ def helper_hvx_desc(self, f): > """)) > def analyze_read(self, f, regno): > f.write(code_fmt(f"""\ > - ctx_log_vreg_read_new(ctx, {self.reg_num}); > + ctx_log_vreg_read_new(ctx, {self.reg_num}, insn_has_hvx_helper); > """)) > > class VRegReadWrite(Register, Hvx, ReadWrite): > @@ -767,13 +778,14 @@ def helper_hvx_desc(self, f): > """)) > def analyze_read(self, f, regno): > f.write(code_fmt(f"""\ > - ctx_log_vreg_read(ctx, {self.reg_num}); > + ctx_log_vreg_read(ctx, {self.reg_num}, insn_has_hvx_helper); > """)) > def analyze_write(self, f, tag, regno): > newv = hvx_newv(tag) > predicated = "true" if is_predicated(tag) else "false" > f.write(code_fmt(f"""\ > - ctx_log_vreg_write(ctx, {self.reg_num}, {newv}, {predicated}); > + ctx_log_vreg_write(ctx, {self.reg_num}, {newv}, {predicated}, > + insn_has_hvx_helper); > """)) > > class VRegTmp(Register, Hvx, ReadWrite): > @@ -801,13 +813,14 @@ def helper_hvx_desc(self, f): > """)) > def analyze_read(self, f, regno): > f.write(code_fmt(f"""\ > - ctx_log_vreg_read(ctx, {self.reg_num}); > + ctx_log_vreg_read(ctx, {self.reg_num}, insn_has_hvx_helper); > """)) > def analyze_write(self, f, tag, regno): > newv = hvx_newv(tag) > predicated = "true" if is_predicated(tag) else "false" > f.write(code_fmt(f"""\ > - ctx_log_vreg_write(ctx, {self.reg_num}, {newv}, {predicated}); > + ctx_log_vreg_write(ctx, {self.reg_num}, {newv}, {predicated}, > + insn_has_hvx_helper); > """)) > > class VRegPairDest(Register, Hvx, Dest): > @@ -832,7 +845,8 @@ def analyze_write(self, f, tag, regno): > newv = hvx_newv(tag) > predicated = "true" if is_predicated(tag) else "false" > f.write(code_fmt(f"""\ > - ctx_log_vreg_write_pair(ctx, {self.reg_num}, {newv}, {predicated}); > + ctx_log_vreg_write_pair(ctx, {self.reg_num}, {newv}, {predicated}, > + insn_has_hvx_helper); > """)) > > class VRegPairSource(Register, Hvx, OldSource): > @@ -859,7 +873,7 @@ def helper_hvx_desc(self, f): > """)) > def analyze_read(self, f, regno): > f.write(code_fmt(f"""\ > - ctx_log_vreg_read_pair(ctx, {self.reg_num}); > + ctx_log_vreg_read_pair(ctx, {self.reg_num}, insn_has_hvx_helper); > """)) > > class VRegPairReadWrite(Register, Hvx, ReadWrite): > @@ -891,13 +905,14 @@ def helper_hvx_desc(self, f): > """)) > def analyze_read(self, f, regno): > f.write(code_fmt(f"""\ > - ctx_log_vreg_read_pair(ctx, {self.reg_num}); > + ctx_log_vreg_read_pair(ctx, {self.reg_num}, insn_has_hvx_helper); > """)) > def analyze_write(self, f, tag, regno): > newv = hvx_newv(tag) > predicated = "true" if is_predicated(tag) else "false" > f.write(code_fmt(f"""\ > - ctx_log_vreg_write_pair(ctx, {self.reg_num}, {newv}, {predicated}); > + ctx_log_vreg_write_pair(ctx, {self.reg_num}, {newv}, {predicated}, > + insn_has_hvx_helper); > """)) > > class QRegDest(Register, Hvx, Dest): > @@ -920,7 +935,7 @@ def helper_hvx_desc(self, f): > """)) > def analyze_write(self, f, tag, regno): > f.write(code_fmt(f"""\ > - ctx_log_qreg_write(ctx, {self.reg_num}); > + ctx_log_qreg_write(ctx, {self.reg_num}, insn_has_hvx_helper); > """)) > > class QRegSource(Register, Hvx, OldSource): > @@ -941,7 +956,7 @@ def helper_hvx_desc(self, f): > """)) > def analyze_read(self, f, regno): > f.write(code_fmt(f"""\ > - ctx_log_qreg_read(ctx, {self.reg_num}); > + ctx_log_qreg_read(ctx, {self.reg_num}, insn_has_hvx_helper); > """)) > > class QRegReadWrite(Register, Hvx, ReadWrite): > @@ -967,11 +982,11 @@ def helper_hvx_desc(self, f): > """)) > def analyze_read(self, f, regno): > f.write(code_fmt(f"""\ > - ctx_log_qreg_read(ctx, {self.reg_num}); > + ctx_log_qreg_read(ctx, {self.reg_num}, insn_has_hvx_helper); > """)) > def analyze_write(self, f, tag, regno): > f.write(code_fmt(f"""\ > - ctx_log_qreg_write(ctx, {self.reg_num}); > + ctx_log_qreg_write(ctx, {self.reg_num}, insn_has_hvx_helper); > """)) > > def init_registers(): > -- > 2.34.1 Reviewed-by: Brian Cain <bcain@quicinc.com>
diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h index d5e7f49ad8..00cc2bcd63 100644 --- a/target/hexagon/translate.h +++ b/target/hexagon/translate.h @@ -50,23 +50,27 @@ typedef struct DisasContext { int tmp_vregs_num[VECTOR_TEMPS_MAX]; int vreg_log[NUM_VREGS]; int vreg_log_idx; + DECLARE_BITMAP(vregs_written, NUM_VREGS); + DECLARE_BITMAP(insn_vregs_written, NUM_VREGS); DECLARE_BITMAP(vregs_updated_tmp, NUM_VREGS); DECLARE_BITMAP(vregs_updated, NUM_VREGS); DECLARE_BITMAP(vregs_select, NUM_VREGS); DECLARE_BITMAP(predicated_future_vregs, NUM_VREGS); DECLARE_BITMAP(predicated_tmp_vregs, NUM_VREGS); - DECLARE_BITMAP(vregs_read, NUM_VREGS); + DECLARE_BITMAP(insn_vregs_read, NUM_VREGS); int qreg_log[NUM_QREGS]; int qreg_log_idx; - DECLARE_BITMAP(qregs_read, NUM_QREGS); + DECLARE_BITMAP(qregs_written, NUM_QREGS); + DECLARE_BITMAP(insn_qregs_written, NUM_QREGS); + DECLARE_BITMAP(insn_qregs_read, NUM_QREGS); bool pre_commit; bool need_commit; TCGCond branch_cond; target_ulong branch_dest; bool is_tight_loop; bool short_circuit; - bool has_hvx_helper; bool read_after_write; + bool has_hvx_overlap; TCGv new_value[TOTAL_PER_THREAD_REGS]; TCGv new_pred_value[NUM_PREGS]; TCGv pred_written; @@ -146,10 +150,25 @@ intptr_t ctx_future_vreg_off(DisasContext *ctx, int regnum, intptr_t ctx_tmp_vreg_off(DisasContext *ctx, int regnum, int num, bool alloc_ok); +static inline void ctx_start_hvx_insn(DisasContext *ctx) +{ + bitmap_zero(ctx->insn_vregs_written, NUM_VREGS); + bitmap_zero(ctx->insn_vregs_read, NUM_VREGS); + bitmap_zero(ctx->insn_qregs_written, NUM_QREGS); + bitmap_zero(ctx->insn_qregs_read, NUM_QREGS); +} + static inline void ctx_log_vreg_write(DisasContext *ctx, int rnum, VRegWriteType type, - bool is_predicated) + bool is_predicated, bool has_helper) { + if (has_helper) { + set_bit(rnum, ctx->insn_vregs_written); + if (test_bit(rnum, ctx->insn_vregs_read)) { + ctx->has_hvx_overlap = true; + } + } + set_bit(rnum, ctx->vregs_written); if (type != EXT_TMP) { if (!test_bit(rnum, ctx->vregs_updated)) { ctx->vreg_log[ctx->vreg_log_idx] = rnum; @@ -175,42 +194,77 @@ static inline void ctx_log_vreg_write(DisasContext *ctx, static inline void ctx_log_vreg_write_pair(DisasContext *ctx, int rnum, VRegWriteType type, - bool is_predicated) + bool is_predicated, bool has_helper) { - ctx_log_vreg_write(ctx, rnum ^ 0, type, is_predicated); - ctx_log_vreg_write(ctx, rnum ^ 1, type, is_predicated); + ctx_log_vreg_write(ctx, rnum ^ 0, type, is_predicated, has_helper); + ctx_log_vreg_write(ctx, rnum ^ 1, type, is_predicated, has_helper); } -static inline void ctx_log_vreg_read(DisasContext *ctx, int rnum) +static inline void ctx_log_vreg_read(DisasContext *ctx, int rnum, + bool has_helper) { - set_bit(rnum, ctx->vregs_read); + if (has_helper) { + set_bit(rnum, ctx->insn_vregs_read); + if (test_bit(rnum, ctx->insn_vregs_written)) { + ctx->has_hvx_overlap = true; + } + } + if (test_bit(rnum, ctx->vregs_written)) { + ctx->read_after_write = true; + } } -static inline void ctx_log_vreg_read_new(DisasContext *ctx, int rnum) +static inline void ctx_log_vreg_read_new(DisasContext *ctx, int rnum, + bool has_helper) { g_assert(is_gather_store_insn(ctx) || test_bit(rnum, ctx->vregs_updated) || test_bit(rnum, ctx->vregs_select) || test_bit(rnum, ctx->vregs_updated_tmp)); - set_bit(rnum, ctx->vregs_read); + if (has_helper) { + set_bit(rnum, ctx->insn_vregs_read); + if (test_bit(rnum, ctx->insn_vregs_written)) { + ctx->has_hvx_overlap = true; + } + } + if (is_gather_store_insn(ctx)) { + ctx->read_after_write = true; + } } -static inline void ctx_log_vreg_read_pair(DisasContext *ctx, int rnum) +static inline void ctx_log_vreg_read_pair(DisasContext *ctx, int rnum, + bool has_helper) { - ctx_log_vreg_read(ctx, rnum ^ 0); - ctx_log_vreg_read(ctx, rnum ^ 1); + ctx_log_vreg_read(ctx, rnum ^ 0, has_helper); + ctx_log_vreg_read(ctx, rnum ^ 1, has_helper); } static inline void ctx_log_qreg_write(DisasContext *ctx, - int rnum) + int rnum, bool has_helper) { + if (has_helper) { + set_bit(rnum, ctx->insn_qregs_written); + if (test_bit(rnum, ctx->insn_qregs_read)) { + ctx->has_hvx_overlap = true; + } + } + set_bit(rnum, ctx->qregs_written); ctx->qreg_log[ctx->qreg_log_idx] = rnum; ctx->qreg_log_idx++; } -static inline void ctx_log_qreg_read(DisasContext *ctx, int qnum) +static inline void ctx_log_qreg_read(DisasContext *ctx, + int qnum, bool has_helper) { - set_bit(qnum, ctx->qregs_read); + if (has_helper) { + set_bit(qnum, ctx->insn_qregs_read); + if (test_bit(qnum, ctx->insn_qregs_written)) { + ctx->has_hvx_overlap = true; + } + } + if (test_bit(qnum, ctx->qregs_written)) { + ctx->read_after_write = true; + } } extern TCGv hex_gpr[TOTAL_PER_THREAD_REGS]; diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c index 751ca71790..ed4b4acd1d 100644 --- a/target/hexagon/translate.c +++ b/target/hexagon/translate.c @@ -378,60 +378,10 @@ static bool need_commit(DisasContext *ctx) return true; } - if (pkt->num_insns == 1) { - if (pkt->pkt_has_hvx) { - /* - * The HVX instructions with generated helpers use - * pass-by-reference, so they need the read/write overlap - * check below. - * The HVX instructions with overrides are OK. - */ - if (!ctx->has_hvx_helper) { - return false; - } - } else { - return false; - } - } - - if (ctx->read_after_write) { + if (ctx->read_after_write || ctx->has_hvx_overlap) { return true; } - /* Check for overlap between HVX reads and writes */ - for (int i = 0; i < ctx->vreg_log_idx; i++) { - int vnum = ctx->vreg_log[i]; - if (test_bit(vnum, ctx->vregs_read)) { - return true; - } - } - if (!bitmap_empty(ctx->vregs_updated_tmp, NUM_VREGS)) { - int i = find_first_bit(ctx->vregs_updated_tmp, NUM_VREGS); - while (i < NUM_VREGS) { - if (test_bit(i, ctx->vregs_read)) { - return true; - } - i = find_next_bit(ctx->vregs_updated_tmp, NUM_VREGS, i + 1); - } - } - if (!bitmap_empty(ctx->vregs_select, NUM_VREGS)) { - int i = find_first_bit(ctx->vregs_select, NUM_VREGS); - while (i < NUM_VREGS) { - if (test_bit(i, ctx->vregs_read)) { - return true; - } - i = find_next_bit(ctx->vregs_select, NUM_VREGS, i + 1); - } - } - - /* Check for overlap between HVX predicate reads and writes */ - for (int i = 0; i < ctx->qreg_log_idx; i++) { - int qnum = ctx->qreg_log[i]; - if (test_bit(qnum, ctx->qregs_read)) { - return true; - } - } - return false; } @@ -453,8 +403,8 @@ static void mark_implicit_pred_reads(DisasContext *ctx) static void analyze_packet(DisasContext *ctx) { Packet *pkt = ctx->pkt; - ctx->has_hvx_helper = false; 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; @@ -485,13 +435,13 @@ static void gen_start_packet(DisasContext *ctx) ctx->future_vregs_idx = 0; ctx->tmp_vregs_idx = 0; ctx->vreg_log_idx = 0; + bitmap_zero(ctx->vregs_written, NUM_VREGS); bitmap_zero(ctx->vregs_updated_tmp, NUM_VREGS); bitmap_zero(ctx->vregs_updated, NUM_VREGS); bitmap_zero(ctx->vregs_select, NUM_VREGS); bitmap_zero(ctx->predicated_future_vregs, NUM_VREGS); bitmap_zero(ctx->predicated_tmp_vregs, NUM_VREGS); - bitmap_zero(ctx->vregs_read, NUM_VREGS); - bitmap_zero(ctx->qregs_read, NUM_QREGS); + bitmap_zero(ctx->qregs_written, NUM_QREGS); ctx->qreg_log_idx = 0; for (i = 0; i < STORES_MAX; i++) { ctx->store_width[i] = 0; diff --git a/target/hexagon/gen_analyze_funcs.py b/target/hexagon/gen_analyze_funcs.py index 890e6a3a95..81e1d9cfa3 100755 --- a/target/hexagon/gen_analyze_funcs.py +++ b/target/hexagon/gen_analyze_funcs.py @@ -43,6 +43,16 @@ def gen_analyze_func(f, tag, regs, imms): f.write("{\n") f.write(" Insn *insn G_GNUC_UNUSED = ctx->insn;\n") + if (hex_common.is_hvx_insn(tag)): + if hex_common.has_hvx_helper(tag): + f.write( + " const bool G_GNUC_UNUSED insn_has_hvx_helper = true;\n" + ) + f.write(" ctx_start_hvx_insn(ctx);\n") + else: + f.write( + " const bool G_GNUC_UNUSED insn_has_hvx_helper = false;\n" + ) ## Declare all the registers for regno, register in enumerate(regs): @@ -64,15 +74,6 @@ def gen_analyze_func(f, tag, regs, imms): if reg.is_written(): reg.analyze_write(f, tag, regno) - has_generated_helper = not hex_common.skip_qemu_helper( - tag - ) and not hex_common.is_idef_parser_enabled(tag) - - ## Mark HVX instructions with generated helpers - if (has_generated_helper and - "A_CVI" in hex_common.attribdict[tag]): - f.write(" ctx->has_hvx_helper = true;\n") - f.write("}\n\n") diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py index 33801e4bd7..9e7f613e3c 100755 --- a/target/hexagon/hex_common.py +++ b/target/hexagon/hex_common.py @@ -241,6 +241,16 @@ def is_idef_parser_enabled(tag): return tag in idef_parser_enabled +def is_hvx_insn(tag): + return "A_CVI" in attribdict[tag] + + +def has_hvx_helper(tag): + return (is_hvx_insn(tag) and + not skip_qemu_helper(tag) and + not is_idef_parser_enabled(tag)) + + def imm_name(immlett): return f"{immlett}iV" @@ -704,7 +714,8 @@ def analyze_write(self, f, tag, regno): newv = hvx_newv(tag) predicated = "true" if is_predicated(tag) else "false" f.write(code_fmt(f"""\ - ctx_log_vreg_write(ctx, {self.reg_num}, {newv}, {predicated}); + ctx_log_vreg_write(ctx, {self.reg_num}, {newv}, {predicated}, + insn_has_hvx_helper); """)) class VRegSource(Register, Hvx, OldSource): @@ -724,7 +735,7 @@ def helper_hvx_desc(self, f): """)) def analyze_read(self, f, regno): f.write(code_fmt(f"""\ - ctx_log_vreg_read(ctx, {self.reg_num}); + ctx_log_vreg_read(ctx, {self.reg_num}, insn_has_hvx_helper); """)) class VRegNewSource(Register, Hvx, NewSource): @@ -741,7 +752,7 @@ def helper_hvx_desc(self, f): """)) def analyze_read(self, f, regno): f.write(code_fmt(f"""\ - ctx_log_vreg_read_new(ctx, {self.reg_num}); + ctx_log_vreg_read_new(ctx, {self.reg_num}, insn_has_hvx_helper); """)) class VRegReadWrite(Register, Hvx, ReadWrite): @@ -767,13 +778,14 @@ def helper_hvx_desc(self, f): """)) def analyze_read(self, f, regno): f.write(code_fmt(f"""\ - ctx_log_vreg_read(ctx, {self.reg_num}); + ctx_log_vreg_read(ctx, {self.reg_num}, insn_has_hvx_helper); """)) def analyze_write(self, f, tag, regno): newv = hvx_newv(tag) predicated = "true" if is_predicated(tag) else "false" f.write(code_fmt(f"""\ - ctx_log_vreg_write(ctx, {self.reg_num}, {newv}, {predicated}); + ctx_log_vreg_write(ctx, {self.reg_num}, {newv}, {predicated}, + insn_has_hvx_helper); """)) class VRegTmp(Register, Hvx, ReadWrite): @@ -801,13 +813,14 @@ def helper_hvx_desc(self, f): """)) def analyze_read(self, f, regno): f.write(code_fmt(f"""\ - ctx_log_vreg_read(ctx, {self.reg_num}); + ctx_log_vreg_read(ctx, {self.reg_num}, insn_has_hvx_helper); """)) def analyze_write(self, f, tag, regno): newv = hvx_newv(tag) predicated = "true" if is_predicated(tag) else "false" f.write(code_fmt(f"""\ - ctx_log_vreg_write(ctx, {self.reg_num}, {newv}, {predicated}); + ctx_log_vreg_write(ctx, {self.reg_num}, {newv}, {predicated}, + insn_has_hvx_helper); """)) class VRegPairDest(Register, Hvx, Dest): @@ -832,7 +845,8 @@ def analyze_write(self, f, tag, regno): newv = hvx_newv(tag) predicated = "true" if is_predicated(tag) else "false" f.write(code_fmt(f"""\ - ctx_log_vreg_write_pair(ctx, {self.reg_num}, {newv}, {predicated}); + ctx_log_vreg_write_pair(ctx, {self.reg_num}, {newv}, {predicated}, + insn_has_hvx_helper); """)) class VRegPairSource(Register, Hvx, OldSource): @@ -859,7 +873,7 @@ def helper_hvx_desc(self, f): """)) def analyze_read(self, f, regno): f.write(code_fmt(f"""\ - ctx_log_vreg_read_pair(ctx, {self.reg_num}); + ctx_log_vreg_read_pair(ctx, {self.reg_num}, insn_has_hvx_helper); """)) class VRegPairReadWrite(Register, Hvx, ReadWrite): @@ -891,13 +905,14 @@ def helper_hvx_desc(self, f): """)) def analyze_read(self, f, regno): f.write(code_fmt(f"""\ - ctx_log_vreg_read_pair(ctx, {self.reg_num}); + ctx_log_vreg_read_pair(ctx, {self.reg_num}, insn_has_hvx_helper); """)) def analyze_write(self, f, tag, regno): newv = hvx_newv(tag) predicated = "true" if is_predicated(tag) else "false" f.write(code_fmt(f"""\ - ctx_log_vreg_write_pair(ctx, {self.reg_num}, {newv}, {predicated}); + ctx_log_vreg_write_pair(ctx, {self.reg_num}, {newv}, {predicated}, + insn_has_hvx_helper); """)) class QRegDest(Register, Hvx, Dest): @@ -920,7 +935,7 @@ def helper_hvx_desc(self, f): """)) def analyze_write(self, f, tag, regno): f.write(code_fmt(f"""\ - ctx_log_qreg_write(ctx, {self.reg_num}); + ctx_log_qreg_write(ctx, {self.reg_num}, insn_has_hvx_helper); """)) class QRegSource(Register, Hvx, OldSource): @@ -941,7 +956,7 @@ def helper_hvx_desc(self, f): """)) def analyze_read(self, f, regno): f.write(code_fmt(f"""\ - ctx_log_qreg_read(ctx, {self.reg_num}); + ctx_log_qreg_read(ctx, {self.reg_num}, insn_has_hvx_helper); """)) class QRegReadWrite(Register, Hvx, ReadWrite): @@ -967,11 +982,11 @@ def helper_hvx_desc(self, f): """)) def analyze_read(self, f, regno): f.write(code_fmt(f"""\ - ctx_log_qreg_read(ctx, {self.reg_num}); + ctx_log_qreg_read(ctx, {self.reg_num}, insn_has_hvx_helper); """)) def analyze_write(self, f, tag, regno): f.write(code_fmt(f"""\ - ctx_log_qreg_write(ctx, {self.reg_num}); + ctx_log_qreg_write(ctx, {self.reg_num}, insn_has_hvx_helper); """)) def init_registers():
Look for read-after-write instead of overlap of reads and writes HVX instructions with helpers have pass-by-reference semantics, so we check for overlaps of reads and writes within the same instruction. Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com> --- target/hexagon/translate.h | 88 +++++++++++++++++++++++------ target/hexagon/translate.c | 58 ++----------------- target/hexagon/gen_analyze_funcs.py | 19 ++++--- target/hexagon/hex_common.py | 45 ++++++++++----- 4 files changed, 115 insertions(+), 95 deletions(-)