[PATCH v3 0/7] silence the compiler warnings

[PATCH v3 0/7] silence the compiler warnings

[Chen Qun]

Hi folks,
  This series fix some "fall through" warnings reported by GCC_9.3. They've been 
reviewed for a long time. Some of these patchs may be important for QEMU 5.2. 
Such as the Patch6 miss a break statement. Others only add "fall through" 
comments and may not have a negative impact for QEMU 5.2.


Thanks,
Chen Qun


Since v2:
- Patch3：Add Richard Henderson、Philippe Mathieu-Daudé and Thomas Huth reviewed tag.
- Patch4: Laurent pull it to master, remove it.
- Patch6->Patch5: Add Richard Henderson and Philippe Mathieu-Daudé reviewed tag.
- Patch7->Patch6: Add Thomas Huth reviewed tag and David Gibson acked tag.
- Patch8->Patch7: Tweak LOG_UNIMP message base on Thomas Huth comment; 
  Add Philippe Mathieu-Daudé and Thomas Huth reviewed tag; Add David Gibson acked tag.

Since v1:
- Patch1: Add comments to explain the two case of fall through.
  Addressed Richard Henderson and Thomas Huth review comment.
- Patch2: Addressed Peter Maydell review comment.
- Patch3: Add QEMU_NORETURN to cpu_exit_tb_from_sighandler() function to
  avoid the compiler warnings.
- Patch4: Addressed Thomas Huth review comment.
- Patch5: Addressed Artyom Tarasenko and Philippe Mathieu-Daudé review
  comment.
- Patch6: Combine the /* fall through */ to the preceding comments.
  Addressed  Artyom Tarasenko review comment.
- Patch7: Add a "break" statement here instead of /* fall through */
  comments.
