| Message ID | 20190719112315.14432-1-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,PATCH-for-4.1] target/i386: Correct misplaced break statement in gen_shiftd_rm_T1() | expand |
On 19/07/19 13:23, Philippe Mathieu-Daudé wrote: > Reported by GCC9 when building with CFLAG -Wimplicit-fallthrough=2: > > CC target/i386/translate.o > target/i386/translate.c: In function ‘gen_shiftd_rm_T1’: > target/i386/translate.c:1785:12: error: this statement may fall through [-Werror=implicit-fallthrough=] > 1785 | if (is_right) { > | ^ > target/i386/translate.c:1810:5: note: here > 1810 | default: > | ^~~~~~~ > cc1: all warnings being treated as errors > > Fixes: f437d0a3c24 > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > target/i386/translate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/i386/translate.c b/target/i386/translate.c > index 03150a86e2..4b2b5937ca 100644 > --- a/target/i386/translate.c > +++ b/target/i386/translate.c > @@ -1805,8 +1805,8 @@ static void gen_shiftd_rm_T1(DisasContext *s, TCGMemOp ot, int op1, > tcg_gen_shri_i64(s->tmp0, s->tmp0, 32); > tcg_gen_shri_i64(s->T0, s->T0, 32); > } > - break; > #endif > + break; > default: > tcg_gen_subi_tl(s->tmp0, count, 1); > if (is_right) { > I haven't looked closely at the code but I would guess that the fallthrough is intended, because the default label has an "ot == MO_16" condition. It certainly needs more comments... :( Paolo
On Fri, 19 Jul 2019 at 12:45, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 19/07/19 13:23, Philippe Mathieu-Daudé wrote: > > Reported by GCC9 when building with CFLAG -Wimplicit-fallthrough=2: > > > > CC target/i386/translate.o > > target/i386/translate.c: In function ‘gen_shiftd_rm_T1’: > > target/i386/translate.c:1785:12: error: this statement may fall through [-Werror=implicit-fallthrough=] > > 1785 | if (is_right) { > > | ^ > > target/i386/translate.c:1810:5: note: here > > 1810 | default: > > | ^~~~~~~ > > cc1: all warnings being treated as errors > > > > Fixes: f437d0a3c24 > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > --- > > target/i386/translate.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/target/i386/translate.c b/target/i386/translate.c > > index 03150a86e2..4b2b5937ca 100644 > > --- a/target/i386/translate.c > > +++ b/target/i386/translate.c > > @@ -1805,8 +1805,8 @@ static void gen_shiftd_rm_T1(DisasContext *s, TCGMemOp ot, int op1, > > tcg_gen_shri_i64(s->tmp0, s->tmp0, 32); > > tcg_gen_shri_i64(s->T0, s->T0, 32); > > } > > - break; > > #endif > > + break; > > default: > > tcg_gen_subi_tl(s->tmp0, count, 1); > > if (is_right) { > > > > I haven't looked closely at the code but I would guess that the > fallthrough is intended, because the default label has an "ot == MO_16" > condition. Yeah, this code is really weird -- if TARGET_X86_64 then MO_16 falls through into MO_32, but if only i386 then MO_16 falls through into the default case ?!? thanks -- PMM
On 19/07/19 13:51, Peter Maydell wrote: >> I haven't looked closely at the code but I would guess that the >> fallthrough is intended, because the default label has an "ot == MO_16" >> condition. > Yeah, this code is really weird -- if TARGET_X86_64 then > MO_16 falls through into MO_32, but if only i386 then > MO_16 falls through into the default case ?!? Yes, and in either case MO_16 falls through into the 32-bit code. However, the i386 32-bit version and the x86-64 64-bit version are unified into a single piece of code for TARGET_LONG_BITS-bit operands. Almost, because you still need that ugly special case for MO_16 in the default label. Paolo
diff --git a/target/i386/translate.c b/target/i386/translate.c index 03150a86e2..4b2b5937ca 100644 --- a/target/i386/translate.c +++ b/target/i386/translate.c @@ -1805,8 +1805,8 @@ static void gen_shiftd_rm_T1(DisasContext *s, TCGMemOp ot, int op1, tcg_gen_shri_i64(s->tmp0, s->tmp0, 32); tcg_gen_shri_i64(s->T0, s->T0, 32); } - break; #endif + break; default: tcg_gen_subi_tl(s->tmp0, count, 1); if (is_right) {
Reported by GCC9 when building with CFLAG -Wimplicit-fallthrough=2: CC target/i386/translate.o target/i386/translate.c: In function ‘gen_shiftd_rm_T1’: target/i386/translate.c:1785:12: error: this statement may fall through [-Werror=implicit-fallthrough=] 1785 | if (is_right) { | ^ target/i386/translate.c:1810:5: note: here 1810 | default: | ^~~~~~~ cc1: all warnings being treated as errors Fixes: f437d0a3c24 Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- target/i386/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)