qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 3/7] target-mips: Always evaluate debugging macr


From: Aurelien Jarno
Subject: Re: [Qemu-devel] [PATCH 3/7] target-mips: Always evaluate debugging macro arguments
Date: Tue, 18 Sep 2012 18:38:51 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

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 <address@hidden>
> ---
>  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
address@hidden                 http://www.aurel32.net



reply via email to

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