To be able to compile this file with -Werror=implicit-fallthrough,
we need to add some fallthrough annotations to the case statements
that might fall through. Unfortunately, the typical "/* fallthrough */"
comments do not work here as expected since some case labels are
wrapped in macros and the compiler fails to match the comments in
this case. But using __attribute__((fallthrough)) seems to work fine,
so let's use that instead.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tcg/optimize.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index 220f4601d5..c2768c9770 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -26,6 +26,12 @@
#include "qemu/osdep.h"
#include "tcg/tcg-op.h"
+#if __has_attribute(fallthrough)
+# define fallthrough() __attribute__((fallthrough))
+#else
+# define fallthrough() do {} while (0)
+#endif
+
#define CASE_OP_32_64(x) \
glue(glue(case INDEX_op_, x), _i32): \
glue(glue(case INDEX_op_, x), _i64)
@@ -855,6 +861,7 @@ void tcg_optimize(TCGContext *s)
if ((arg_info(op->args[1])->mask & 0x80) != 0) {
break;
}
+ fallthrough();
CASE_OP_32_64(ext8u):
mask = 0xff;
goto and_const;
@@ -862,6 +869,7 @@ void tcg_optimize(TCGContext *s)
if ((arg_info(op->args[1])->mask & 0x8000) != 0) {
break;
}
+ fallthrough();
CASE_OP_32_64(ext16u):
mask = 0xffff;
goto and_const;
@@ -869,6 +877,7 @@ void tcg_optimize(TCGContext *s)
if ((arg_info(op->args[1])->mask & 0x80000000) != 0) {
break;
}
+ fallthrough();
case INDEX_op_ext32u_i64:
mask = 0xffffffffU;
goto and_const;
@@ -886,6 +895,7 @@ void tcg_optimize(TCGContext *s)
if ((arg_info(op->args[1])->mask & 0x80000000) != 0) {
break;
}
+ fallthrough();
case INDEX_op_extu_i32_i64:
/* We do not compute affected as it is a size changing op. */
mask = (uint32_t)arg_info(op->args[1])->mask;
--
2.18.2
On 10/29/20 1:28 PM, Thomas Huth wrote: > To be able to compile this file with -Werror=implicit-fallthrough, > we need to add some fallthrough annotations to the case statements > that might fall through. Unfortunately, the typical "/* fallthrough */" > comments do not work here as expected since some case labels are > wrapped in macros and the compiler fails to match the comments in > this case. But using __attribute__((fallthrough)) seems to work fine, > so let's use that instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > tcg/optimize.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/tcg/optimize.c b/tcg/optimize.c > index 220f4601d5..c2768c9770 100644 > --- a/tcg/optimize.c > +++ b/tcg/optimize.c > @@ -26,6 +26,12 @@ > #include "qemu/osdep.h" > #include "tcg/tcg-op.h" > > +#if __has_attribute(fallthrough) > +# define fallthrough() __attribute__((fallthrough)) > +#else > +# define fallthrough() do {} while (0) > +#endif This looks something we can reuse, what about adding it as QEMU_FALLTHROUGH in "qemu/compiler.h"? > + > #define CASE_OP_32_64(x) \ > glue(glue(case INDEX_op_, x), _i32): \ > glue(glue(case INDEX_op_, x), _i64) > @@ -855,6 +861,7 @@ void tcg_optimize(TCGContext *s) > if ((arg_info(op->args[1])->mask & 0x80) != 0) { > break; > } > + fallthrough(); > CASE_OP_32_64(ext8u): > mask = 0xff; > goto and_const; > @@ -862,6 +869,7 @@ void tcg_optimize(TCGContext *s) > if ((arg_info(op->args[1])->mask & 0x8000) != 0) { > break; > } > + fallthrough(); > CASE_OP_32_64(ext16u): > mask = 0xffff; > goto and_const; > @@ -869,6 +877,7 @@ void tcg_optimize(TCGContext *s) > if ((arg_info(op->args[1])->mask & 0x80000000) != 0) { > break; > } > + fallthrough(); > case INDEX_op_ext32u_i64: > mask = 0xffffffffU; > goto and_const; > @@ -886,6 +895,7 @@ void tcg_optimize(TCGContext *s) > if ((arg_info(op->args[1])->mask & 0x80000000) != 0) { > break; > } > + fallthrough(); > case INDEX_op_extu_i32_i64: > /* We do not compute affected as it is a size changing op. */ > mask = (uint32_t)arg_info(op->args[1])->mask; >
On 10/29/20 5:28 AM, Thomas Huth wrote: > To be able to compile this file with -Werror=implicit-fallthrough, > we need to add some fallthrough annotations to the case statements > that might fall through. Unfortunately, the typical "/* fallthrough */" > comments do not work here as expected since some case labels are > wrapped in macros and the compiler fails to match the comments in > this case. But using __attribute__((fallthrough)) seems to work fine, > so let's use that instead. Why would the macro matter? It expands to two case statements with nothing in between them. This sounds like a compiler bug that should be reported. r~
> -----Original Message----- > From: Richard Henderson [mailto:richard.henderson@linaro.org] > Sent: Friday, October 30, 2020 4:07 AM > To: Thomas Huth <thuth@redhat.com>; Richard Henderson <rth@twiddle.net>; > qemu-devel@nongnu.org > Cc: Chenqun (kuhn) <kuhn.chenqun@huawei.com> > Subject: Re: [PATCH] tcg/optimize: Add fallthrough annotations > > On 10/29/20 5:28 AM, Thomas Huth wrote: > > To be able to compile this file with -Werror=implicit-fallthrough, we > > need to add some fallthrough annotations to the case statements that > > might fall through. Unfortunately, the typical "/* fallthrough */" > > comments do not work here as expected since some case labels are > > wrapped in macros and the compiler fails to match the comments in this > > case. But using __attribute__((fallthrough)) seems to work fine, so > > let's use that instead. > > Why would the macro matter? It expands to two case statements with > nothing in between them. > > This sounds like a compiler bug that should be reported. > Hi all, I have queried the GCC options description about the Wimplicit-fallthrough and verified it. The value of Wimplicit-fallthrough ranges from 0 to 5. The value 0 is to ignore all warnings, which is certainly not what we need. If the value is set to 1 or 2, most fall through on the QEMU can take effect. Eg:/* FALLTHRU */、/* fallthru */、/* fall-through */、/* FALLTHOUGH */、/* fall through */、/* fallthrough */.. When the value ranges from 3 to 5, more fallthrough comments become invalid as the value increases. So, I agree with Philippe's suggestion to add a QEMU_FALLTHROUGH to unify this compiler property. Thanks, Chen Qun Additional gcc information is as follows: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html -Wimplicit-fallthrough is the same as -Wimplicit-fallthrough=3 and -Wno-implicit-fallthrough is the same as -Wimplicit-fallthrough=0. The option argument n specifies what kind of comments are accepted: -Wimplicit-fallthrough=0 disables the warning altogether. -Wimplicit-fallthrough=1 matches .* regular expression, any comment is used as fallthrough comment. -Wimplicit-fallthrough=2 case insensitively matches .*falls?[ \t-]*thr(ough|u).* regular expression. -Wimplicit-fallthrough=3 case sensitively matches one of the following regular expressions: -fallthrough @fallthrough@ lint -fallthrough[ \t]* [ \t.!]*(ELSE,? |INTENTIONAL(LY)? )? FALL(S | |-)?THR(OUGH|U)[ \t.!]*(-[^\n\r]*)? [ \t.!]*(Else,? |Intentional(ly)? )? Fall((s | |-)[Tt]|t)hr(ough|u)[ \t.!]*(-[^\n\r]*)? [ \t.!]*([Ee]lse,? |[Ii]ntentional(ly)? )? fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)? -Wimplicit-fallthrough=4 case sensitively matches one of the following regular expressions: -fallthrough @fallthrough@ lint -fallthrough[ \t]* [ \t]*FALLTHR(OUGH|U)[ \t]* -Wimplicit-fallthrough=5 doesn’t recognize any comments as fallthrough comments, only attributes disable the warning.
© 2016 - 2024 Red Hat, Inc.