diff mbox series

[1/2] tcg: Link branches to the labels

Message ID 20230303223056.966286-2-richard.henderson@linaro.org
State Superseded
Headers show
Series tcg: Merge two sequential labels | expand

Commit Message

Richard Henderson March 3, 2023, 10:30 p.m. UTC
This allows us to easily find all branches that use a label.
Since 'refs' is only tested vs zero, remove it and test for
an empty list instead.  Drop the use of bitfields, which had
been used to pack refs into a single 32-bit word.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/tcg/tcg-op.h |  7 +------
 include/tcg/tcg.h    | 19 +++++++++++++------
 tcg/tcg-op.c         | 22 +++++++++++++++++++---
 tcg/tcg.c            | 30 ++++++++++++++++++++----------
 4 files changed, 53 insertions(+), 25 deletions(-)

Comments

Taylor Simpson March 3, 2023, 11:26 p.m. UTC | #1
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Friday, March 3, 2023 3:31 PM
> To: qemu-devel@nongnu.org
> Cc: anjo@rev.ng; Taylor Simpson <tsimpson@quicinc.com>
> Subject: [PATCH 1/2] tcg: Link branches to the labels
> 
> This allows us to easily find all branches that use a label.
> Since 'refs' is only tested vs zero, remove it and test for an empty list instead.
> Drop the use of bitfields, which had been used to pack refs into a single 32-
> bit word.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/tcg/tcg-op.h |  7 +------
>  include/tcg/tcg.h    | 19 +++++++++++++------
>  tcg/tcg-op.c         | 22 +++++++++++++++++++---
>  tcg/tcg.c            | 30 ++++++++++++++++++++----------
>  4 files changed, 53 insertions(+), 25 deletions(-)
> 
> diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c index f2269a1b91..befc7c000d 100644
> --- a/tcg/tcg-op.c
> +++ b/tcg/tcg-op.c
> @@ -84,6 +84,22 @@ void tcg_gen_op6(TCGOpcode opc, TCGArg a1, TCGArg
> a2, TCGArg a3,
>      op->args[5] = a6;
>  }
> 
> +/* Generic ops.  */
> +
> +static void last_is_label_use(TCGLabel *l) {

It would be more clear to name this function "add_label_use".

Otherwise,
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
Richard Henderson March 4, 2023, 6:31 p.m. UTC | #2
On 3/3/23 15:26, Taylor Simpson wrote:
>> +/* Generic ops.  */
>> +
>> +static void last_is_label_use(TCGLabel *l) {
> 
> It would be more clear to name this function "add_label_use".

I've used "add_last_as_label_use" to emphasize where the use is coming from.
Thanks.

r~
diff mbox series

Patch

diff --git a/include/tcg/tcg-op.h b/include/tcg/tcg-op.h
index 353d430a63..70856147c5 100644
--- a/include/tcg/tcg-op.h
+++ b/include/tcg/tcg-op.h
@@ -259,12 +259,7 @@  static inline void gen_set_label(TCGLabel *l)
     tcg_gen_op1(INDEX_op_set_label, label_arg(l));
 }
 
-static inline void tcg_gen_br(TCGLabel *l)
-{
-    l->refs++;
-    tcg_gen_op1(INDEX_op_br, label_arg(l));
-}
-
+void tcg_gen_br(TCGLabel *l);
 void tcg_gen_mb(TCGBar);
 
 /* Helper calls. */
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 7e2b954dbc..0dc88011ce 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -238,16 +238,23 @@  struct TCGRelocation {
     int type;
 };
 
+typedef struct TCGOp TCGOp;
+typedef struct TCGLabelUse TCGLabelUse;
+struct TCGLabelUse {
+    QSIMPLEQ_ENTRY(TCGLabelUse) next;
+    TCGOp *op;
+};
+
 typedef struct TCGLabel TCGLabel;
 struct TCGLabel {
-    unsigned present : 1;
-    unsigned has_value : 1;
-    unsigned id : 14;
-    unsigned refs : 16;
+    bool present;
+    bool has_value;
+    uint16_t id;
     union {
         uintptr_t value;
         const tcg_insn_unit *value_ptr;
     } u;
+    QSIMPLEQ_HEAD(, TCGLabelUse) branches;
     QSIMPLEQ_HEAD(, TCGRelocation) relocs;
     QSIMPLEQ_ENTRY(TCGLabel) next;
 };
@@ -487,7 +494,7 @@  typedef struct TCGTempSet {
 #define SYNC_ARG  (1 << 0)
 typedef uint32_t TCGLifeData;
 
-typedef struct TCGOp {
+struct TCGOp {
     TCGOpcode opc   : 8;
     unsigned nargs  : 8;
 
@@ -506,7 +513,7 @@  typedef struct TCGOp {
 
     /* Arguments for the opcode.  */
     TCGArg args[];
-} TCGOp;
+};
 
 #define TCGOP_CALLI(X)    (X)->param1
 #define TCGOP_CALLO(X)    (X)->param2
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index f2269a1b91..befc7c000d 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -84,6 +84,22 @@  void tcg_gen_op6(TCGOpcode opc, TCGArg a1, TCGArg a2, TCGArg a3,
     op->args[5] = a6;
 }
 
+/* Generic ops.  */
+
+static void last_is_label_use(TCGLabel *l)
+{
+    TCGLabelUse *u = tcg_malloc(sizeof(TCGLabelUse));
+
+    u->op = tcg_last_op();
+    QSIMPLEQ_INSERT_TAIL(&l->branches, u, next);
+}
+
+void tcg_gen_br(TCGLabel *l)
+{
+    tcg_gen_op1(INDEX_op_br, label_arg(l));
+    last_is_label_use(l);
+}
+
 void tcg_gen_mb(TCGBar mb_type)
 {
     if (tcg_ctx->gen_tb->cflags & CF_PARALLEL) {
@@ -216,8 +232,8 @@  void tcg_gen_brcond_i32(TCGCond cond, TCGv_i32 arg1, TCGv_i32 arg2, TCGLabel *l)
     if (cond == TCG_COND_ALWAYS) {
         tcg_gen_br(l);
     } else if (cond != TCG_COND_NEVER) {
-        l->refs++;
         tcg_gen_op4ii_i32(INDEX_op_brcond_i32, arg1, arg2, cond, label_arg(l));
+        last_is_label_use(l);
     }
 }
 
@@ -1474,7 +1490,6 @@  void tcg_gen_brcond_i64(TCGCond cond, TCGv_i64 arg1, TCGv_i64 arg2, TCGLabel *l)
     if (cond == TCG_COND_ALWAYS) {
         tcg_gen_br(l);
     } else if (cond != TCG_COND_NEVER) {
-        l->refs++;
         if (TCG_TARGET_REG_BITS == 32) {
             tcg_gen_op6ii_i32(INDEX_op_brcond2_i32, TCGV_LOW(arg1),
                               TCGV_HIGH(arg1), TCGV_LOW(arg2),
@@ -1483,6 +1498,7 @@  void tcg_gen_brcond_i64(TCGCond cond, TCGv_i64 arg1, TCGv_i64 arg2, TCGLabel *l)
             tcg_gen_op4ii_i64(INDEX_op_brcond_i64, arg1, arg2, cond,
                               label_arg(l));
         }
+        last_is_label_use(l);
     }
 }
 
@@ -1493,12 +1509,12 @@  void tcg_gen_brcondi_i64(TCGCond cond, TCGv_i64 arg1, int64_t arg2, TCGLabel *l)
     } else if (cond == TCG_COND_ALWAYS) {
         tcg_gen_br(l);
     } else if (cond != TCG_COND_NEVER) {
-        l->refs++;
         tcg_gen_op6ii_i32(INDEX_op_brcond2_i32,
                           TCGV_LOW(arg1), TCGV_HIGH(arg1),
                           tcg_constant_i32(arg2),
                           tcg_constant_i32(arg2 >> 32),
                           cond, label_arg(l));
+        last_is_label_use(l);
     }
 }
 
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 5cccc06ae3..a8ec11dfb9 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -282,6 +282,7 @@  TCGLabel *gen_new_label(void)
 
     memset(l, 0, sizeof(TCGLabel));
     l->id = s->nb_labels++;
+    QSIMPLEQ_INIT(&l->branches);
     QSIMPLEQ_INIT(&l->relocs);
 
     QSIMPLEQ_INSERT_TAIL(&s->labels, l, next);
@@ -2519,23 +2520,32 @@  static void process_op_defs(TCGContext *s)
     }
 }
 
