| Message ID | 1538387078-21892-3-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] kbuild: add -Wno-pointer-sign flag unconditionally | expand |
On Mon, Oct 1, 2018 at 2:45 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > We have raised the compiler requirement from time to time. > With commit cafa0010cd51 ("Raise the minimum required gcc version > to 4.6"), the minimum for GCC is 4.6 now. > > This flag was added by GCC 4.6, and it is recognized by Clang and > ICC as well. This doesn't seem to be the case for Clang: https://godbolt.org/z/qesF5o Nacked-by: Nick Desaulniers <ndesaulniers@google.com> > > Let's rip off the cc-disable-warning switch, and see if somebody > complains about it. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 8a23fee..2627266 100644 > --- a/Makefile > +++ b/Makefile > @@ -716,7 +716,7 @@ else > > # These warnings generated too much noise in a regular build. > # Use make W=1 to enable them (see scripts/Makefile.extrawarn) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > +KBUILD_CFLAGS += -Wno-unused-but-set-variable > endif > > KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > -- > 2.7.4 > -- Thanks, ~Nick Desaulniers
Hi Nick, 2018年10月2日(火) 2:18 Nick Desaulniers <ndesaulniers@google.com>: > > On Mon, Oct 1, 2018 at 2:45 AM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > We have raised the compiler requirement from time to time. > > With commit cafa0010cd51 ("Raise the minimum required gcc version > > to 4.6"), the minimum for GCC is 4.6 now. > > > > This flag was added by GCC 4.6, and it is recognized by Clang and > > ICC as well. > > This doesn't seem to be the case for Clang: > https://godbolt.org/z/qesF5o > > Nacked-by: Nick Desaulniers <ndesaulniers@google.com> Hmm, I tested this patch with pre-built Clang 6.0.1 / 7.0.0 downloaded from http://releases.llvm.org/download.html Was this option dropped by clang 8 ? > > > > > Let's rip off the cc-disable-warning switch, and see if somebody > > complains about it. > > > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > --- > > > > Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Makefile b/Makefile > > index 8a23fee..2627266 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -716,7 +716,7 @@ else > > > > # These warnings generated too much noise in a regular build. > > # Use make W=1 to enable them (see scripts/Makefile.extrawarn) > > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > > +KBUILD_CFLAGS += -Wno-unused-but-set-variable > > endif > > > > KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > > -- > > 2.7.4 > > > > > -- > Thanks, > ~Nick Desaulniers -- Best Regards Masahiro Yamada
On Mon, Oct 1, 2018 at 12:02 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > Hi Nick, > > 2018年10月2日(火) 2:18 Nick Desaulniers <ndesaulniers@google.com>: > > > > On Mon, Oct 1, 2018 at 2:45 AM Masahiro Yamada > > <yamada.masahiro@socionext.com> wrote: > > > > > > We have raised the compiler requirement from time to time. > > > With commit cafa0010cd51 ("Raise the minimum required gcc version > > > to 4.6"), the minimum for GCC is 4.6 now. > > > > > > This flag was added by GCC 4.6, and it is recognized by Clang and > > > ICC as well. > > > > This doesn't seem to be the case for Clang: > > https://godbolt.org/z/qesF5o > > > > Nacked-by: Nick Desaulniers <ndesaulniers@google.com> > > Hmm, I tested this patch with pre-built Clang 6.0.1 / 7.0.0 > downloaded from http://releases.llvm.org/download.html > > Was this option dropped by clang 8 ? From the Godbolt link above, it seems all versions of Clang do not recognize this flag. It doesn't look like the kernel sets -Wno-unknown-warning-option to prevent this warning. Can you please triple check that compiling with Clang and this patch causes no warnings? I suspect something might be amiss then if this patch doesn't produce warnings with Clang, since on the smaller Godbolt example it does. > > > > > > > > > > Let's rip off the cc-disable-warning switch, and see if somebody > > > complains about it. > > > > > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > > --- > > > > > > Makefile | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/Makefile b/Makefile > > > index 8a23fee..2627266 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -716,7 +716,7 @@ else > > > > > > # These warnings generated too much noise in a regular build. > > > # Use make W=1 to enable them (see scripts/Makefile.extrawarn) > > > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > > > +KBUILD_CFLAGS += -Wno-unused-but-set-variable > > > endif > > > > > > KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > > > -- > > > 2.7.4 > > > > > > > > > -- > > Thanks, > > ~Nick Desaulniers > > > > -- > Best Regards > Masahiro Yamada -- Thanks, ~Nick Desaulniers
2018年10月2日(火) 4:58 Nick Desaulniers <ndesaulniers@google.com>: > > On Mon, Oct 1, 2018 at 12:02 PM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > Hi Nick, > > > > 2018年10月2日(火) 2:18 Nick Desaulniers <ndesaulniers@google.com>: > > > > > > On Mon, Oct 1, 2018 at 2:45 AM Masahiro Yamada > > > <yamada.masahiro@socionext.com> wrote: > > > > > > > > We have raised the compiler requirement from time to time. > > > > With commit cafa0010cd51 ("Raise the minimum required gcc version > > > > to 4.6"), the minimum for GCC is 4.6 now. > > > > > > > > This flag was added by GCC 4.6, and it is recognized by Clang and > > > > ICC as well. > > > > > > This doesn't seem to be the case for Clang: > > > https://godbolt.org/z/qesF5o > > > > > > Nacked-by: Nick Desaulniers <ndesaulniers@google.com> > > > > Hmm, I tested this patch with pre-built Clang 6.0.1 / 7.0.0 > > downloaded from http://releases.llvm.org/download.html > > > > Was this option dropped by clang 8 ? > > From the Godbolt link above, it seems all versions of Clang do not > recognize this flag. It doesn't look like the kernel sets > -Wno-unknown-warning-option to prevent this warning. Can you please > triple check that compiling with Clang and this patch causes no > warnings? I suspect something might be amiss then if this patch > doesn't produce warnings with Clang, since on the smaller Godbolt > example it does. I got it. Clang does NOT recognize -Wno-unused-but-set-variable. But, the code I changed is not parsed for clang. That is why I did not see any problem with this patch. See the following code. ifeq ($(cc-name),clang) KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) KBUILD_CFLAGS += $(call cc-disable-warning, gnu) KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) # Quiet clang warning: comparison of unsigned expression < 0 is always false KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) # CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the # source of a reference will be _MergedGlobals and not on of the whitelisted names. # See modpost pattern 2 KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) else # These warnings generated too much noise in a regular build. # Use make W=1 to enable them (see scripts/Makefile.extrawarn) KBUILD_CFLAGS += -Wno-unused-but-set-variable endif -- Best Regards Masahiro Yamada
On Mon, Oct 1, 2018 at 3:24 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > 2018年10月2日(火) 4:58 Nick Desaulniers <ndesaulniers@google.com>: > > > > On Mon, Oct 1, 2018 at 12:02 PM Masahiro Yamada > > <yamada.masahiro@socionext.com> wrote: > > > > > > Hi Nick, > > > > > > 2018年10月2日(火) 2:18 Nick Desaulniers <ndesaulniers@google.com>: > > > > > > > > On Mon, Oct 1, 2018 at 2:45 AM Masahiro Yamada > > > > <yamada.masahiro@socionext.com> wrote: > > > > > > > > > > We have raised the compiler requirement from time to time. > > > > > With commit cafa0010cd51 ("Raise the minimum required gcc version > > > > > to 4.6"), the minimum for GCC is 4.6 now. > > > > > > > > > > This flag was added by GCC 4.6, and it is recognized by Clang and > > > > > ICC as well. > > > > > > > > This doesn't seem to be the case for Clang: > > > > https://godbolt.org/z/qesF5o > > > > > > > > Nacked-by: Nick Desaulniers <ndesaulniers@google.com> > > > > > > Hmm, I tested this patch with pre-built Clang 6.0.1 / 7.0.0 > > > downloaded from http://releases.llvm.org/download.html > > > > > > Was this option dropped by clang 8 ? > > > > From the Godbolt link above, it seems all versions of Clang do not > > recognize this flag. It doesn't look like the kernel sets > > -Wno-unknown-warning-option to prevent this warning. Can you please > > triple check that compiling with Clang and this patch causes no > > warnings? I suspect something might be amiss then if this patch > > doesn't produce warnings with Clang, since on the smaller Godbolt > > example it does. > > > I got it. > > Clang does NOT recognize -Wno-unused-but-set-variable. > > But, the code I changed is not parsed for clang. > That is why I did not see any problem with this patch. > > See the following code. > > > > ifeq ($(cc-name),clang) > KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) > KBUILD_CFLAGS += $(call cc-disable-warning, gnu) > KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) > # Quiet clang warning: comparison of unsigned expression < 0 is always false > KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) > # CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the > # source of a reference will be _MergedGlobals and not on of the > whitelisted names. > # See modpost pattern 2 > KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) > KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) > else > > # These warnings generated too much noise in a regular build. > # Use make W=1 to enable them (see scripts/Makefile.extrawarn) > KBUILD_CFLAGS += -Wno-unused-but-set-variable Ah! I see, it's in the else part of an if clang check. Ok, in that case I think the commit message can be cleaned up with this detail, then I'd be happy to sign off on it. Sorry I didn't notice that before. > endif > > > > > > -- > Best Regards > Masahiro Yamada -- Thanks, ~Nick Desaulniers
diff --git a/Makefile b/Makefile index 8a23fee..2627266 100644 --- a/Makefile +++ b/Makefile @@ -716,7 +716,7 @@ else # These warnings generated too much noise in a regular build. # Use make W=1 to enable them (see scripts/Makefile.extrawarn) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) +KBUILD_CFLAGS += -Wno-unused-but-set-variable endif KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable)
We have raised the compiler requirement from time to time. With commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6"), the minimum for GCC is 4.6 now. This flag was added by GCC 4.6, and it is recognized by Clang and ICC as well. Let's rip off the cc-disable-warning switch, and see if somebody complains about it. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.7.4