> -----Original Message-----
> From: Taylor Simpson <ltaylorsimp...@gmail.com>
> Sent: Thursday, February 1, 2024 4:34 AM
> To: qemu-devel@nongnu.org
> Cc: Brian Cain <bc...@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathb...@quicinc.com>; Sid Manning <sidn...@quicinc.com>;
> Marco Liebel (QUIC) <quic_mlie...@quicinc.com>;
> richard.hender...@linaro.org; phi...@linaro.org; a...@rev.ng; a...@rev.ng;
> ltaylorsimp...@gmail.com
> Subject: [PATCH v2 2/3] Hexagon (target/hexagon) Enable more short-circuit
> packets (scalar core)
>
> 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
>
> Here is an example with overalp but no read-after-write:
> 0x000200fc: 0x38103876 { R0 = add(R0,R1); R6 = add(R6,R7) }
>
> BEFORE:
> ---- 00000000000200fc
> mov_i32 loc2,$0x0
> mov_i32 loc2,r0
> add_i32 loc3,loc2,r1
> mov_i32 loc2,loc3
> mov_i32 loc4,$0x0
> mov_i32 loc4,r6
> add_i32 loc5,loc4,r7
> mov_i32 loc4,loc5
> mov_i32 r0,loc2
> mov_i32 r6,loc4
>
> AFTER:
> ---- 00000000000200fc
> add_i32 loc2,r0,r1
> mov_i32 r0,loc2
> add_i32 loc3,r6,r7
> mov_i32 r6,loc3
>
> We can also short-circuit packets with .new values by reading from the
> real destination instead of the temporary.
> 0x00020100: 0x78005ff3 { R19 = #0xff
> 0x00020104: 0x2002e204 if (cmp.eq(N19.new,R2)) jump:t PC+8 }
>
> BEFORE:
> ---- 0000000000020100
> mov_i32 pc,$0x20108
> mov_i32 loc8,$0x0
> mov_i32 loc8,$0xff
> setcond_i32 loc10,loc8,r2,eq
> mov_i32 loc6,loc10
> mov_i32 r19,loc8
> add_i32 pkt_cnt,pkt_cnt,$0x2
> add_i32 insn_cnt,insn_cnt,$0x4
> brcond_i32 loc6,$0x0,eq,$L1
> goto_tb $0x0
> mov_i32 pc,$0x20108
> exit_tb $0x7fbb54000040
> set_label $L1
> goto_tb $0x1
> exit_tb $0x7fbb54000041
> set_label $L0
> exit_tb $0x7fbb54000043
>
> AFTER:
> ---- 0000000000020100
> mov_i32 pc,$0x20108
> mov_i32 r19,$0xff
> setcond_i32 loc7,r19,r2,eq
> mov_i32 loc4,loc7
> add_i32 pkt_cnt,pkt_cnt,$0x2
> add_i32 insn_cnt,insn_cnt,$0x4
> brcond_i32 loc4,$0x0,eq,$L1
> goto_tb $0x0
> mov_i32 pc,$0x20108
> exit_tb $0x7f9764000040
> set_label $L1
> goto_tb $0x1
> exit_tb $0x7f9764000041
> set_label $L0
> exit_tb $0x7f9764000043
>
> Signed-off-by: Taylor Simpson <ltaylorsimp...@gmail.com>
> ---
> target/hexagon/translate.h | 13 +++++++------
> target/hexagon/translate.c | 21 ++++-----------------
> 2 files changed, 11 insertions(+), 23 deletions(-)
>
> diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h
> index f06d71fc53..d5e7f49ad8 100644
> --- a/target/hexagon/translate.h
> +++ b/target/hexagon/translate.h
> @@ -38,12 +38,10 @@ typedef struct DisasContext {
> int reg_log[REG_WRITES_MAX];
> int reg_log_idx;
> DECLARE_BITMAP(regs_written, TOTAL_PER_THREAD_REGS);
> - DECLARE_BITMAP(regs_read, TOTAL_PER_THREAD_REGS);
> DECLARE_BITMAP(predicated_regs, TOTAL_PER_THREAD_REGS);
> int preg_log[PRED_WRITES_MAX];
> int preg_log_idx;
> DECLARE_BITMAP(pregs_written, NUM_PREGS);
> - DECLARE_BITMAP(pregs_read, NUM_PREGS);
> uint8_t store_width[STORES_MAX];
> bool s1_store_processed;
> int future_vregs_idx;
> @@ -68,6 +66,7 @@ typedef struct DisasContext {
> bool is_tight_loop;
> bool short_circuit;
> bool has_hvx_helper;
> + bool read_after_write;
> TCGv new_value[TOTAL_PER_THREAD_REGS];
> TCGv new_pred_value[NUM_PREGS];
> TCGv pred_written;
> @@ -88,13 +87,14 @@ static inline void ctx_log_pred_write(DisasContext
> *ctx, int pnum)
>
> static inline void ctx_log_pred_read(DisasContext *ctx, int pnum)
> {
> - set_bit(pnum, ctx->pregs_read);
> + if (test_bit(pnum, ctx->pregs_written)) {
> + ctx->read_after_write = true;
> + }
> }
>
> static inline void ctx_log_pred_read_new(DisasContext *ctx, int pnum)
> {
> g_assert(test_bit(pnum, ctx->pregs_written));
> - set_bit(pnum, ctx->pregs_read);
> }
>
> static inline void ctx_log_reg_write(DisasContext *ctx, int rnum,
> @@ -125,13 +125,14 @@ static inline void
> ctx_log_reg_write_pair(DisasContext *ctx, int rnum,
>
> static inline void ctx_log_reg_read(DisasContext *ctx, int rnum)
> {
> - set_bit(rnum, ctx->regs_read);
> + if (test_bit(rnum, ctx->regs_written)) {
> + ctx->read_after_write = true;
> + }
> }
>
> static inline void ctx_log_reg_read_new(DisasContext *ctx, int rnum)
> {
> g_assert(test_bit(rnum, ctx->regs_written));
> - set_bit(rnum, ctx->regs_read);
> }
>
> static inline void ctx_log_reg_read_pair(DisasContext *ctx, int rnum)
> diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
> index 95579ae243..751ca71790 100644
> --- a/target/hexagon/translate.c
> +++ b/target/hexagon/translate.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright(c) 2019-2023 Qualcomm Innovation Center, Inc. All Rights
> Reserved.
> + * Copyright(c) 2019-2024 Qualcomm Innovation Center, Inc. All Rights
> Reserved.
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -394,20 +394,8 @@ static bool need_commit(DisasContext *ctx)
> }
> }
>
> - /* Check for overlap between register reads and writes */
> - for (int i = 0; i < ctx->reg_log_idx; i++) {
> - int rnum = ctx->reg_log[i];
> - if (test_bit(rnum, ctx->regs_read)) {
> - return true;
> - }
> - }
> -
> - /* Check for overlap between predicate reads and writes */
> - for (int i = 0; i < ctx->preg_log_idx; i++) {
> - int pnum = ctx->preg_log[i];
> - if (test_bit(pnum, ctx->pregs_read)) {
> - return true;
> - }
> + if (ctx->read_after_write) {
> + return true;
> }
>
> /* Check for overlap between HVX reads and writes */
> @@ -466,6 +454,7 @@ static void analyze_packet(DisasContext *ctx)
> {
> Packet *pkt = ctx->pkt;
> ctx->has_hvx_helper = false;
> + ctx->read_after_write = false;
> for (int i = 0; i < pkt->num_insns; i++) {
> Insn *insn = &pkt->insn[i];
> ctx->insn = insn;
> @@ -490,11 +479,9 @@ static void gen_start_packet(DisasContext *ctx)
> ctx->next_PC = next_PC;
> ctx->reg_log_idx = 0;
> bitmap_zero(ctx->regs_written, TOTAL_PER_THREAD_REGS);
> - bitmap_zero(ctx->regs_read, TOTAL_PER_THREAD_REGS);
> bitmap_zero(ctx->predicated_regs, TOTAL_PER_THREAD_REGS);
> ctx->preg_log_idx = 0;
> bitmap_zero(ctx->pregs_written, NUM_PREGS);
> - bitmap_zero(ctx->pregs_read, NUM_PREGS);
> ctx->future_vregs_idx = 0;
> ctx->tmp_vregs_idx = 0;
> ctx->vreg_log_idx = 0;
> --
> 2.34.1
Reviewed-by: Brian Cain <bc...@quicinc.com>