+static void remove_label_use(TCGOp *op, int idx)
+{
+    TCGLabel *label = arg_label(op->args[idx]);
+    TCGLabelUse *use;
+
+    QSIMPLEQ_FOREACH(use, &label->branches, next) {
+        if (use->op == op) {
+            QSIMPLEQ_REMOVE(&label->branches, use, TCGLabelUse, next);
+            return;
+        }
+    }
+    g_assert_not_reached();
+}
+
 void tcg_op_remove(TCGContext *s, TCGOp *op)
 {
-    TCGLabel *label;
-
     switch (op->opc) {
     case INDEX_op_br:
-        label = arg_label(op->args[0]);
-        label->refs--;
+        remove_label_use(op, 0);
         break;
     case INDEX_op_brcond_i32:
     case INDEX_op_brcond_i64:
-        label = arg_label(op->args[3]);
-        label->refs--;
+        remove_label_use(op, 3);
         break;
     case INDEX_op_brcond2_i32:
-        label = arg_label(op->args[5]);
-        label->refs--;
+        remove_label_use(op, 5);
         break;
     default:
         break;
@@ -2647,7 +2657,7 @@  reachable_code_pass(TCGContext *s)
                 dead = false;
             }
 
-            if (label->refs == 0) {
+            if (QSIMPLEQ_EMPTY(&label->branches)) {
                 /*
                  * While there is an occasional backward branch, virtually
                  * all branches generated by the translators are forward.
@@ -4891,7 +4901,7 @@  int tcg_gen_code(TCGContext *s, TranslationBlock *tb, target_ulong pc_start)
         bool error = false;
 
         QSIMPLEQ_FOREACH(l, &s->labels, next) {
-            if (unlikely(!l->present) && l->refs) {
+            if (unlikely(!l->present) && !QSIMPLEQ_EMPTY(&l->branches)) {
                 qemu_log_mask(CPU_LOG_TB_OP,
                               "$L%d referenced but not present.\n", l->id);
                 error = true;