[RFC 12/19] target-i386: Refactor debug output macros

[RFC 12/19] target-i386: Refactor debug output macros

[Andreas Färber]

Make debug output compile-testable even if disabled.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
---
 target-i386/helper.c     |   33 ++++++++++++++++-----------------
 target-i386/kvm.c        |   16 +++++++---------
 target-i386/seg_helper.c |   20 +++++++++++---------
 3 Dateien geändert, 34 Zeilen hinzugefügt(+), 35 Zeilen entfernt(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 547c25e..aa94289 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -24,7 +24,12 @@
 #include "monitor/monitor.h"
 #endif
 
-//#define DEBUG_MMU
+#define DEBUG_MMU 0
+#define MMU_DPRINTF(...) G_STMT_START \
+    if (DEBUG_MMU) { \
+        printf(__VA_ARGS__); \
+    } \
+    G_STMT_END
 
 static void cpu_x86_version(CPUX86State *env, int *family, int *model)
 {
@@ -370,9 +375,8 @@ void cpu_x86_set_a20(CPUX86State *env, int a20_state)
 {
     a20_state = (a20_state != 0);
     if (a20_state != ((env->a20_mask >> 20) & 1)) {
-#if defined(DEBUG_MMU)
-        printf("A20 update: a20=%d\n", a20_state);
-#endif
+        MMU_DPRINTF("A20 update: a20=%d\n", a20_state);
+
         /* if the cpu is currently executing code, we must unlink it and
            all the potentially executing TB */
         cpu_interrupt(env, CPU_INTERRUPT_EXITTB);
@@ -388,9 +392,8 @@ void cpu_x86_update_cr0(CPUX86State *env, uint32_t new_cr0)
 {
     int pe_state;
 
-#if defined(DEBUG_MMU)
-    printf("CR0 update: CR0=0x%08x\n", new_cr0);
-#endif
+    MMU_DPRINTF("CR0 update: CR0=0x%08x\n", new_cr0);
+
     if ((new_cr0 & (CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK)) !=
         (env->cr[0] & (CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK))) {
         tlb_flush(env, 1);
@@ -431,18 +434,15 @@ void cpu_x86_update_cr3(CPUX86State *env, target_ulong new_cr3)
 {
     env->cr[3] = new_cr3;
     if (env->cr[0] & CR0_PG_MASK) {
-#if defined(DEBUG_MMU)
-        printf("CR3 update: CR3=" TARGET_FMT_lx "\n", new_cr3);
-#endif
+        MMU_DPRINTF("CR3 update: CR3=" TARGET_FMT_lx "\n", new_cr3);
         tlb_flush(env, 0);
     }
 }
 
 void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
 {
-#if defined(DEBUG_MMU)
-    printf("CR4 update: CR4=%08x\n", (uint32_t)env->cr[4]);
-#endif
+    MMU_DPRINTF("CR4 update: CR4=%08x\n", (uint32_t)env->cr[4]);
+
     if ((new_cr4 ^ env->cr[4]) &
         (CR4_PGE_MASK | CR4_PAE_MASK | CR4_PSE_MASK |
          CR4_SMEP_MASK | CR4_SMAP_MASK)) {
@@ -508,10 +508,9 @@ int cpu_x86_handle_mmu_fault(CPUX86State *env, target_ulong addr,
     target_ulong vaddr, virt_addr;
 
     is_user = mmu_idx == MMU_USER_IDX;
-#if defined(DEBUG_MMU)
-    printf("MMU fault: addr=" TARGET_FMT_lx " w=%d u=%d eip=" TARGET_FMT_lx "\n",
-           addr, is_write1, is_user, env->eip);
-#endif
+    MMU_DPRINTF("MMU fault: addr=" TARGET_FMT_lx " w=%d u=%d"
+                " eip=" TARGET_FMT_lx "\n",
+                addr, is_write1, is_user, env->eip);
     is_write = is_write1 & 1;
 
     if (!(env->cr[0] & CR0_PG_MASK)) {
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 3acff40..732e5fd 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -34,15 +34,13 @@
 #include "hyperv.h"
 #include "hw/pci/pci.h"
 
-//#define DEBUG_KVM
-
-#ifdef DEBUG_KVM
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
+#define DEBUG_KVM 0
+
+#define DPRINTF(fmt, ...) G_STMT_START \
+    if (DEBUG_KVM) { \
+        fprintf(stderr, fmt, ## __VA_ARGS__); \
+    } \
+    G_STMT_END
 
 #define MSR_KVM_WALL_CLOCK  0x11
 #define MSR_KVM_SYSTEM_TIME 0x12
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index 3247dee..b626b68 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -22,20 +22,22 @@
 #include "qemu/log.h"
 #include "helper.h"
 
-//#define DEBUG_PCALL
+#define DEBUG_PCALL 0
 
 #if !defined(CONFIG_USER_ONLY)
 #include "exec/softmmu_exec.h"
 #endif /* !defined(CONFIG_USER_ONLY) */
 
-#ifdef DEBUG_PCALL
-# define LOG_PCALL(...) qemu_log_mask(CPU_LOG_PCALL, ## __VA_ARGS__)
-# define LOG_PCALL_STATE(env)                                  \
-    log_cpu_state_mask(CPU_LOG_PCALL, (env), CPU_DUMP_CCOP)
-#else
-# define LOG_PCALL(...) do { } while (0)
-# define LOG_PCALL_STATE(env) do { } while (0)
-#endif
+#define LOG_PCALL(...) G_STMT_START \
+    if (DEBUG_PCALL) { \
+        qemu_log_mask(CPU_LOG_PCALL, ## __VA_ARGS__); \
+    } \
+    G_STMT_END
+#define LOG_PCALL_STATE(env) G_STMT_START \
+    if (DEBUG_PCALL) { \
+        log_cpu_state_mask(CPU_LOG_PCALL, (env), CPU_DUMP_CCOP); \
+    } \
+    G_STMT_END
 
 /* return non zero if error */
 static inline int load_segment(CPUX86State *env, uint32_t *e1_ptr,
-- 
1.7.10.4

[RFC 16/19] target-ppc: Refactor debug output macros

[Andreas Färber]

Make debug output compile-testable even if disabled.

Inline DEBUG_OP check in excp_helper.c.
Inline LOG_MMU_STATE() in mmu_helper.c.
Inline PPC_DEBUG_SPR check in translate_init.c.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-ppc/excp_helper.c    |   22 +++++++--------
 target-ppc/kvm.c            |    9 ++-----
 target-ppc/mem_helper.c     |    2 --
 target-ppc/mmu_helper.c     |   63 +++++++++++++++++++++----------------------
 target-ppc/translate.c      |   12 ++++-----
 target-ppc/translate_init.c |   10 +++----
 6 Dateien geändert, 55 Zeilen hinzugefügt(+), 63 Zeilen entfernt(-)

diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index 0a1ac86..54722c4 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -21,14 +21,14 @@
 
 #include "helper_regs.h"
 
-//#define DEBUG_OP
-//#define DEBUG_EXCEPTIONS
+#define DEBUG_OP 0
+#define DEBUG_EXCEPTIONS 0
 
-#ifdef DEBUG_EXCEPTIONS
-#  define LOG_EXCP(...) qemu_log(__VA_ARGS__)
-#else
-#  define LOG_EXCP(...) do { } while (0)
-#endif
+#define LOG_EXCP(...) G_STMT_START \
+    if (DEBUG_EXCEPTIONS) { \
+        qemu_log(__VA_ARGS__); \
+    } \
+    G_STMT_END
 
 /*****************************************************************************/
 /* PowerPC Hypercall emulation */
@@ -777,7 +777,7 @@ void ppc_hw_interrupt(CPUPPCState *env)
 }
 #endif /* !CONFIG_USER_ONLY */
 
-#if defined(DEBUG_OP)
+#ifndef CONFIG_USER_ONLY
 static void cpu_dump_rfi(target_ulong RA, target_ulong msr)
 {
     qemu_log("Return from exception at " TARGET_FMT_lx " with flags "
@@ -835,9 +835,9 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr,
     /* XXX: beware: this is false if VLE is supported */
     env->nip = nip & ~((target_ulong)0x00000003);
     hreg_store_msr(env, msr, 1);
-#if defined(DEBUG_OP)
-    cpu_dump_rfi(env->nip, env->msr);
-#endif
+    if (DEBUG_OP) {
+        cpu_dump_rfi(env->nip, env->msr);
+    }
     /* No need to raise an exception here,
      * as rfi is always the last insn of a TB
      */
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 2f4f068..0dc6657 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -37,15 +37,10 @@
 #include "hw/spapr.h"
 #include "hw/spapr_vio.h"
 
-//#define DEBUG_KVM
+#define DEBUG_KVM 0
 
-#ifdef DEBUG_KVM
 #define dprintf(fmt, ...) \
-    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-#else
-#define dprintf(fmt, ...) \
-    do { } while (0)
-#endif
+    do { if (DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
 
 #define PROC_DEVTREE_CPU      "/proc/device-tree/cpus/"
 
diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
index 902b1cd..5c7a5ce 100644
--- a/target-ppc/mem_helper.c
+++ b/target-ppc/mem_helper.c
@@ -26,8 +26,6 @@
 #include "exec/softmmu_exec.h"
 #endif /* !defined(CONFIG_USER_ONLY) */
 
-//#define DEBUG_OP
-
 /*****************************************************************************/
 /* Memory load and stores */
 
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index ee168f1..9340fbb 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -21,39 +21,36 @@
 #include "sysemu/kvm.h"
 #include "kvm_ppc.h"
 
-//#define DEBUG_MMU
-//#define DEBUG_BATS
-//#define DEBUG_SLB
-//#define DEBUG_SOFTWARE_TLB
+#define DEBUG_MMU 0
+#define DEBUG_BATS 0
+#define DEBUG_SLB 0
+#define DEBUG_SOFTWARE_TLB 0
 //#define DUMP_PAGE_TABLES
-//#define DEBUG_SOFTWARE_TLB
 //#define FLUSH_ALL_TLBS
 
-#ifdef DEBUG_MMU
-#  define LOG_MMU(...) qemu_log(__VA_ARGS__)
-#  define LOG_MMU_STATE(env) log_cpu_state((env), 0)
-#else
-#  define LOG_MMU(...) do { } while (0)
-#  define LOG_MMU_STATE(...) do { } while (0)
-#endif
-
-#ifdef DEBUG_SOFTWARE_TLB
-#  define LOG_SWTLB(...) qemu_log(__VA_ARGS__)
-#else
-#  define LOG_SWTLB(...) do { } while (0)
-#endif
-
-#ifdef DEBUG_BATS
-#  define LOG_BATS(...) qemu_log(__VA_ARGS__)
-#else
-#  define LOG_BATS(...) do { } while (0)
-#endif
-
-#ifdef DEBUG_SLB
-#  define LOG_SLB(...) qemu_log(__VA_ARGS__)
-#else
-#  define LOG_SLB(...) do { } while (0)
-#endif
+#define LOG_MMU(...) G_STMT_START \
+    if (DEBUG_MMU) { \
+        qemu_log(__VA_ARGS__); \
+    } \
+    G_STMT_END
+
+#define LOG_SWTLB(...) G_STMT_START \
+    if (DEBUG_SOFTWARE_TLB) { \
+        qemu_log(__VA_ARGS__); \
+    } \
+    G_STMT_END
+
+#define LOG_BATS(...) G_STMT_START \
+    if (DEBUG_BATS) { \
+        qemu_log(__VA_ARGS__); \
+    } \
+    G_STMT_END
+
+#define LOG_SLB(...) G_STMT_START \
+    if (DEBUG_SLB) { \
+        qemu_log(__VA_ARGS__); \
+    } \
+    G_STMT_END
 
 /*****************************************************************************/
 /* PowerPC MMU emulation */
@@ -534,7 +531,7 @@ static inline int get_bat(CPUPPCState *env, mmu_ctx_t *ctx,
         }
     }
     if (ret < 0) {
-#if defined(DEBUG_BATS)
+#if DEBUG_BATS
         if (qemu_log_enabled()) {
             LOG_BATS("no BAT match for " TARGET_FMT_lx ":\n", virtual);
             for (i = 0; i < 4; i++) {
@@ -1860,7 +1857,9 @@ int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address, int rw,
                      mmu_idx, TARGET_PAGE_SIZE);
         ret = 0;
     } else if (ret < 0) {
-        LOG_MMU_STATE(env);
+        if (DEBUG_MMU) {
+            log_cpu_state(env, 0);
+        }
         if (access_type == ACCESS_CODE) {
             switch (ret) {
             case -1:
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 37ce55f..c599fa7 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -32,14 +32,14 @@
 #define GDBSTUB_SINGLE_STEP 0x4
 
 /* Include definitions for instructions classes and implementations flags */
-//#define PPC_DEBUG_DISAS
+#define PPC_DEBUG_DISAS 0
 //#define DO_PPC_STATISTICS
 
-#ifdef PPC_DEBUG_DISAS
-#  define LOG_DISAS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
-#else
-#  define LOG_DISAS(...) do { } while (0)
-#endif
+#define LOG_DISAS(...) G_STMT_START \
+    if (PPC_DEBUG_DISAS) { \
+        qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__); \
+    } \
+    G_STMT_END
 /*****************************************************************************/
 /* Code translation helpers                                                  */
 
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 4f767c9..7ab4d61 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -31,7 +31,7 @@
 #include "sysemu/cpus.h"
 
 //#define PPC_DUMP_CPU
-//#define PPC_DEBUG_SPR
+#define PPC_DEBUG_SPR 0
 //#define PPC_DUMP_SPR_ACCESSES
 #if defined(CONFIG_USER_ONLY)
 #define TODO_USER_ONLY 1
@@ -611,10 +611,10 @@ static inline void spr_register (CPUPPCState *env, int num,
         printf("Error: Trying to register SPR %d (%03x) twice !\n", num, num);
         exit(1);
     }
-#if defined(PPC_DEBUG_SPR)
-    printf("*** register spr %d (%03x) %s val " TARGET_FMT_lx "\n", num, num,
-           name, initial_value);
-#endif
+    if (PPC_DEBUG_SPR) {
+        printf("*** register spr %d (%03x) %s val " TARGET_FMT_lx "\n",
+               num, num, name, initial_value);
+    }
     spr->name = name;
     spr->uea_read = uea_read;
     spr->uea_write = uea_write;
-- 
1.7.10.4

Re: [RFC 16/19] target-ppc: Refactor debug output macros

[Anthony Liguori]

Andreas Färber <afaerber@suse.de> writes:

> Make debug output compile-testable even if disabled.
>
> Inline DEBUG_OP check in excp_helper.c.
> Inline LOG_MMU_STATE() in mmu_helper.c.
> Inline PPC_DEBUG_SPR check in translate_init.c.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  target-ppc/excp_helper.c    |   22 +++++++--------
>  target-ppc/kvm.c            |    9 ++-----
>  target-ppc/mem_helper.c     |    2 --
>  target-ppc/mmu_helper.c     |   63 +++++++++++++++++++++----------------------
>  target-ppc/translate.c      |   12 ++++-----
>  target-ppc/translate_init.c |   10 +++----
>  6 Dateien geändert, 55 Zeilen hinzugefügt(+), 63 Zeilen entfernt(-)
>
> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
> index 0a1ac86..54722c4 100644
> --- a/target-ppc/excp_helper.c
> +++ b/target-ppc/excp_helper.c
> @@ -21,14 +21,14 @@
>  
>  #include "helper_regs.h"
>  
> -//#define DEBUG_OP
> -//#define DEBUG_EXCEPTIONS
> +#define DEBUG_OP 0
> +#define DEBUG_EXCEPTIONS 0
>  
> -#ifdef DEBUG_EXCEPTIONS
> -#  define LOG_EXCP(...) qemu_log(__VA_ARGS__)
> -#else
> -#  define LOG_EXCP(...) do { } while (0)
> -#endif
> +#define LOG_EXCP(...) G_STMT_START \
> +    if (DEBUG_EXCEPTIONS) { \
> +        qemu_log(__VA_ARGS__); \
> +    } \
> +    G_STMT_END

Just thinking out loud a bit..  This form becomes pretty common and it's
ashame to use a macro here if we don't have to.

I think:

static inline void LOG_EXCP(const char *fmt, ...)
{
    if (debug_exceptions) {
       va_list ap;
       va_start(ap, fmt);
       qemu_logv(fmt, ap);
       va_end(ap);
    }
}

Probably would have equivalent performance.  debug_exception would be
read-mostly and ought to be very predictable as a result.  I strongly
expect that the compiler would actually inline LOG_EXCP too.

I see LOG_EXCP and LOG_DIS in this series.  Perhaps we could just
introduce these functions and then make these flags run-time
controllable?

BTW, one advantage of this over your original proposal back to your
point is that you still won't catch linker errors with your proposal.
Dead code eliminate will kill off those branches before the linker ever
sees them.

Regards,

Anthony Liguori

>  
>  /*****************************************************************************/
>  /* PowerPC Hypercall emulation */
> @@ -777,7 +777,7 @@ void ppc_hw_interrupt(CPUPPCState *env)
>  }
>  #endif /* !CONFIG_USER_ONLY */
>  
> -#if defined(DEBUG_OP)
> +#ifndef CONFIG_USER_ONLY
>  static void cpu_dump_rfi(target_ulong RA, target_ulong msr)
>  {
>      qemu_log("Return from exception at " TARGET_FMT_lx " with flags "
> @@ -835,9 +835,9 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr,
>      /* XXX: beware: this is false if VLE is supported */
>      env->nip = nip & ~((target_ulong)0x00000003);
>      hreg_store_msr(env, msr, 1);
> -#if defined(DEBUG_OP)
> -    cpu_dump_rfi(env->nip, env->msr);
> -#endif
> +    if (DEBUG_OP) {
> +        cpu_dump_rfi(env->nip, env->msr);
> +    }
>      /* No need to raise an exception here,
>       * as rfi is always the last insn of a TB
>       */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 2f4f068..0dc6657 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -37,15 +37,10 @@
>  #include "hw/spapr.h"
>  #include "hw/spapr_vio.h"
>  
> -//#define DEBUG_KVM
> +#define DEBUG_KVM 0
>  
> -#ifdef DEBUG_KVM
>  #define dprintf(fmt, ...) \
> -    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> -#else
> -#define dprintf(fmt, ...) \
> -    do { } while (0)
> -#endif
> +    do { if (DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
>  
>  #define PROC_DEVTREE_CPU      "/proc/device-tree/cpus/"
>  
> diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
> index 902b1cd..5c7a5ce 100644
> --- a/target-ppc/mem_helper.c
> +++ b/target-ppc/mem_helper.c
> @@ -26,8 +26,6 @@
>  #include "exec/softmmu_exec.h"
>  #endif /* !defined(CONFIG_USER_ONLY) */
>  
> -//#define DEBUG_OP
> -
>  /*****************************************************************************/
>  /* Memory load and stores */
>  
> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> index ee168f1..9340fbb 100644
> --- a/target-ppc/mmu_helper.c
> +++ b/target-ppc/mmu_helper.c
> @@ -21,39 +21,36 @@
>  #include "sysemu/kvm.h"
>  #include "kvm_ppc.h"
>  
> -//#define DEBUG_MMU
> -//#define DEBUG_BATS
> -//#define DEBUG_SLB
> -//#define DEBUG_SOFTWARE_TLB
> +#define DEBUG_MMU 0
> +#define DEBUG_BATS 0
> +#define DEBUG_SLB 0
> +#define DEBUG_SOFTWARE_TLB 0
>  //#define DUMP_PAGE_TABLES
> -//#define DEBUG_SOFTWARE_TLB
>  //#define FLUSH_ALL_TLBS
>  
> -#ifdef DEBUG_MMU
> -#  define LOG_MMU(...) qemu_log(__VA_ARGS__)
> -#  define LOG_MMU_STATE(env) log_cpu_state((env), 0)
> -#else
> -#  define LOG_MMU(...) do { } while (0)
> -#  define LOG_MMU_STATE(...) do { } while (0)
> -#endif
> -
> -#ifdef DEBUG_SOFTWARE_TLB
> -#  define LOG_SWTLB(...) qemu_log(__VA_ARGS__)
> -#else
> -#  define LOG_SWTLB(...) do { } while (0)
> -#endif
> -
> -#ifdef DEBUG_BATS
> -#  define LOG_BATS(...) qemu_log(__VA_ARGS__)
> -#else
> -#  define LOG_BATS(...) do { } while (0)
> -#endif
> -
> -#ifdef DEBUG_SLB
> -#  define LOG_SLB(...) qemu_log(__VA_ARGS__)
> -#else
> -#  define LOG_SLB(...) do { } while (0)
> -#endif
> +#define LOG_MMU(...) G_STMT_START \
> +    if (DEBUG_MMU) { \
> +        qemu_log(__VA_ARGS__); \
> +    } \
> +    G_STMT_END
> +
> +#define LOG_SWTLB(...) G_STMT_START \
> +    if (DEBUG_SOFTWARE_TLB) { \
> +        qemu_log(__VA_ARGS__); \
> +    } \
> +    G_STMT_END
> +
> +#define LOG_BATS(...) G_STMT_START \
> +    if (DEBUG_BATS) { \
> +        qemu_log(__VA_ARGS__); \
> +    } \
> +    G_STMT_END
> +
> +#define LOG_SLB(...) G_STMT_START \
> +    if (DEBUG_SLB) { \
> +        qemu_log(__VA_ARGS__); \
> +    } \
> +    G_STMT_END
>  
>  /*****************************************************************************/
>  /* PowerPC MMU emulation */
> @@ -534,7 +531,7 @@ static inline int get_bat(CPUPPCState *env, mmu_ctx_t *ctx,
>          }
>      }
>      if (ret < 0) {
> -#if defined(DEBUG_BATS)
> +#if DEBUG_BATS
>          if (qemu_log_enabled()) {
>              LOG_BATS("no BAT match for " TARGET_FMT_lx ":\n", virtual);
>              for (i = 0; i < 4; i++) {
> @@ -1860,7 +1857,9 @@ int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address, int rw,
>                       mmu_idx, TARGET_PAGE_SIZE);
>          ret = 0;
>      } else if (ret < 0) {
> -        LOG_MMU_STATE(env);
> +        if (DEBUG_MMU) {
> +            log_cpu_state(env, 0);
> +        }
>          if (access_type == ACCESS_CODE) {
>              switch (ret) {
>              case -1:
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 37ce55f..c599fa7 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -32,14 +32,14 @@
>  #define GDBSTUB_SINGLE_STEP 0x4
>  
>  /* Include definitions for instructions classes and implementations flags */
> -//#define PPC_DEBUG_DISAS
> +#define PPC_DEBUG_DISAS 0
>  //#define DO_PPC_STATISTICS
>  
> -#ifdef PPC_DEBUG_DISAS
> -#  define LOG_DISAS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
> -#else
> -#  define LOG_DISAS(...) do { } while (0)
> -#endif
> +#define LOG_DISAS(...) G_STMT_START \
> +    if (PPC_DEBUG_DISAS) { \
> +        qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__); \
> +    } \
> +    G_STMT_END
>  /*****************************************************************************/
>  /* Code translation helpers                                                  */
>  
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 4f767c9..7ab4d61 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -31,7 +31,7 @@
>  #include "sysemu/cpus.h"
>  
>  //#define PPC_DUMP_CPU
> -//#define PPC_DEBUG_SPR
> +#define PPC_DEBUG_SPR 0
>  //#define PPC_DUMP_SPR_ACCESSES
>  #if defined(CONFIG_USER_ONLY)
>  #define TODO_USER_ONLY 1
> @@ -611,10 +611,10 @@ static inline void spr_register (CPUPPCState *env, int num,
>          printf("Error: Trying to register SPR %d (%03x) twice !\n", num, num);
>          exit(1);
>      }
> -#if defined(PPC_DEBUG_SPR)
> -    printf("*** register spr %d (%03x) %s val " TARGET_FMT_lx "\n", num, num,
> -           name, initial_value);
> -#endif
> +    if (PPC_DEBUG_SPR) {
> +        printf("*** register spr %d (%03x) %s val " TARGET_FMT_lx "\n",
> +               num, num, name, initial_value);
> +    }
>      spr->name = name;
>      spr->uea_read = uea_read;
>      spr->uea_write = uea_write;
> -- 
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 16/19] target-ppc: Refactor debug output macros

[Andreas Färber]

Am 27.01.2013 15:14, schrieb Anthony Liguori:
> Andreas Färber <afaerber@suse.de> writes:
>> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
>> index 0a1ac86..54722c4 100644
>> --- a/target-ppc/excp_helper.c
>> +++ b/target-ppc/excp_helper.c
>> @@ -21,14 +21,14 @@
>>  
>>  #include "helper_regs.h"
>>  
>> -//#define DEBUG_OP
>> -//#define DEBUG_EXCEPTIONS
>> +#define DEBUG_OP 0
>> +#define DEBUG_EXCEPTIONS 0
>>  
>> -#ifdef DEBUG_EXCEPTIONS
>> -#  define LOG_EXCP(...) qemu_log(__VA_ARGS__)
>> -#else
>> -#  define LOG_EXCP(...) do { } while (0)
>> -#endif
>> +#define LOG_EXCP(...) G_STMT_START \
>> +    if (DEBUG_EXCEPTIONS) { \
>> +        qemu_log(__VA_ARGS__); \
>> +    } \
>> +    G_STMT_END
> 
> Just thinking out loud a bit..  This form becomes pretty common and it's
> ashame to use a macro here if we don't have to.
> 
> I think:
> 
> static inline void LOG_EXCP(const char *fmt, ...)
> {
>     if (debug_exceptions) {
>        va_list ap;
>        va_start(ap, fmt);
>        qemu_logv(fmt, ap);
>        va_end(ap);
>     }
> }
> 
> Probably would have equivalent performance.  debug_exception would be
> read-mostly and ought to be very predictable as a result.  I strongly
> expect that the compiler would actually inline LOG_EXCP too.

Thanks for your early feedback. I merely tried to stay close to the
original code. I wouldn't mind inline functions either. Or even more
harmonization for that matter.

> I see LOG_EXCP and LOG_DIS in this series.  Perhaps we could just
> introduce these functions and then make these flags run-time
> controllable?

I was feeling conservative during that series in light of compile-time
decided conditional; if we want to go down that route we should probably
sprinkle quite some unlikely()s for optimization.

I think the if (0) { ... } approach would already catch a few things. As
a next step, some mechanism as proposed by Peter C. (?) to enable things
at configure-time could be built on top. Run-time would need some
stabilization phase to avoid command line compatibility issues.

> BTW, one advantage of this over your original proposal back to your
> point is that you still won't catch linker errors with your proposal.
> Dead code eliminate will kill off those branches before the linker ever
> sees them.

Linker errors would be limited to renamed/dropped/#ifdef'ed functions,
wouldn't they? In the past I caught that using existing --enable-debug.

My recurring issue is overlooking env->something after removing fields
from CPU_COMMON/CPUArchState. I was hoping that to be caught inside
if (0) { ... } during my 3x KVM + BSD + MinGW builds rather than
patching individual files.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [RFC 16/19] target-ppc: Refactor debug output macros

[Alexander Graf]

On 27.01.2013, at 15:35, Andreas Färber wrote:

> Am 27.01.2013 15:14, schrieb Anthony Liguori:
>> Andreas Färber <afaerber@suse.de> writes:
>>> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
>>> index 0a1ac86..54722c4 100644
>>> --- a/target-ppc/excp_helper.c
>>> +++ b/target-ppc/excp_helper.c
>>> @@ -21,14 +21,14 @@
>>> 
>>> #include "helper_regs.h"
>>> 
>>> -//#define DEBUG_OP
>>> -//#define DEBUG_EXCEPTIONS
>>> +#define DEBUG_OP 0
>>> +#define DEBUG_EXCEPTIONS 0
>>> 
>>> -#ifdef DEBUG_EXCEPTIONS
>>> -#  define LOG_EXCP(...) qemu_log(__VA_ARGS__)
>>> -#else
>>> -#  define LOG_EXCP(...) do { } while (0)
>>> -#endif
>>> +#define LOG_EXCP(...) G_STMT_START \
>>> +    if (DEBUG_EXCEPTIONS) { \
>>> +        qemu_log(__VA_ARGS__); \
>>> +    } \
>>> +    G_STMT_END
>> 
>> Just thinking out loud a bit..  This form becomes pretty common and it's
>> ashame to use a macro here if we don't have to.
>> 
>> I think:
>> 
>> static inline void LOG_EXCP(const char *fmt, ...)
>> {
>>    if (debug_exceptions) {
>>       va_list ap;
>>       va_start(ap, fmt);
>>       qemu_logv(fmt, ap);
>>       va_end(ap);
>>    }
>> }
>> 
>> Probably would have equivalent performance.  debug_exception would be
>> read-mostly and ought to be very predictable as a result.  I strongly
>> expect that the compiler would actually inline LOG_EXCP too.
> 
> Thanks for your early feedback. I merely tried to stay close to the
> original code. I wouldn't mind inline functions either. Or even more
> harmonization for that matter.

I fully agree. Just recently Scott revamped the openpic debug print code:


//#define DEBUG_OPENPIC

#ifdef DEBUG_OPENPIC
static const int debug_openpic = 1;
#else
static const int debug_openpic = 0;
#endif

#define DPRINTF(fmt, ...) do { \
        if (debug_openpic) { \
            printf(fmt , ## __VA_ARGS__); \
        } \
    } while (0)

I like that approach. It keeps all users identical. The #define stays identical. The callers stay identical. But we do get proper compile time checks. Of course Anthony's approach works too, but the thing I'd definitely like to see is that the #defines don't become numerical, but rather stay of an #ifdef basis.


Alex

Re: [RFC 16/19] target-ppc: Refactor debug output macros

[Andreas Färber]

Am 27.01.2013 15:46, schrieb Alexander Graf:
> 
> On 27.01.2013, at 15:35, Andreas Färber wrote:
> 
>> Am 27.01.2013 15:14, schrieb Anthony Liguori:
>>> Andreas Färber <afaerber@suse.de> writes:
>>>> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
>>>> index 0a1ac86..54722c4 100644
>>>> --- a/target-ppc/excp_helper.c
>>>> +++ b/target-ppc/excp_helper.c
>>>> @@ -21,14 +21,14 @@
>>>>
>>>> #include "helper_regs.h"
>>>>
>>>> -//#define DEBUG_OP
>>>> -//#define DEBUG_EXCEPTIONS
>>>> +#define DEBUG_OP 0
>>>> +#define DEBUG_EXCEPTIONS 0
>>>>
>>>> -#ifdef DEBUG_EXCEPTIONS
>>>> -#  define LOG_EXCP(...) qemu_log(__VA_ARGS__)
>>>> -#else
>>>> -#  define LOG_EXCP(...) do { } while (0)
>>>> -#endif
>>>> +#define LOG_EXCP(...) G_STMT_START \
>>>> +    if (DEBUG_EXCEPTIONS) { \
>>>> +        qemu_log(__VA_ARGS__); \
>>>> +    } \
>>>> +    G_STMT_END
>>>
>>> Just thinking out loud a bit..  This form becomes pretty common and it's
>>> ashame to use a macro here if we don't have to.
>>>
>>> I think:
>>>
>>> static inline void LOG_EXCP(const char *fmt, ...)
>>> {
>>>    if (debug_exceptions) {
>>>       va_list ap;
>>>       va_start(ap, fmt);
>>>       qemu_logv(fmt, ap);
>>>       va_end(ap);
>>>    }
>>> }
>>>
>>> Probably would have equivalent performance.  debug_exception would be
>>> read-mostly and ought to be very predictable as a result.  I strongly
>>> expect that the compiler would actually inline LOG_EXCP too.
>>
>> Thanks for your early feedback. I merely tried to stay close to the
>> original code. I wouldn't mind inline functions either. Or even more
>> harmonization for that matter.
> 
> I fully agree. Just recently Scott revamped the openpic debug print code:
> 
> 
> //#define DEBUG_OPENPIC
> 
> #ifdef DEBUG_OPENPIC
> static const int debug_openpic = 1;
> #else
> static const int debug_openpic = 0;
> #endif
> 
> #define DPRINTF(fmt, ...) do { \
>         if (debug_openpic) { \
>             printf(fmt , ## __VA_ARGS__); \
>         } \
>     } while (0)

Like :)

I'll wait for more feedback from affected maintainers though before I
redo or widen this series.
Or were you proposing to do a ppc-only refactoring à la Scott for 1.4?

Andreas

> I like that approach. It keeps all users identical. The #define stays identical. The callers stay identical. But we do get proper compile time checks. Of course Anthony's approach works too, but the thing I'd definitely like to see is that the #defines don't become numerical, but rather stay of an #ifdef basis.
> 
> 
> Alex

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [RFC 16/19] target-ppc: Refactor debug output macros

[Alexander Graf]

Am 27.01.2013 um 15:54 schrieb Andreas Färber <afaerber@suse.de>:

> Am 27.01.2013 15:46, schrieb Alexander Graf:
>> 
>> On 27.01.2013, at 15:35, Andreas Färber wrote:
>> 
>>> Am 27.01.2013 15:14, schrieb Anthony Liguori:
>>>> Andreas Färber <afaerber@suse.de> writes:
>>>>> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
>>>>> index 0a1ac86..54722c4 100644
>>>>> --- a/target-ppc/excp_helper.c
>>>>> +++ b/target-ppc/excp_helper.c
>>>>> @@ -21,14 +21,14 @@
>>>>> 
>>>>> #include "helper_regs.h"
>>>>> 
>>>>> -//#define DEBUG_OP
>>>>> -//#define DEBUG_EXCEPTIONS
>>>>> +#define DEBUG_OP 0
>>>>> +#define DEBUG_EXCEPTIONS 0
>>>>> 
>>>>> -#ifdef DEBUG_EXCEPTIONS
>>>>> -#  define LOG_EXCP(...) qemu_log(__VA_ARGS__)
>>>>> -#else
>>>>> -#  define LOG_EXCP(...) do { } while (0)
>>>>> -#endif
>>>>> +#define LOG_EXCP(...) G_STMT_START \
>>>>> +    if (DEBUG_EXCEPTIONS) { \
>>>>> +        qemu_log(__VA_ARGS__); \
>>>>> +    } \
>>>>> +    G_STMT_END
>>>> 
>>>> Just thinking out loud a bit..  This form becomes pretty common and it's
>>>> ashame to use a macro here if we don't have to.
>>>> 
>>>> I think:
>>>> 
>>>> static inline void LOG_EXCP(const char *fmt, ...)
>>>> {
>>>>   if (debug_exceptions) {
>>>>      va_list ap;
>>>>      va_start(ap, fmt);
>>>>      qemu_logv(fmt, ap);
>>>>      va_end(ap);
>>>>   }
>>>> }
>>>> 
>>>> Probably would have equivalent performance.  debug_exception would be
>>>> read-mostly and ought to be very predictable as a result.  I strongly
>>>> expect that the compiler would actually inline LOG_EXCP too.
>>> 
>>> Thanks for your early feedback. I merely tried to stay close to the
>>> original code. I wouldn't mind inline functions either. Or even more
>>> harmonization for that matter.
>> 
>> I fully agree. Just recently Scott revamped the openpic debug print code:
>> 
>> 
>> //#define DEBUG_OPENPIC
>> 
>> #ifdef DEBUG_OPENPIC
>> static const int debug_openpic = 1;
>> #else
>> static const int debug_openpic = 0;
>> #endif
>> 
>> #define DPRINTF(fmt, ...) do { \
>>        if (debug_openpic) { \
>>            printf(fmt , ## __VA_ARGS__); \
>>        } \
>>    } while (0)
> 
> Like :)
> 
> I'll wait for more feedback from affected maintainers though before I
> redo or widen this series.
> Or were you proposing to do a ppc-only refactoring à la Scott for 1.4?

No, I think it makes a lot more sense to do this tree-wide after 1.4 :).

Alex

> 
> Andreas
> 
>> I like that approach. It keeps all users identical. The #define stays identical. The callers stay identical. But we do get proper compile time checks. Of course Anthony's approach works too, but the thing I'd definitely like to see is that the #defines don't become numerical, but rather stay of an #ifdef basis.
>> 
>> 
>> Alex
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[RFC 17/19] target-s390x: Refactor debug output macros

[Andreas Färber]

Make debug output compile-testable even if disabled.

Inline S390X_DEBUG_DISAS in translate.c.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-s390x/cc_helper.c   |   13 +++++++------
 target-s390x/fpu_helper.c  |   13 +++++++------
 target-s390x/helper.c      |   42 ++++++++++++++++++------------------------
 target-s390x/int_helper.c  |   13 +++++++------
 target-s390x/kvm.c         |    9 ++-------
 target-s390x/mem_helper.c  |   13 +++++++------
 target-s390x/misc_helper.c |   13 +++++++------
 target-s390x/translate.c   |   18 ++++++++----------
 8 Dateien geändert, 63 Zeilen hinzugefügt(+), 71 Zeilen entfernt(-)

diff --git a/target-s390x/cc_helper.c b/target-s390x/cc_helper.c
index a6d60bf..a6fad6a 100644
--- a/target-s390x/cc_helper.c
+++ b/target-s390x/cc_helper.c
@@ -22,12 +22,13 @@
 #include "helper.h"
 #include "qemu/host-utils.h"
 
-/* #define DEBUG_HELPER */
-#ifdef DEBUG_HELPER
-#define HELPER_LOG(x...) qemu_log(x)
-#else
-#define HELPER_LOG(x...)
-#endif
+#define DEBUG_HELPER 0
+
+#define HELPER_LOG(...) G_STMT_START \
+    if (DEBUG_HELPER) { \
+        qemu_log(__VA_ARGS__); \
+    } \
+    G_STMT_END
 
 static uint32_t cc_calc_ltgt_32(int32_t src, int32_t dst)
 {
diff --git a/target-s390x/fpu_helper.c b/target-s390x/fpu_helper.c
index 94375b6..aba5e2c 100644
--- a/target-s390x/fpu_helper.c
+++ b/target-s390x/fpu_helper.c
@@ -25,12 +25,13 @@
 #include "exec/softmmu_exec.h"
 #endif
 
-/* #define DEBUG_HELPER */
-#ifdef DEBUG_HELPER
-#define HELPER_LOG(x...) qemu_log(x)
-#else
-#define HELPER_LOG(x...)
-#endif
+#define DEBUG_HELPER 0
+
+#define HELPER_LOG(...) G_STMT_START \
+    if (DEBUG_HELPER) { \
+        qemu_log(__VA_ARGS__); \
+    } \
+    G_STMT_END
 
 #define RET128(F) (env->retxl = F.low, F.high)
 
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 76fd558..90d62db 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -25,30 +25,24 @@
 #include "sysemu/sysemu.h"
 #endif
 
-//#define DEBUG_S390
-//#define DEBUG_S390_PTE
-//#define DEBUG_S390_STDOUT
-
-#ifdef DEBUG_S390
-#ifdef DEBUG_S390_STDOUT
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stderr, fmt, ## __VA_ARGS__); \
-         qemu_log(fmt, ##__VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { qemu_log(fmt, ## __VA_ARGS__); } while (0)
-#endif
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
-
-#ifdef DEBUG_S390_PTE
-#define PTE_DPRINTF DPRINTF
-#else
-#define PTE_DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
+#define DEBUG_S390 0
+#define DEBUG_S390_PTE 0
+#define DEBUG_S390_STDOUT 0
+
+#define DPRINTF(fmt, ...) G_STMT_START \
+    if (DEBUG_S390) { \
+        if (DEBUG_S390_STDOUT) { \
+            fprintf(stderr, fmt, ## __VA_ARGS__); \
+        } \
+        qemu_log(fmt, ## __VA_ARGS__); \
+    } \
+    G_STMT_END
+
+#define PTE_DPRINTF(fmt, ...) G_STMT_START \
+    if (DEBUG_S390_PTE) { \
+        DPRINTF(fmt, ## __VA_ARGS__); \
+    } \
+    G_STMT_END
 
 #ifndef CONFIG_USER_ONLY
 void s390x_tod_timer(void *opaque)
diff --git a/target-s390x/int_helper.c b/target-s390x/int_helper.c
index 6858301..acf2de0 100644
--- a/target-s390x/int_helper.c
+++ b/target-s390x/int_helper.c
@@ -22,12 +22,13 @@
 #include "qemu/host-utils.h"
 #include "helper.h"
 
-/* #define DEBUG_HELPER */
-#ifdef DEBUG_HELPER
-#define HELPER_LOG(x...) qemu_log(x)
-#else
-#define HELPER_LOG(x...)
-#endif
+#define DEBUG_HELPER 0
+
+#define HELPER_LOG(...) G_STMT_START \
+    if (DEBUG_HELPER) { \
+        qemu_log(__VA_ARGS__); \
+    } \
+    G_STMT_END
 
 /* 64/64 -> 128 unsigned multiplication */
 uint64_t HELPER(mul128)(CPUS390XState *env, uint64_t v1, uint64_t v2)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index add6a58..ee4c7f9 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -35,15 +35,10 @@
 #include "cpu.h"
 #include "sysemu/device_tree.h"
 
-/* #define DEBUG_KVM */
+#define DEBUG_KVM 0
 
-#ifdef DEBUG_KVM
 #define dprintf(fmt, ...) \
-    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-#else
-#define dprintf(fmt, ...) \
-    do { } while (0)
-#endif
+    do { if (DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
 
 #define IPA0_DIAG                       0x8300
 #define IPA0_SIGP                       0xae00
diff --git a/target-s390x/mem_helper.c b/target-s390x/mem_helper.c
index 372334b..99e3d45 100644
--- a/target-s390x/mem_helper.c
+++ b/target-s390x/mem_helper.c
@@ -61,12 +61,13 @@ void tlb_fill(CPUS390XState *env, target_ulong addr, int is_write, int mmu_idx,
 
 #endif
 
-/* #define DEBUG_HELPER */
-#ifdef DEBUG_HELPER
-#define HELPER_LOG(x...) qemu_log(x)
-#else
-#define HELPER_LOG(x...)
-#endif
+#define DEBUG_HELPER 0
+
+#define HELPER_LOG(...) G_STMT_START \
+    if (DEBUG_HELPER) { \
+        qemu_log(__VA_ARGS__); \
+    } \
+    G_STMT_END
 
 #ifndef CONFIG_USER_ONLY
 static void mvc_fast_memset(CPUS390XState *env, uint32_t l, uint64_t dest,
diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
index 09301d0..72dbdc1 100644
--- a/target-s390x/misc_helper.c
+++ b/target-s390x/misc_helper.c
@@ -34,12 +34,13 @@
 #include "sysemu/sysemu.h"
 #endif
 
-/* #define DEBUG_HELPER */
-#ifdef DEBUG_HELPER
-#define HELPER_LOG(x...) qemu_log(x)
-#else
-#define HELPER_LOG(x...)
-#endif
+#define DEBUG_HELPER 0
+
+#define HELPER_LOG(...) G_STMT_START \
+    if (DEBUG_HELPER) { \
+        qemu_log(__VA_ARGS__); \
+    } \
+    G_STMT_END
 
 /* Raise an exception dynamically from a helper function.  */
 void QEMU_NORETURN runtime_exception(CPUS390XState *env, int excp,
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index a57296c..d72f9e0 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -19,14 +19,14 @@
  */
 
 /* #define DEBUG_INLINE_BRANCHES */
-#define S390X_DEBUG_DISAS
-/* #define S390X_DEBUG_DISAS_VERBOSE */
+#define S390X_DEBUG_DISAS 1
+#define S390X_DEBUG_DISAS_VERBOSE 0
 
-#ifdef S390X_DEBUG_DISAS_VERBOSE
-#  define LOG_DISAS(...) qemu_log(__VA_ARGS__)
-#else
-#  define LOG_DISAS(...) do { } while (0)
-#endif
+#define LOG_DISAS(...) G_STMT_START \
+    if (S390X_DEBUG_DISAS_VERBOSE) { \
+        qemu_log(__VA_ARGS__); \
+    } \
+    G_STMT_END
 
 #include "cpu.h"
 #include "disas/disas.h"
@@ -4859,13 +4859,11 @@ static inline void gen_intermediate_code_internal(CPUS390XState *env,
         tb->icount = num_insns;
     }
 
-#if defined(S390X_DEBUG_DISAS)
-    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
+    if (S390X_DEBUG_DISAS && qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
         log_target_disas(env, pc_start, dc.pc - pc_start, 1);
         qemu_log("\n");
     }
-#endif
 }
 
 void gen_intermediate_code (CPUS390XState *env, struct TranslationBlock *tb)
-- 
1.7.10.4