- Patch8: Replace the TODO by a LOG_UNIMP call and add break statement
- Patch9: Discard this patch since a patch already exists for fix this 
  issue(https://lore.kernel.org/qemu-devel/20200711154242.41222-1-ysato@users)


Chen Qun (7):
  target/i386: silence the compiler warnings in gen_shiftd_rm_T1
  hw/intc/arm_gicv3_kvm: silence the compiler warnings
  accel/tcg/user-exec: silence the compiler warnings
  target/sparc/translate: silence the compiler warnings
  target/sparc/win_helper: silence the compiler warnings
  ppc: Add a missing break for PPC6xx_INPUT_TBEN
  target/ppc: replaced the TODO with LOG_UNIMP and add break for silence
    warnings

 accel/tcg/user-exec.c     | 3 ++-
 hw/intc/arm_gicv3_kvm.c   | 8 ++++++++
 hw/ppc/ppc.c              | 1 +
 target/i386/translate.c   | 7 +++++--
 target/ppc/mmu_helper.c   | 5 +++--
 target/sparc/translate.c  | 2 +-
 target/sparc/win_helper.c | 2 +-
 7 files changed, 21 insertions(+), 7 deletions(-)

-- 
2.27.0

[PATCH v3 1/7] target/i386: silence the compiler warnings in gen_shiftd_rm_T1

[Chen Qun]

The current "#ifdef TARGET_X86_64" statement affects
the compiler's determination of fall through.

When using -Wimplicit-fallthrough in our CFLAGS on GCC9, the compiler showed warning:
target/i386/translate.c: In function ‘gen_shiftd_rm_T1’:
target/i386/translate.c:1773:12: warning: this statement may fall through [-Wimplicit-fallthrough=]
         if (is_right) {
            ^
target/i386/translate.c:1782:5: note: here
     case MO_32:
     ^~~~

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
v1->v2: Add comments to explain the two case of fall through,
depending on whether TARGET_X86_64 is defined.

Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <ehabkost@redhat.com>
---
 target/i386/translate.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index caea6f5fb1..77cb66208e 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -1777,9 +1777,12 @@ static void gen_shiftd_rm_T1(DisasContext *s, MemOp ot, int op1,
         } else {
             tcg_gen_deposit_tl(s->T1, s->T0, s->T1, 16, 16);
         }
-        /* FALLTHRU */
-#ifdef TARGET_X86_64
+        /*
+         * If TARGET_X86_64 defined then fall through into MO_32 case,
+         * otherwise fall through default case.
+         */
     case MO_32:
+#ifdef TARGET_X86_64
         /* Concatenate the two 32-bit values and use a 64-bit shift.  */
         tcg_gen_subi_tl(s->tmp0, count, 1);
         if (is_right) {
-- 
2.27.0

[PATCH v3 2/7] hw/intc/arm_gicv3_kvm: silence the compiler warnings

[Chen Qun]

When using -Wimplicit-fallthrough in our CFLAGS on GCC9, the compiler showed warning:
hw/intc/arm_gicv3_kvm.c: In function ‘kvm_arm_gicv3_put’:
hw/intc/arm_gicv3_kvm.c:484:13: warning: this statement may fall through [-Wimplicit-fallthrough=]
             kvm_gicc_access(s, ICC_AP0R_EL1(1), ncpu, &reg64, true);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hw/intc/arm_gicv3_kvm.c:485:9: note: here
         default:
         ^~~~~~~
hw/intc/arm_gicv3_kvm.c:495:13: warning: this statement may fall through [-Wimplicit-fallthrough=]
             kvm_gicc_access(s, ICC_AP1R_EL1(2), ncpu, &reg64, true);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hw/intc/arm_gicv3_kvm.c:496:9: note: here
         case 6:
         ^~~~
hw/intc/arm_gicv3_kvm.c:498:13: warning: this statement may fall through [-Wimplicit-fallthrough=]
             kvm_gicc_access(s, ICC_AP1R_EL1(1), ncpu, &reg64, true);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hw/intc/arm_gicv3_kvm.c:499:9: note: here
         default:
         ^~~~~~~

hw/intc/arm_gicv3_kvm.c: In function ‘kvm_arm_gicv3_get’:
hw/intc/arm_gicv3_kvm.c:634:37: warning: this statement may fall through [-Wimplicit-fallthrough=]
             c->icc_apr[GICV3_G0][2] = reg64;
             ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
hw/intc/arm_gicv3_kvm.c:635:9: note: here
         case 6:
         ^~~~
hw/intc/arm_gicv3_kvm.c:637:37: warning: this statement may fall through [-Wimplicit-fallthrough=]
             c->icc_apr[GICV3_G0][1] = reg64;
             ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
hw/intc/arm_gicv3_kvm.c:638:9: note: here
         default:
         ^~~~~~~
hw/intc/arm_gicv3_kvm.c:648:39: warning: this statement may fall through [-Wimplicit-fallthrough=]
             c->icc_apr[GICV3_G1NS][2] = reg64;
             ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
hw/intc/arm_gicv3_kvm.c:649:9: note: here
         case 6:
         ^~~~
hw/intc/arm_gicv3_kvm.c:651:39: warning: this statement may fall through [-Wimplicit-fallthrough=]
             c->icc_apr[GICV3_G1NS][1] = reg64;
             ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
hw/intc/arm_gicv3_kvm.c:652:9: note: here
         default:
         ^~~~~~~

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
---
 hw/intc/arm_gicv3_kvm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 187eb054e0..d040a5d1e9 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -478,9 +478,11 @@ static void kvm_arm_gicv3_put(GICv3State *s)
             kvm_gicc_access(s, ICC_AP0R_EL1(3), ncpu, &reg64, true);
             reg64 = c->icc_apr[GICV3_G0][2];
             kvm_gicc_access(s, ICC_AP0R_EL1(2), ncpu, &reg64, true);
+            /* fall through */
         case 6:
             reg64 = c->icc_apr[GICV3_G0][1];
             kvm_gicc_access(s, ICC_AP0R_EL1(1), ncpu, &reg64, true);
+            /* fall through */
         default:
             reg64 = c->icc_apr[GICV3_G0][0];
             kvm_gicc_access(s, ICC_AP0R_EL1(0), ncpu, &reg64, true);
@@ -492,9 +494,11 @@ static void kvm_arm_gicv3_put(GICv3State *s)
             kvm_gicc_access(s, ICC_AP1R_EL1(3), ncpu, &reg64, true);
             reg64 = c->icc_apr[GICV3_G1NS][2];
             kvm_gicc_access(s, ICC_AP1R_EL1(2), ncpu, &reg64, true);
+            /* fall through */
         case 6:
             reg64 = c->icc_apr[GICV3_G1NS][1];
             kvm_gicc_access(s, ICC_AP1R_EL1(1), ncpu, &reg64, true);
+            /* fall through */
         default:
             reg64 = c->icc_apr[GICV3_G1NS][0];
             kvm_gicc_access(s, ICC_AP1R_EL1(0), ncpu, &reg64, true);
@@ -631,9 +635,11 @@ static void kvm_arm_gicv3_get(GICv3State *s)
             c->icc_apr[GICV3_G0][3] = reg64;
             kvm_gicc_access(s, ICC_AP0R_EL1(2), ncpu, &reg64, false);
             c->icc_apr[GICV3_G0][2] = reg64;
+            /* fall through */
         case 6:
             kvm_gicc_access(s, ICC_AP0R_EL1(1), ncpu, &reg64, false);
             c->icc_apr[GICV3_G0][1] = reg64;
+            /* fall through */
         default:
             kvm_gicc_access(s, ICC_AP0R_EL1(0), ncpu, &reg64, false);
             c->icc_apr[GICV3_G0][0] = reg64;
@@ -645,9 +651,11 @@ static void kvm_arm_gicv3_get(GICv3State *s)
             c->icc_apr[GICV3_G1NS][3] = reg64;
             kvm_gicc_access(s, ICC_AP1R_EL1(2), ncpu, &reg64, false);
             c->icc_apr[GICV3_G1NS][2] = reg64;
+            /* fall through */
         case 6:
             kvm_gicc_access(s, ICC_AP1R_EL1(1), ncpu, &reg64, false);
             c->icc_apr[GICV3_G1NS][1] = reg64;
+            /* fall through */
         default:
             kvm_gicc_access(s, ICC_AP1R_EL1(0), ncpu, &reg64, false);
             c->icc_apr[GICV3_G1NS][0] = reg64;
-- 
2.27.0

[PATCH v3 3/7] accel/tcg/user-exec: silence the compiler warnings

[Chen Qun]

When using -Wimplicit-fallthrough in our CFLAGS on GCC9, the compiler showed warning:
../accel/tcg/user-exec.c: In function ‘handle_cpu_signal’:
../accel/tcg/user-exec.c:169:13: warning: this statement may fall through [-Wimplicit-fallthrough=]
  169 |             cpu_exit_tb_from_sighandler(cpu, old_set);
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../accel/tcg/user-exec.c:172:9: note: here
  172 |         default:

Mark the cpu_exit_tb_from_sighandler() function with QEMU_NORETURN to fix it.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
v1->v2: Add QEMU_NORETURN to cpu_exit_tb_from_sighandler() function
to avoid the compiler warnings(Base on Thomas's and Richard's comments).

Cc: Thomas Huth <thuth@redhat.com>
Cc: Riku Voipio <riku.voipio@iki.fi>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/tcg/user-exec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 4ebe25461a..293ee86ea4 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -49,7 +49,8 @@ __thread uintptr_t helper_retaddr;
 /* exit the current TB from a signal handler. The host registers are
    restored in a state compatible with the CPU emulator
  */
-static void cpu_exit_tb_from_sighandler(CPUState *cpu, sigset_t *old_set)
+static void QEMU_NORETURN cpu_exit_tb_from_sighandler(CPUState *cpu,
+                                                      sigset_t *old_set)
 {
     /* XXX: use siglongjmp ? */
     sigprocmask(SIG_SETMASK, old_set, NULL);
-- 
2.27.0

Re: [PATCH v3 3/7] accel/tcg/user-exec: silence the compiler warnings

[Alex Bennée]

Chen Qun <kuhn.chenqun@huawei.com> writes:

> When using -Wimplicit-fallthrough in our CFLAGS on GCC9, the compiler showed warning:
> ../accel/tcg/user-exec.c: In function ‘handle_cpu_signal’:
> ../accel/tcg/user-exec.c:169:13: warning: this statement may fall through [-Wimplicit-fallthrough=]
>   169 |             cpu_exit_tb_from_sighandler(cpu, old_set);
>       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../accel/tcg/user-exec.c:172:9: note: here
>   172 |         default:
>
> Mark the cpu_exit_tb_from_sighandler() function with QEMU_NORETURN to fix it.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

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

-- 
Alex Bennée

[PATCH v3 4/7] target/sparc/translate: silence the compiler warnings

[Chen Qun]

When using -Wimplicit-fallthrough in our CFLAGS on GCC9, the compiler showed warning:
target/sparc/translate.c: In function ‘gen_st_asi’:
target/sparc/translate.c:2320:12: warning: this statement may fall through [-Wimplicit-fallthrough=]
 2320 |         if (!(dc->def->features & CPU_FEATURE_HYPV)) {
      |            ^
target/sparc/translate.c:2329:5: note: here
 2329 |     case GET_ASI_DIRECT:
      |     ^~~~

The "fall through" statement place is not correctly identified by the compiler.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Artyom Tarasenko <atar4qemu@gmail.com>
---
 target/sparc/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 1a4efd4ed6..a3d9aaa46b 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -2324,8 +2324,8 @@ static void gen_st_asi(DisasContext *dc, TCGv src, TCGv addr,
         }
         /* in OpenSPARC T1+ CPUs TWINX ASIs in store instructions
          * are ST_BLKINIT_ ASIs */
-        /* fall through */
 #endif
+        /* fall through */
     case GET_ASI_DIRECT:
         gen_address_mask(dc, addr);
         tcg_gen_qemu_st_tl(src, addr, da.mem_idx, da.memop);
-- 
2.27.0

[PATCH v3 5/7] target/sparc/win_helper: silence the compiler warnings

[Chen Qun]

When using -Wimplicit-fallthrough in our CFLAGS on GCC9, the compiler showed warning:
target/sparc/win_helper.c: In function ‘get_gregset’:
target/sparc/win_helper.c:304:9: warning: this statement may fall through [-Wimplicit-fallthrough=]
  304 |         trace_win_helper_gregset_error(pstate);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
target/sparc/win_helper.c:306:5: note: here
  306 |     case 0:
      |     ^~~~

Add the corresponding "fall through" comment to fix it.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
v1->v2: Combine the /* fall through */ to the preceding comments
(Base on Philippe's comments).

Cc: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Artyom Tarasenko <atar4qemu@gmail.com>
---
 target/sparc/win_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/sparc/win_helper.c b/target/sparc/win_helper.c
index 8290a21142..e78660b60a 100644
--- a/target/sparc/win_helper.c
+++ b/target/sparc/win_helper.c
@@ -302,7 +302,7 @@ static inline uint64_t *get_gregset(CPUSPARCState *env, uint32_t pstate)
     switch (pstate) {
     default:
         trace_win_helper_gregset_error(pstate);
-        /* pass through to normal set of global registers */
+        /* fall through to normal set of global registers */
     case 0:
         return env->bgregs;
     case PS_AG:
-- 
2.27.0

[PATCH v3 6/7] ppc: Add a missing break for PPC6xx_INPUT_TBEN

[Chen Qun]

When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning:
hw/ppc/ppc.c: In function ‘ppc6xx_set_irq’:
hw/ppc/ppc.c:118:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
  118 |             if (level) {
      |                ^
hw/ppc/ppc.c:123:9: note: here
  123 |         case PPC6xx_INPUT_INT:
      |         ^~~~

According to the discussion, a break statement needs to be added here.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
---
v1->v2: Add a "break" statement here instead of /* fall through */ comments
(Base on Thomas's and David review).

Cc: Thomas Huth <thuth@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/ppc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 4a11fb1640..1b98272076 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -120,6 +120,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
             } else {
                 cpu_ppc_tb_stop(env);
             }
+            break;
         case PPC6xx_INPUT_INT:
             /* Level sensitive - active high */
             LOG_IRQ("%s: set the external IRQ state to %d\n",
-- 
2.27.0

Re: [PATCH-for-5.2 v3 6/7] ppc: Add a missing break for PPC6xx_INPUT_TBEN

[Philippe Mathieu-Daudé]

David, can you queue this patch for 5.2 (bugfix)?

On 11/16/20 3:48 AM, Chen Qun wrote:
> When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning:
> hw/ppc/ppc.c: In function ‘ppc6xx_set_irq’:
> hw/ppc/ppc.c:118:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
>   118 |             if (level) {
>       |                ^
> hw/ppc/ppc.c:123:9: note: here
>   123 |         case PPC6xx_INPUT_INT:
>       |         ^~~~
> 
> According to the discussion, a break statement needs to be added here.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> v1->v2: Add a "break" statement here instead of /* fall through */ comments
> (Base on Thomas's and David review).
> 
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/ppc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 4a11fb1640..1b98272076 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -120,6 +120,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>              } else {
>                  cpu_ppc_tb_stop(env);
>              }
> +            break;
>          case PPC6xx_INPUT_INT:
>              /* Level sensitive - active high */
>              LOG_IRQ("%s: set the external IRQ state to %d\n",
> 

Re: [PATCH-for-5.2 v3 6/7] ppc: Add a missing break for PPC6xx_INPUT_TBEN

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2098 bytes --]

On Mon, Nov 16, 2020 at 12:46:32PM +0100, Philippe Mathieu-Daudé wrote:
> David, can you queue this patch for 5.2 (bugfix)?

Sorry about this, I've been on vacation.

Although it is a bugfix, it's been there for a very long time and
no-one's hit it in practice.

So, I'm disinclined to push it in this late in the 5.2 cycle.

> 
> On 11/16/20 3:48 AM, Chen Qun wrote:
> > When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning:
> > hw/ppc/ppc.c: In function ‘ppc6xx_set_irq’:
> > hw/ppc/ppc.c:118:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >   118 |             if (level) {
> >       |                ^
> > hw/ppc/ppc.c:123:9: note: here
> >   123 |         case PPC6xx_INPUT_INT:
> >       |         ^~~~
> > 
> > According to the discussion, a break statement needs to be added here.
> > 
> > Reported-by: Euler Robot <euler.robot@huawei.com>
> > Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > Acked-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > v1->v2: Add a "break" statement here instead of /* fall through */ comments
> > (Base on Thomas's and David review).
> > 
> > Cc: Thomas Huth <thuth@redhat.com>
> > Cc: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/ppc/ppc.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> > index 4a11fb1640..1b98272076 100644
> > --- a/hw/ppc/ppc.c
> > +++ b/hw/ppc/ppc.c
> > @@ -120,6 +120,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
> >              } else {
> >                  cpu_ppc_tb_stop(env);
> >              }
> > +            break;
> >          case PPC6xx_INPUT_INT:
> >              /* Level sensitive - active high */
> >              LOG_IRQ("%s: set the external IRQ state to %d\n",
> > 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 6/7] ppc: Add a missing break for PPC6xx_INPUT_TBEN

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]

On Mon, Nov 16, 2020 at 10:48:09AM +0800, Chen Qun wrote:
> When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning:
> hw/ppc/ppc.c: In function ‘ppc6xx_set_irq’:
> hw/ppc/ppc.c:118:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
>   118 |             if (level) {
>       |                ^
> hw/ppc/ppc.c:123:9: note: here
>   123 |         case PPC6xx_INPUT_INT:
>       |         ^~~~
> 
> According to the discussion, a break statement needs to be added here.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>

Applied to ppc-for-6.0, thanks.

> ---
> v1->v2: Add a "break" statement here instead of /* fall through */ comments
> (Base on Thomas's and David review).
> 
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/ppc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 4a11fb1640..1b98272076 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -120,6 +120,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>              } else {
>                  cpu_ppc_tb_stop(env);
>              }
> +            break;
>          case PPC6xx_INPUT_INT:
>              /* Level sensitive - active high */
>              LOG_IRQ("%s: set the external IRQ state to %d\n",

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 7/7] target/ppc: replaced the TODO with LOG_UNIMP and add break for silence warnings

[Chen Qun]

When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning:
target/ppc/mmu_helper.c: In function ‘dump_mmu’:
target/ppc/mmu_helper.c:1351:12: warning: this statement may fall through [-Wimplicit-fallthrough=]
 1351 |         if (ppc64_v3_radix(env_archcpu(env))) {
      |            ^
target/ppc/mmu_helper.c:1358:5: note: here
 1358 |     default:
      |     ^~~~~~~

Use "qemu_log_mask(LOG_UNIMP**)" instead of the TODO comment.
And add the break statement to fix it.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
---
v1->v2: replace the TODO by a LOG_UNIMP call and add break statement(Base on Philippe's comments)

Cc: Thomas Huth <thuth@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/ppc/mmu_helper.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 8972714775..3bb50546f8 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -1349,11 +1349,12 @@ void dump_mmu(CPUPPCState *env)
         break;
     case POWERPC_MMU_3_00:
         if (ppc64_v3_radix(env_archcpu(env))) {
-            /* TODO - Unsupported */
+            qemu_log_mask(LOG_UNIMP, "%s: the PPC64 MMU is unsupported\n",
+                          __func__);
         } else {
             dump_slb(env_archcpu(env));
-            break;
         }
+        break;
 #endif
     default:
         qemu_log_mask(LOG_UNIMP, "%s: unimplemented\n", __func__);
-- 
2.27.0

Re: [PATCH v3 7/7] target/ppc: replaced the TODO with LOG_UNIMP and add break for silence warnings

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2155 bytes --]

On Mon, Nov 16, 2020 at 10:48:10AM +0800, Chen Qun wrote:
> When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning:
> target/ppc/mmu_helper.c: In function ‘dump_mmu’:
> target/ppc/mmu_helper.c:1351:12: warning: this statement may fall through [-Wimplicit-fallthrough=]
>  1351 |         if (ppc64_v3_radix(env_archcpu(env))) {
>       |            ^
> target/ppc/mmu_helper.c:1358:5: note: here
>  1358 |     default:
>       |     ^~~~~~~
> 
> Use "qemu_log_mask(LOG_UNIMP**)" instead of the TODO comment.
> And add the break statement to fix it.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>

Applied to ppc-for-6.0, thanks.

> ---
> v1->v2: replace the TODO by a LOG_UNIMP call and add break statement(Base on Philippe's comments)
> 
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  target/ppc/mmu_helper.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 8972714775..3bb50546f8 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -1349,11 +1349,12 @@ void dump_mmu(CPUPPCState *env)
>          break;
>      case POWERPC_MMU_3_00:
>          if (ppc64_v3_radix(env_archcpu(env))) {
> -            /* TODO - Unsupported */
> +            qemu_log_mask(LOG_UNIMP, "%s: the PPC64 MMU is unsupported\n",
> +                          __func__);
>          } else {
>              dump_slb(env_archcpu(env));
> -            break;
>          }
> +        break;
>  #endif
>      default:
>          qemu_log_mask(LOG_UNIMP, "%s: unimplemented\n", __func__);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v3 0/7] silence the compiler warnings

[Chenqun (kuhn)]

Kindly ping!

Hi all,
  Patch 1 to Patch 5 are not in the queue.  Could someone pick them up？

Patch1~Patch5:
  target/i386: silence the compiler warnings in gen_shiftd_rm_T1
  hw/intc/arm_gicv3_kvm: silence the compiler warnings
  accel/tcg/user-exec: silence the compiler warnings
  target/sparc/translate: silence the compiler warnings
  target/sparc/win_helper: silence the compiler warnings

Thanks,
Chen Qun

> -----Original Message-----
> From: Chenqun (kuhn)
> Sent: Monday, November 16, 2020 10:48 AM
> To: qemu-devel@nongnu.org; qemu-trivial@nongnu.org
> Cc: Zhanghailiang <zhang.zhanghailiang@huawei.com>; ganqixin
> <ganqixin@huawei.com>; Chenqun (kuhn) <kuhn.chenqun@huawei.com>
> Subject: [PATCH v3 0/7] silence the compiler warnings
> 
> Hi folks,
>   This series fix some "fall through" warnings reported by GCC_9.3. They've
> been reviewed for a long time. Some of these patchs may be important for
> QEMU 5.2.
> Such as the Patch6 miss a break statement. Others only add "fall through"
> comments and may not have a negative impact for QEMU 5.2.
> 
> 
> Thanks,
> Chen Qun
> 
> 
> Since v2:
> - Patch3：Add Richard Henderson、Philippe Mathieu-Daudé and Thomas Huth
> reviewed tag.
> - Patch4: Laurent pull it to master, remove it.
> - Patch6->Patch5: Add Richard Henderson and Philippe Mathieu-Daudé
> reviewed tag.
> - Patch7->Patch6: Add Thomas Huth reviewed tag and David Gibson acked tag.
> - Patch8->Patch7: Tweak LOG_UNIMP message base on Thomas Huth
> comment;
>   Add Philippe Mathieu-Daudé and Thomas Huth reviewed tag; Add David
> Gibson acked tag.
> 
> Since v1:
> - Patch1: Add comments to explain the two case of fall through.
>   Addressed Richard Henderson and Thomas Huth review comment.
> - Patch2: Addressed Peter Maydell review comment.
> - Patch3: Add QEMU_NORETURN to cpu_exit_tb_from_sighandler() function to
>   avoid the compiler warnings.
> - Patch4: Addressed Thomas Huth review comment.
> - Patch5: Addressed Artyom Tarasenko and Philippe Mathieu-Daudé review
>   comment.
> - Patch6: Combine the /* fall through */ to the preceding comments.
>   Addressed  Artyom Tarasenko review comment.
> - Patch7: Add a "break" statement here instead of /* fall through */
>   comments.
> - Patch8: Replace the TODO by a LOG_UNIMP call and add break statement
> - Patch9: Discard this patch since a patch already exists for fix this
> 
> issue(https://lore.kernel.org/qemu-devel/20200711154242.41222-1-ysato@us
> ers)
> 
> 
> Chen Qun (7):
>   target/i386: silence the compiler warnings in gen_shiftd_rm_T1
>   hw/intc/arm_gicv3_kvm: silence the compiler warnings
>   accel/tcg/user-exec: silence the compiler warnings
>   target/sparc/translate: silence the compiler warnings
>   target/sparc/win_helper: silence the compiler warnings
>   ppc: Add a missing break for PPC6xx_INPUT_TBEN
>   target/ppc: replaced the TODO with LOG_UNIMP and add break for silence
>     warnings
> 
>  accel/tcg/user-exec.c     | 3 ++-
>  hw/intc/arm_gicv3_kvm.c   | 8 ++++++++
>  hw/ppc/ppc.c              | 1 +
>  target/i386/translate.c   | 7 +++++--
>  target/ppc/mmu_helper.c   | 5 +++--
>  target/sparc/translate.c  | 2 +-
>  target/sparc/win_helper.c | 2 +-
>  7 files changed, 21 insertions(+), 7 deletions(-)
> 
> --
> 2.27.0

Re: [PATCH v3 0/7] silence the compiler warnings

[Thomas Huth]

On 11/12/2020 03.22, Chenqun (kuhn) wrote:
> Kindly ping!
> 
> Hi all,
>   Patch 1 to Patch 5 are not in the queue.  Could someone pick them up？

 Hi,

yes, I'm currently preparing another patch series that includes your
patches, which will finally turn on -Wimplicit-fallthrough for everybody.
I'll send it out once David's ppc pull request has been merged, so that I
don't have to include the ppc patches again.

 Thomas