qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [PATCH v2 2/3] Hexagon (target/hexagon) Enable more short-circuit pa


From: Brian Cain
Subject: RE: [PATCH v2 2/3] Hexagon (target/hexagon) Enable more short-circuit packets (scalar core)
Date: Fri, 29 Mar 2024 02:02:43 +0000


> -----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 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 <ltaylorsimpson@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 <bcain@quicinc.com>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]