[PATCH 1/3] kbuild: add -Wno-pointer-sign flag unconditionally

[PATCH 1/3] kbuild: add -Wno-pointer-sign flag unconditionally

[Masahiro Yamada]

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.

The -Wno-pointer-sign has been here since the pre-git era.  It is
documented in the GCC 4.6 manual, 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(-)

diff --git a/Makefile b/Makefile
index c6d1f2f..505a324 100644
--- a/Makefile
+++ b/Makefile
@@ -800,7 +800,7 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
 KBUILD_CFLAGS += $(call cc-option,-Wdeclaration-after-statement,)
 
 # disable pointer signed / unsigned warnings in gcc 4.0
-KBUILD_CFLAGS += $(call cc-disable-warning, pointer-sign)
+KBUILD_CFLAGS += -Wno-pointer-sign
 
 # disable stringop warnings in gcc 8+
 KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation)
-- 
2.7.4

[PATCH 2/3] kbuild: add -Wdeclaration-after-statement flag unconditionally

[Masahiro Yamada]

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 has been here since the pre-git era.  It is documented in
the GCC 4.6 manual, and it is recognized by Clang and ICC as well.

Let's rip off the cc-option 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 505a324..8a23fee 100644
--- a/Makefile
+++ b/Makefile
@@ -797,7 +797,7 @@ endif
 NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
 
 # warn about C99 declaration after statement
-KBUILD_CFLAGS += $(call cc-option,-Wdeclaration-after-statement,)
+KBUILD_CFLAGS += -Wdeclaration-after-statement
 
 # disable pointer signed / unsigned warnings in gcc 4.0
 KBUILD_CFLAGS += -Wno-pointer-sign
-- 
2.7.4

[PATCH 3/3] kbuild: add -Wno-unused-but-set-variable compiler flag unconditionally

[Masahiro Yamada]

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(-)

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

Re: [PATCH 3/3] kbuild: add -Wno-unused-but-set-variable compiler flag unconditionally

[Nick Desaulniers]

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

Re: [PATCH 2/3] kbuild: add -Wdeclaration-after-statement flag unconditionally

[Nick Desaulniers]

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 has been here since the pre-git era.  It is documented in
> the GCC 4.6 manual, and it is recognized by Clang and ICC as well.
>
> Let's rip off the cc-option 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 505a324..8a23fee 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -797,7 +797,7 @@ endif
>  NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
>
>  # warn about C99 declaration after statement
> -KBUILD_CFLAGS += $(call cc-option,-Wdeclaration-after-statement,)
> +KBUILD_CFLAGS += -Wdeclaration-after-statement

Tested-by: Nick Desaulniers <ndesaulniers@google.com>
https://godbolt.org/z/TP5722

>
>  # disable pointer signed / unsigned warnings in gcc 4.0
>  KBUILD_CFLAGS += -Wno-pointer-sign
> --
> 2.7.4
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 1/3] kbuild: add -Wno-pointer-sign flag unconditionally

[Nick Desaulniers]

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.
>
> The -Wno-pointer-sign has been here since the pre-git era.  It is
> documented in the GCC 4.6 manual, 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(-)
>
> diff --git a/Makefile b/Makefile
> index c6d1f2f..505a324 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -800,7 +800,7 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
>  KBUILD_CFLAGS += $(call cc-option,-Wdeclaration-after-statement,)
>
>  # disable pointer signed / unsigned warnings in gcc 4.0
> -KBUILD_CFLAGS += $(call cc-disable-warning, pointer-sign)
> +KBUILD_CFLAGS += -Wno-pointer-sign

Tested-by: Nick Desaulniers <ndesaulniers@google.com>
https://godbolt.org/z/7vpHIf

>
>  # disable stringop warnings in gcc 8+
>  KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation)
> --
> 2.7.4
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 3/3] kbuild: add -Wno-unused-but-set-variable compiler flag unconditionally

[Masahiro Yamada]

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

Re: [PATCH 3/3] kbuild: add -Wno-unused-but-set-variable compiler flag unconditionally

[Nick Desaulniers]

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

Re: [PATCH 3/3] kbuild: add -Wno-unused-but-set-variable compiler flag unconditionally

[Masahiro Yamada]

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

Re: [PATCH 3/3] kbuild: add -Wno-unused-but-set-variable compiler flag unconditionally

[Nick Desaulniers]

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

Re: [PATCH 1/3] kbuild: add -Wno-pointer-sign flag unconditionally

[Masahiro Yamada]

On Mon, Oct 1, 2018 at 6:45 PM 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.
>
> The -Wno-pointer-sign has been here since the pre-git era.  It is
> documented in the GCC 4.6 manual, 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>
> ---

Applied to linux-kbuild.


>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index c6d1f2f..505a324 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -800,7 +800,7 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
>  KBUILD_CFLAGS += $(call cc-option,-Wdeclaration-after-statement,)
>
>  # disable pointer signed / unsigned warnings in gcc 4.0
> -KBUILD_CFLAGS += $(call cc-disable-warning, pointer-sign)
> +KBUILD_CFLAGS += -Wno-pointer-sign
>
>  # disable stringop warnings in gcc 8+
>  KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation)
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] kbuild: add -Wdeclaration-after-statement flag unconditionally

[Masahiro Yamada]

On Mon, Oct 1, 2018 at 6:45 PM 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 has been here since the pre-git era.  It is documented in
> the GCC 4.6 manual, and it is recognized by Clang and ICC as well.
>
> Let's rip off the cc-option switch, and see if somebody complains
> about it.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---

Applied to linux-kbuild.


>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 505a324..8a23fee 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -797,7 +797,7 @@ endif
>  NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
>
>  # warn about C99 declaration after statement
> -KBUILD_CFLAGS += $(call cc-option,-Wdeclaration-after-statement,)
> +KBUILD_CFLAGS += -Wdeclaration-after-statement
>
>  # disable pointer signed / unsigned warnings in gcc 4.0
>  KBUILD_CFLAGS += -Wno-pointer-sign
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada