From: Aurelien Jarno <aurelien@aurel32.net>
To: Richard Henderson <rth@twiddle.net>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 3/7] target-mips: Always evaluate debugging macro arguments
Date: Tue, 18 Sep 2012 18:38:51 +0200 [thread overview]
Message-ID: <20120918163851.GA22866@ohm.aurel32.net> (raw)
In-Reply-To: <1347917713-23343-4-git-send-email-rth@twiddle.net>
On Mon, Sep 17, 2012 at 02:35:09PM -0700, Richard Henderson wrote:
> Done inside an if (0) so that it is easily eliminated as dead code.
> But this will prevent some of the compilation errors with debugging
> enabled from creeping back in.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> target-mips/translate.c | 48 +++++++++++-------------------------------------
> 1 file changed, 11 insertions(+), 37 deletions(-)
>
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index f93b444..775c3a1 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -567,14 +567,18 @@ static const char *fregnames[] =
> "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31", };
>
> #ifdef MIPS_DEBUG_DISAS
> -#define MIPS_DEBUG(fmt, ...) \
> - qemu_log_mask(CPU_LOG_TB_IN_ASM, \
> - TARGET_FMT_lx ": %08x " fmt "\n", \
> - ctx->pc, ctx->opcode , ## __VA_ARGS__)
> +#define MIPS_DEBUG(fmt, ...) \
> + qemu_log_mask(CPU_LOG_TB_IN_ASM, \
> + TARGET_FMT_lx ": %08x " fmt "\n", \
> + ctx->pc, ctx->opcode , ## __VA_ARGS__)
> #define LOG_DISAS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
> #else
> -#define MIPS_DEBUG(fmt, ...) do { } while(0)
> -#define LOG_DISAS(...) do { } while (0)
> +#define MIPS_DEBUG(fmt, ...) \
> + do { if (0) { \
> + qemu_log_mask(0, "%x" fmt, ctx->opcode, ## __VA_ARGS__); \
> + } } while(0)
> +#define LOG_DISAS(...) \
> + do { if (0) { qemu_log_mask(0, ## __VA_ARGS__); } } while (0)
> #endif
Instead of having almost twice the same code, couldn't we use something
like "if (MIPS_DEBUG_DISAS)" instead of "if (0)". Of course it means
MIPS_DEBUG_DISAS has to be defined to 0/1 instead of defined/not
defined, but I don't think it's a problem.
> #define MIPS_INVAL(op) \
> @@ -1163,7 +1167,6 @@ static void gen_ld (CPUMIPSState *env, DisasContext *ctx, uint32_t opc,
> opn = "ll";
> break;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %d(%s)", opn, regnames[rt], offset, regnames[base]);
> tcg_temp_free(t0);
> tcg_temp_free(t1);
> @@ -1223,7 +1226,6 @@ static void gen_st (DisasContext *ctx, uint32_t opc, int rt,
> opn = "swr";
> break;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %d(%s)", opn, regnames[rt], offset, regnames[base]);
> tcg_temp_free(t0);
> tcg_temp_free(t1);
> @@ -1259,7 +1261,6 @@ static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt,
> opn = "sc";
> break;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %d(%s)", opn, regnames[rt], offset, regnames[base]);
> tcg_temp_free(t1);
> tcg_temp_free(t0);
> @@ -1325,7 +1326,6 @@ static void gen_flt_ldst (DisasContext *ctx, uint32_t opc, int ft,
> generate_exception(ctx, EXCP_RI);
> goto out;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %d(%s)", opn, fregnames[ft], offset, regnames[base]);
> out:
> tcg_temp_free(t0);
> @@ -1426,7 +1426,6 @@ static void gen_arith_imm (CPUMIPSState *env, DisasContext *ctx, uint32_t opc,
> break;
> #endif
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %s, " TARGET_FMT_lx, opn, regnames[rt], regnames[rs], uimm);
> }
>
> @@ -1470,7 +1469,6 @@ static void gen_logic_imm(CPUMIPSState *env, DisasContext *ctx, uint32_t opc,
> opn = "lui";
> break;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %s, " TARGET_FMT_lx, opn, regnames[rt], regnames[rs], uimm);
> }
>
> @@ -1499,7 +1497,6 @@ static void gen_slt_imm(CPUMIPSState *env, DisasContext *ctx, uint32_t opc,
> opn = "sltiu";
> break;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %s, " TARGET_FMT_lx, opn, regnames[rt], regnames[rs], uimm);
> tcg_temp_free(t0);
> }
> @@ -1591,7 +1588,6 @@ static void gen_shift_imm(CPUMIPSState *env, DisasContext *ctx, uint32_t opc,
> break;
> #endif
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %s, " TARGET_FMT_lx, opn, regnames[rt], regnames[rs], uimm);
> tcg_temp_free(t0);
> }
> @@ -1772,7 +1768,6 @@ static void gen_arith (CPUMIPSState *env, DisasContext *ctx, uint32_t opc,
> opn = "mul";
> break;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %s, %s", opn, regnames[rd], regnames[rs], regnames[rt]);
> }
>
> @@ -1811,7 +1806,6 @@ static void gen_cond_move(CPUMIPSState *env, DisasContext *ctx, uint32_t opc,
> tcg_gen_movi_tl(cpu_gpr[rd], 0);
> gen_set_label(l1);
>
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %s, %s", opn, regnames[rd], regnames[rs], regnames[rt]);
> }
>
> @@ -1873,7 +1867,6 @@ static void gen_logic(CPUMIPSState *env, DisasContext *ctx, uint32_t opc,
> opn = "xor";
> break;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %s, %s", opn, regnames[rd], regnames[rs], regnames[rt]);
> }
>
> @@ -1904,7 +1897,6 @@ static void gen_slt(CPUMIPSState *env, DisasContext *ctx, uint32_t opc,
> opn = "sltu";
> break;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %s, %s", opn, regnames[rd], regnames[rs], regnames[rt]);
> tcg_temp_free(t0);
> tcg_temp_free(t1);
> @@ -1985,7 +1977,6 @@ static void gen_shift (CPUMIPSState *env, DisasContext *ctx, uint32_t opc,
> break;
> #endif
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %s, %s", opn, regnames[rd], regnames[rs], regnames[rt]);
> tcg_temp_free(t0);
> tcg_temp_free(t1);
> @@ -2025,7 +2016,6 @@ static void gen_HILO (DisasContext *ctx, uint32_t opc, int reg)
> opn = "mtlo";
> break;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s", opn, regnames[reg]);
> }
>
> @@ -2258,7 +2248,6 @@ static void gen_muldiv (DisasContext *ctx, uint32_t opc,
> generate_exception(ctx, EXCP_RI);
> goto out;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s %s", opn, regnames[rs], regnames[rt]);
> out:
> tcg_temp_free(t0);
> @@ -2338,7 +2327,6 @@ static void gen_mul_vr54xx (DisasContext *ctx, uint32_t opc,
> goto out;
> }
> gen_store_gpr(t0, rd);
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %s, %s", opn, regnames[rd], regnames[rs], regnames[rt]);
>
> out:
> @@ -2379,7 +2367,6 @@ static void gen_cl (DisasContext *ctx, uint32_t opc,
> break;
> #endif
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %s", opn, regnames[rd], regnames[rs]);
> tcg_temp_free(t0);
> }
> @@ -2593,8 +2580,7 @@ static void gen_loongson_integer (DisasContext *ctx, uint32_t opc,
> #endif
> }
>
> - (void)opn; /* avoid a compiler warning */
> - MIPS_DEBUG("%s %s, %s", opn, regnames[rd], regnames[rs]);
> + MIPS_DEBUG("%s %s, %s, %s", opn, regnames[rd], regnames[rs], regnames[rt]);
> tcg_temp_free(t0);
> tcg_temp_free(t1);
> }
> @@ -3765,7 +3751,6 @@ static void gen_mfc0 (CPUMIPSState *env, DisasContext *ctx, TCGv arg, int reg, i
> default:
> goto die;
> }
> - (void)rn; /* avoid a compiler warning */
> LOG_DISAS("mfc0 %s (reg %d sel %d)\n", rn, reg, sel);
> return;
>
> @@ -4356,7 +4341,6 @@ static void gen_mtc0 (CPUMIPSState *env, DisasContext *ctx, TCGv arg, int reg, i
> default:
> goto die;
> }
> - (void)rn; /* avoid a compiler warning */
> LOG_DISAS("mtc0 %s (reg %d sel %d)\n", rn, reg, sel);
> /* For simplicity assume that all writes can cause interrupts. */
> if (use_icount) {
> @@ -4931,7 +4915,6 @@ static void gen_dmfc0 (CPUMIPSState *env, DisasContext *ctx, TCGv arg, int reg,
> default:
> goto die;
> }
> - (void)rn; /* avoid a compiler warning */
> LOG_DISAS("dmfc0 %s (reg %d sel %d)\n", rn, reg, sel);
> return;
>
> @@ -5523,7 +5506,6 @@ static void gen_dmtc0 (CPUMIPSState *env, DisasContext *ctx, TCGv arg, int reg,
> default:
> goto die;
> }
> - (void)rn; /* avoid a compiler warning */
> LOG_DISAS("dmtc0 %s (reg %d sel %d)\n", rn, reg, sel);
> /* For simplicity assume that all writes can cause interrupts. */
> if (use_icount) {
> @@ -6071,7 +6053,6 @@ static void gen_cp0 (CPUMIPSState *env, DisasContext *ctx, uint32_t opc, int rt,
> generate_exception(ctx, EXCP_RI);
> return;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s %d", opn, regnames[rt], rd);
> }
> #endif /* !CONFIG_USER_ONLY */
> @@ -6181,7 +6162,6 @@ static void gen_compute_branch1 (CPUMIPSState *env, DisasContext *ctx, uint32_t
> generate_exception (ctx, EXCP_RI);
> goto out;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s: cond %02x target " TARGET_FMT_lx, opn,
> ctx->hflags, btarget);
> ctx->btarget = btarget;
> @@ -6411,7 +6391,6 @@ static void gen_cp1 (DisasContext *ctx, uint32_t opc, int rt, int fs)
> generate_exception (ctx, EXCP_RI);
> goto out;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s %s", opn, regnames[rt], fregnames[fs]);
>
> out:
> @@ -7739,7 +7718,6 @@ static void gen_farith (DisasContext *ctx, enum fopcode op1,
> generate_exception (ctx, EXCP_RI);
> return;
> }
> - (void)opn; /* avoid a compiler warning */
> switch (optype) {
> case BINOP:
> MIPS_DEBUG("%s %s, %s, %s", opn, fregnames[fd], fregnames[fs], fregnames[ft]);
> @@ -7851,7 +7829,6 @@ static void gen_flt3_ldst (DisasContext *ctx, uint32_t opc,
> break;
> }
> tcg_temp_free(t0);
> - (void)opn; (void)store; /* avoid compiler warnings */
> MIPS_DEBUG("%s %s, %s(%s)", opn, fregnames[store ? fs : fd],
> regnames[index], regnames[base]);
> }
> @@ -8125,7 +8102,6 @@ static void gen_flt3_arith (DisasContext *ctx, uint32_t opc,
> generate_exception (ctx, EXCP_RI);
> return;
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s %s, %s, %s, %s", opn, fregnames[fd], fregnames[fr],
> fregnames[fs], fregnames[ft]);
> }
> @@ -9894,7 +9870,6 @@ static void gen_ldst_multiple (DisasContext *ctx, uint32_t opc, int reglist,
> break;
> #endif
> }
> - (void)opn;
> MIPS_DEBUG("%s, %x, %d(%s)", opn, reglist, offset, regnames[base]);
> tcg_temp_free(t0);
> tcg_temp_free(t1);
> @@ -10119,7 +10094,6 @@ static void gen_ldst_pair (DisasContext *ctx, uint32_t opc, int rd,
> break;
> #endif
> }
> - (void)opn; /* avoid a compiler warning */
> MIPS_DEBUG("%s, %s, %d(%s)", opn, regnames[rd], offset, regnames[base]);
> tcg_temp_free(t0);
> tcg_temp_free(t1);
> --
> 1.7.11.4
>
The remaining looks fine and is a nice cleanup.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
next prev parent reply other threads:[~2012-09-18 16:39 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-17 21:35 [Qemu-devel] [PATCH v2 0/7] target-mips improvements Richard Henderson
2012-09-17 21:35 ` [Qemu-devel] [PATCH 1/7] target-mips: Set opn in gen_ldst_multiple Richard Henderson
2012-09-18 16:38 ` Aurelien Jarno
2012-09-17 21:35 ` [Qemu-devel] [PATCH 2/7] target-mips: Fix MIPS_DEBUG Richard Henderson
2012-09-18 16:38 ` Aurelien Jarno
2012-09-17 21:35 ` [Qemu-devel] [PATCH 3/7] target-mips: Always evaluate debugging macro arguments Richard Henderson
2012-09-18 16:38 ` Aurelien Jarno [this message]
2012-09-17 21:35 ` [Qemu-devel] [PATCH 4/7] target-mips: Pass DisasContext to fpr32 load/store routines Richard Henderson
2012-09-18 16:39 ` Aurelien Jarno
2012-09-17 21:35 ` [Qemu-devel] [PATCH 5/7] target-mips: Use TCG registers for the FPU Richard Henderson
2012-09-18 16:39 ` Aurelien Jarno
2012-09-17 21:35 ` [Qemu-devel] [PATCH 6/7] target-mips: Add accessors for the two 32-bit halves of a 64-bit FPR Richard Henderson
2012-09-17 21:35 ` [Qemu-devel] [PATCH 7/7] target-mips: Implement Loongson Multimedia Instructions Richard Henderson
2012-09-18 16:39 ` Aurelien Jarno
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120918163851.GA22866@ohm.aurel32.net \
--to=aurelien@aurel32.net \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).