diff mbox series

[v2,11/39] tcg: Pass the locked filepointer to tcg_dump_ops

Message ID 20220326132534.543738-20-richard.henderson@linaro.org
State Superseded
Headers show
Series Logging cleanup and per-thread logfiles | expand

Commit Message

Richard Henderson March 26, 2022, 1:25 p.m. UTC 
We have already looked up and locked the filepointer.
Use fprintf instead of qemu_log directly for output
in and around tcg_dump_ops.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tcg.c | 109 ++++++++++++++++++++++++++----------------------------
 1 file changed, 52 insertions(+), 57 deletions(-)

Comments

Alex Bennée April 13, 2022, 9:08 a.m. UTC | #1 
Richard Henderson <richard.henderson@linaro.org> writes:

> We have already looked up and locked the filepointer.
> Use fprintf instead of qemu_log directly for output
> in and around tcg_dump_ops.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/tcg.c | 109 ++++++++++++++++++++++++++----------------------------
>  1 file changed, 52 insertions(+), 57 deletions(-)
>
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 892f640fce..25e987d881 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -1808,7 +1808,11 @@ static inline TCGReg tcg_regset_first(TCGRegSet d)
>      }
>  }
>  
> -static void tcg_dump_ops(TCGContext *s, bool have_prefs)
> +/* Return only the number of characters output -- no error return. */
> +#define ne_fprintf(...) \
> +    ({ int ret_ = fprintf(__VA_ARGS__); ret_ >= 0 ? ret_ : 0; })
> +
> +static void tcg_dump_ops(TCGContext *s, FILE *f, bool have_prefs)
>  {
>      char buf[128];
>      TCGOp *op;
> @@ -1824,7 +1828,7 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
>  
>          if (c == INDEX_op_insn_start) {
>              nb_oargs = 0;
> -            col += qemu_log("\n ----");
> +            col += ne_fprintf(f, "\n ----");
>  
>              for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {
>                  target_ulong a;
> @@ -1833,7 +1837,7 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
>  #else
>                  a = op->args[i];
>  #endif
> -                col += qemu_log(" " TARGET_FMT_lx, a);
> +                col += ne_fprintf(f, " " TARGET_FMT_lx, a);
>              }
>          } else if (c == INDEX_op_call) {
>              const TCGHelperInfo *info = tcg_call_info(op);
> @@ -1844,7 +1848,7 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
>              nb_iargs = TCGOP_CALLI(op);
>              nb_cargs = def->nb_cargs;
>  
> -            col += qemu_log(" %s ", def->name);
> +            col += ne_fprintf(f, " %s ", def->name);
>  
>              /*
>               * Print the function name from TCGHelperInfo, if available.
> @@ -1852,15 +1856,15 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
>               * but the actual function pointer comes from the plugin.
>               */
>              if (func == info->func) {
> -                col += qemu_log("%s", info->name);
> +                col += ne_fprintf(f, "%s", info->name);
>              } else {
> -                col += qemu_log("plugin(%p)", func);
> +                col += ne_fprintf(f, "plugin(%p)", func);
>              }
>  
> -            col += qemu_log(",$0x%x,$%d", info->flags, nb_oargs);
> +            col += ne_fprintf(f, ",$0x%x,$%d", info->flags, nb_oargs);
>              for (i = 0; i < nb_oargs; i++) {
> -                col += qemu_log(",%s", tcg_get_arg_str(s, buf, sizeof(buf),
> -                                                       op->args[i]));
> +                col += ne_fprintf(f, ",%s", tcg_get_arg_str(s, buf, sizeof(buf),
> +                                                            op->args[i]));
>              }
>              for (i = 0; i < nb_iargs; i++) {
>                  TCGArg arg = op->args[nb_oargs + i];
> @@ -1868,34 +1872,32 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
>                  if (arg != TCG_CALL_DUMMY_ARG) {
>                      t = tcg_get_arg_str(s, buf, sizeof(buf), arg);
>                  }
> -                col += qemu_log(",%s", t);
> +                col += ne_fprintf(f, ",%s", t);
>              }
>          } else {
> -            col += qemu_log(" %s ", def->name);
> +            col += ne_fprintf(f, " %s ", def->name);
>  
>              nb_oargs = def->nb_oargs;
>              nb_iargs = def->nb_iargs;
>              nb_cargs = def->nb_cargs;
>  
>              if (def->flags & TCG_OPF_VECTOR) {
> -                col += qemu_log("v%d,e%d,", 64 << TCGOP_VECL(op),
> -                                8 << TCGOP_VECE(op));
> +                col += ne_fprintf(f, "v%d,e%d,", 64 << TCGOP_VECL(op),
> +                                  8 << TCGOP_VECE(op));
>              }
>  
>              k = 0;
>              for (i = 0; i < nb_oargs; i++) {
> -                if (k != 0) {
> -                    col += qemu_log(",");
> -                }
> -                col += qemu_log("%s", tcg_get_arg_str(s, buf, sizeof(buf),
> -                                                      op->args[k++]));
> +                const char *sep =  k ? "," : "";
> +                col += ne_fprintf(f, "%s%s", sep,
> +                                  tcg_get_arg_str(s, buf, sizeof(buf),
> +                                                  op->args[k++]));
>              }
>              for (i = 0; i < nb_iargs; i++) {
> -                if (k != 0) {
> -                    col += qemu_log(",");
> -                }
> -                col += qemu_log("%s", tcg_get_arg_str(s, buf, sizeof(buf),
> -                                                      op->args[k++]));
> +                const char *sep =  k ? "," : "";
> +                col += ne_fprintf(f, "%s%s", sep,
> +                                  tcg_get_arg_str(s, buf, sizeof(buf),
> +                                                  op->args[k++]));
>              }
>              switch (c) {
>              case INDEX_op_brcond_i32:
> @@ -1910,9 +1912,9 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
>              case INDEX_op_cmpsel_vec:
>                  if (op->args[k] < ARRAY_SIZE(cond_name)
>                      && cond_name[op->args[k]]) {
> -                    col += qemu_log(",%s", cond_name[op->args[k++]]);
> +                    col += ne_fprintf(f, ",%s", cond_name[op->args[k++]]);
>                  } else {
> -                    col += qemu_log(",$0x%" TCG_PRIlx, op->args[k++]);
> +                    col += ne_fprintf(f, ",$0x%" TCG_PRIlx, op->args[k++]);
>                  }
>                  i = 1;
>                  break;
> @@ -1927,12 +1929,12 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
>                      unsigned ix = get_mmuidx(oi);
>  
>                      if (op & ~(MO_AMASK | MO_BSWAP | MO_SSIZE)) {
> -                        col += qemu_log(",$0x%x,%u", op, ix);
> +                        col += ne_fprintf(f, ",$0x%x,%u", op, ix);
>                      } else {
>                          const char *s_al, *s_op;
>                          s_al = alignment_name[(op & MO_AMASK) >> MO_ASHIFT];
>                          s_op = ldst_name[op & (MO_BSWAP | MO_SSIZE)];
> -                        col += qemu_log(",%s%s,%u", s_al, s_op, ix);
> +                        col += ne_fprintf(f, ",%s%s,%u", s_al, s_op, ix);
>                      }
>                      i = 1;
>                  }
> @@ -1950,9 +1952,9 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
>                          name = bswap_flag_name[flags];
>                      }
>                      if (name) {
> -                        col += qemu_log(",%s", name);
> +                        col += ne_fprintf(f, ",%s", name);
>                      } else {
> -                        col += qemu_log(",$0x%" TCG_PRIlx, flags);
> +                        col += ne_fprintf(f, ",$0x%" TCG_PRIlx, flags);
>                      }
>                      i = k = 1;
>                  }
> @@ -1967,49 +1969,42 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
>              case INDEX_op_brcond_i32:
>              case INDEX_op_brcond_i64:
>              case INDEX_op_brcond2_i32:
> -                col += qemu_log("%s$L%d", k ? "," : "",
> -                                arg_label(op->args[k])->id);
> +                col += ne_fprintf(f, "%s$L%d", k ? "," : "",
> +                                  arg_label(op->args[k])->id);
>                  i++, k++;
>                  break;
>              default:
>                  break;
>              }
>              for (; i < nb_cargs; i++, k++) {
> -                col += qemu_log("%s$0x%" TCG_PRIlx, k ? "," : "", op->args[k]);
> +                col += ne_fprintf(f, "%s$0x%" TCG_PRIlx, k ? "," : "",
> +                                  op->args[k]);
>              }
>          }
>  
>          if (have_prefs || op->life) {
> -
> -            QemuLogFile *logfile;
> -
> -            rcu_read_lock();
> -            logfile = qatomic_rcu_read(&qemu_logfile);
> -            if (logfile) {
> -                for (; col < 40; ++col) {
> -                    putc(' ', logfile->fd);
> -                }
> +            for (; col < 40; ++col) {
> +                putc(' ', f);
>              }
> -            rcu_read_unlock();
>          }
>  
>          if (op->life) {
>              unsigned life = op->life;
>  
>              if (life & (SYNC_ARG * 3)) {
> -                qemu_log("  sync:");
> +                ne_fprintf(f, "  sync:");
>                  for (i = 0; i < 2; ++i) {
>                      if (life & (SYNC_ARG << i)) {
> -                        qemu_log(" %d", i);
> +                        ne_fprintf(f, " %d", i);
>                      }
>                  }
>              }
>              life /= DEAD_ARG;
>              if (life) {
> -                qemu_log("  dead:");
> +                ne_fprintf(f, "  dead:");
>                  for (i = 0; life; ++i, life >>= 1) {
>                      if (life & 1) {
> -                        qemu_log(" %d", i);
> +                        ne_fprintf(f, " %d", i);
>                      }
>                  }
>              }
> @@ -2020,28 +2015,28 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
>                  TCGRegSet set = op->output_pref[i];
>  
>                  if (i == 0) {
> -                    qemu_log("  pref=");
> +                    ne_fprintf(f, "  pref=");
>                  } else {
> -                    qemu_log(",");
> +                    ne_fprintf(f, ",");
>                  }
>                  if (set == 0) {
> -                    qemu_log("none");
> +                    ne_fprintf(f, "none");
>                  } else if (set == MAKE_64BIT_MASK(0, TCG_TARGET_NB_REGS)) {
> -                    qemu_log("all");
> +                    ne_fprintf(f, "all");
>  #ifdef CONFIG_DEBUG_TCG
>                  } else if (tcg_regset_single(set)) {
>                      TCGReg reg = tcg_regset_first(set);
> -                    qemu_log("%s", tcg_target_reg_names[reg]);
> +                    ne_fprintf(f, "%s", tcg_target_reg_names[reg]);
>  #endif
>                  } else if (TCG_TARGET_NB_REGS <= 32) {
> -                    qemu_log("%#x", (uint32_t)set);
> +                    ne_fprintf(f, "%#x", (uint32_t)set);
>                  } else {
> -                    qemu_log("%#" PRIx64, (uint64_t)set);
> +                    ne_fprintf(f, "%#" PRIx64, (uint64_t)set);

checkpatch saw these lines change and complains:

  ERROR: Don't use '#' flag of printf format ('%#') in format strings, use '0x' prefix instead

probably worth fixing up as we go.

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
diff mbox series

Patch

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 892f640fce..25e987d881 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1808,7 +1808,11 @@  static inline TCGReg tcg_regset_first(TCGRegSet d)
     }
 }
 
-static void tcg_dump_ops(TCGContext *s, bool have_prefs)
+/* Return only the number of characters output -- no error return. */
+#define ne_fprintf(...) \
+    ({ int ret_ = fprintf(__VA_ARGS__); ret_ >= 0 ? ret_ : 0; })
+
+static void tcg_dump_ops(TCGContext *s, FILE *f, bool have_prefs)
 {
     char buf[128];
     TCGOp *op;
@@ -1824,7 +1828,7 @@  static void tcg_dump_ops(TCGContext *s, bool have_prefs)
 
         if (c == INDEX_op_insn_start) {
             nb_oargs = 0;
-            col += qemu_log("\n ----");
+            col += ne_fprintf(f, "\n ----");
 
             for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {
                 target_ulong a;
@@ -1833,7 +1837,7 @@  static void tcg_dump_ops(TCGContext *s, bool have_prefs)
 #else
                 a = op->args[i];
 #endif
-                col += qemu_log(" " TARGET_FMT_lx, a);
+                col += ne_fprintf(f, " " TARGET_FMT_lx, a);
             }
         } else if (c == INDEX_op_call) {
             const TCGHelperInfo *info = tcg_call_info(op);
@@ -1844,7 +1848,7 @@  static void tcg_dump_ops(TCGContext *s, bool have_prefs)
             nb_iargs = TCGOP_CALLI(op);
             nb_cargs = def->nb_cargs;
 
-            col += qemu_log(" %s ", def->name);
+            col += ne_fprintf(f, " %s ", def->name);
 
             /*
              * Print the function name from TCGHelperInfo, if available.
@@ -1852,15 +1856,15 @@  static void tcg_dump_ops(TCGContext *s, bool have_prefs)
              * but the actual function pointer comes from the plugin.
              */
             if (func == info->func) {
-                col += qemu_log("%s", info->name);
+                col += ne_fprintf(f, "%s", info->name);
             } else {
-                col += qemu_log("plugin(%p)", func);
+                col += ne_fprintf(f, "plugin(%p)", func);
             }
 
-            col += qemu_log(",$0x%x,$%d", info->flags, nb_oargs);
+            col += ne_fprintf(f, ",$0x%x,$%d", info->flags, nb_oargs);
             for (i = 0; i < nb_oargs; i++) {
-                col += qemu_log(",%s", tcg_get_arg_str(s, buf, sizeof(buf),
-                                                       op->args[i]));
+                col += ne_fprintf(f, ",%s", tcg_get_arg_str(s, buf, sizeof(buf),
+                                                            op->args[i]));
             }
             for (i = 0; i < nb_iargs; i++) {
                 TCGArg arg = op->args[nb_oargs + i];
@@ -1868,34 +1872,32 @@  static void tcg_dump_ops(TCGContext *s, bool have_prefs)
                 if (arg != TCG_CALL_DUMMY_ARG) {
                     t = tcg_get_arg_str(s, buf, sizeof(buf), arg);
                 }
-                col += qemu_log(",%s", t);
+                col += ne_fprintf(f, ",%s", t);
             }
         } else {
-            col += qemu_log(" %s ", def->name);
+            col += ne_fprintf(f, " %s ", def->name);
 
             nb_oargs = def->nb_oargs;
             nb_iargs = def->nb_iargs;
             nb_cargs = def->nb_cargs;
 
             if (def->flags & TCG_OPF_VECTOR) {
-                col += qemu_log("v%d,e%d,", 64 << TCGOP_VECL(op),
-                                8 << TCGOP_VECE(op));
+                col += ne_fprintf(f, "v%d,e%d,", 64 << TCGOP_VECL(op),
+                                  8 << TCGOP_VECE(op));
             }
 
             k = 0;
             for (i = 0; i < nb_oargs; i++) {
-                if (k != 0) {
-                    col += qemu_log(",");
-                }
-                col += qemu_log("%s", tcg_get_arg_str(s, buf, sizeof(buf),
-                                                      op->args[k++]));
+                const char *sep =  k ? "," : "";
+                col += ne_fprintf(f, "%s%s", sep,
+                                  tcg_get_arg_str(s, buf, sizeof(buf),
+                                                  op->args[k++]));
             }
             for (i = 0; i < nb_iargs; i++) {
-                if (k != 0) {
-                    col += qemu_log(",");
-                }
-                col += qemu_log("%s", tcg_get_arg_str(s, buf, sizeof(buf),
-                                                      op->args[k++]));
+                const char *sep =  k ? "," : "";
+                col += ne_fprintf(f, "%s%s", sep,
+                                  tcg_get_arg_str(s, buf, sizeof(buf),
+                                                  op->args[k++]));
             }
             switch (c) {
             case INDEX_op_brcond_i32:
@@ -1910,9 +1912,9 @@  static void tcg_dump_ops(TCGContext *s, bool have_prefs)
             case INDEX_op_cmpsel_vec:
                 if (op->args[k] < ARRAY_SIZE(cond_name)
                     && cond_name[op->args[k]]) {
-                    col += qemu_log(",%s", cond_name[op->args[k++]]);
+                    col += ne_fprintf(f, ",%s", cond_name[op->args[k++]]);
                 } else {
-                    col += qemu_log(",$0x%" TCG_PRIlx, op->args[k++]);
+                    col += ne_fprintf(f, ",$0x%" TCG_PRIlx, op->args[k++]);
                 }
                 i = 1;
                 break;
@@ -1927,12 +1929,12 @@  static void tcg_dump_ops(TCGContext *s, bool have_prefs)
                     unsigned ix = get_mmuidx(oi);
 
                     if (op & ~(MO_AMASK | MO_BSWAP | MO_SSIZE)) {
-                        col += qemu_log(",$0x%x,%u", op, ix);
+                        col += ne_fprintf(f, ",$0x%x,%u", op, ix);
                     } else {
                         const char *s_al, *s_op;
                         s_al = alignment_name[(op & MO_AMASK) >> MO_ASHIFT];
                         s_op = ldst_name[op & (MO_BSWAP | MO_SSIZE)];
-                        col += qemu_log(",%s%s,%u", s_al, s_op, ix);
+                        col += ne_fprintf(f, ",%s%s,%u", s_al, s_op, ix);
                     }
                     i = 1;
                 }
@@ -1950,9 +1952,9 @@  static void tcg_dump_ops(TCGContext *s, bool have_prefs)
                         name = bswap_flag_name[flags];
                     }
                     if (name) {
-                        col += qemu_log(",%s", name);
+                        col += ne_fprintf(f, ",%s", name);
                     } else {
-                        col += qemu_log(",$0x%" TCG_PRIlx, flags);
+                        col += ne_fprintf(f, ",$0x%" TCG_PRIlx, flags);
                     }
                     i = k = 1;
                 }
@@ -1967,49 +1969,42 @@  static void tcg_dump_ops(TCGContext *s, bool have_prefs)
             case INDEX_op_brcond_i32:
             case INDEX_op_brcond_i64:
             case INDEX_op_brcond2_i32:
-                col += qemu_log("%s$L%d", k ? "," : "",
-                                arg_label(op->args[k])->id);
+                col += ne_fprintf(f, "%s$L%d", k ? "," : "",
+                                  arg_label(op->args[k])->id);
                 i++, k++;
                 break;
             default:
                 break;
             }
             for (; i < nb_cargs; i++, k++) {
-                col += qemu_log("%s$0x%" TCG_PRIlx, k ? "," : "", op->args[k]);
+                col += ne_fprintf(f, "%s$0x%" TCG_PRIlx, k ? "," : "",
+                                  op->args[k]);
             }
         }
 
         if (have_prefs || op->life) {
-
-            QemuLogFile *logfile;
-
-            rcu_read_lock();
-            logfile = qatomic_rcu_read(&qemu_logfile);
-            if (logfile) {
-                for (; col < 40; ++col) {
-                    putc(' ', logfile->fd);
-                }
+            for (; col < 40; ++col) {
+                putc(' ', f);
             }
-            rcu_read_unlock();
         }
 
         if (op->life) {
             unsigned life = op->life;
 
             if (life & (SYNC_ARG * 3)) {
-                qemu_log("  sync:");
+                ne_fprintf(f, "  sync:");
                 for (i = 0; i < 2; ++i) {
                     if (life & (SYNC_ARG << i)) {
-                        qemu_log(" %d", i);
+                        ne_fprintf(f, " %d", i);
                     }
                 }
             }
             life /= DEAD_ARG;
             if (life) {
-                qemu_log("  dead:");
+                ne_fprintf(f, "  dead:");
                 for (i = 0; life; ++i, life >>= 1) {
                     if (life & 1) {
-                        qemu_log(" %d", i);
+                        ne_fprintf(f, " %d", i);
                     }
                 }
             }
@@ -2020,28 +2015,28 @@  static void tcg_dump_ops(TCGContext *s, bool have_prefs)
                 TCGRegSet set = op->output_pref[i];
 
                 if (i == 0) {
-                    qemu_log("  pref=");
+                    ne_fprintf(f, "  pref=");
                 } else {
-                    qemu_log(",");
+                    ne_fprintf(f, ",");
                 }
                 if (set == 0) {
-                    qemu_log("none");
+                    ne_fprintf(f, "none");
                 } else if (set == MAKE_64BIT_MASK(0, TCG_TARGET_NB_REGS)) {
-                    qemu_log("all");
+                    ne_fprintf(f, "all");
 #ifdef CONFIG_DEBUG_TCG
                 } else if (tcg_regset_single(set)) {
                     TCGReg reg = tcg_regset_first(set);
-                    qemu_log("%s", tcg_target_reg_names[reg]);
+                    ne_fprintf(f, "%s", tcg_target_reg_names[reg]);
 #endif
                 } else if (TCG_TARGET_NB_REGS <= 32) {
-                    qemu_log("%#x", (uint32_t)set);
+                    ne_fprintf(f, "%#x", (uint32_t)set);
                 } else {
-                    qemu_log("%#" PRIx64, (uint64_t)set);
+                    ne_fprintf(f, "%#" PRIx64, (uint64_t)set);
                 }
             }
         }
 
-        qemu_log("\n");
+        putc('\n', f);
     }
 }
 
@@ -4207,7 +4202,7 @@  int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
         FILE *logfile = qemu_log_trylock();
         if (logfile) {
             fprintf(logfile, "OP:\n");
-            tcg_dump_ops(s, false);
+            tcg_dump_ops(s, logfile, false);
             fprintf(logfile, "\n");
             qemu_log_unlock(logfile);
         }
@@ -4254,7 +4249,7 @@  int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
             FILE *logfile = qemu_log_trylock();
             if (logfile) {
                 fprintf(logfile, "OP before indirect lowering:\n");
-                tcg_dump_ops(s, false);
+                tcg_dump_ops(s, logfile, false);
                 fprintf(logfile, "\n");
                 qemu_log_unlock(logfile);
             }
@@ -4277,7 +4272,7 @@  int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
         FILE *logfile = qemu_log_trylock();
         if (logfile) {
             fprintf(logfile, "OP after optimization and liveness analysis:\n");
-            tcg_dump_ops(s, true);
+            tcg_dump_ops(s, logfile, true);
             fprintf(logfile, "\n");
             qemu_log_unlock(logfile);
         }