[PATCH] kbuild: add most of Clang-specific flags unconditionally

[Masahiro Yamada]

We do not support old Clang versions. Upgrade your clang version
if any of these flags is unsupported.

Let's add flags within ifdef CONFIG_CC_IS_CLANG unconditionally,
except -fcatch-undefined-behavior.

Signed-off-by: Masahiro Yamada <yamada....@socionext.com>
---

Makefile | 10 +++++-----
scripts/Makefile.extrawarn | 12 ++++++------
2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/Makefile b/Makefile
index bd7ae11947cb..c71ffb6f55b5 100644
--- a/Makefile
+++ b/Makefile
@@ -731,15 +731,15 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG) := -fstack-protector-strong
KBUILD_CFLAGS += $(stackp-flags-y)

ifdef CONFIG_CC_IS_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_CPPFLAGS += -Qunused-arguments
+KBUILD_CFLAGS += -Wno-format-invalid-specifier
+KBUILD_CFLAGS += -Wno-gnu
# Quiet clang warning: comparison of unsigned expression < 0 is always false
-KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
+KBUILD_CFLAGS += -Wno-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 += -mno-global-merge
KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
else

diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 768306add591..523c4cafe2dc 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -66,11 +66,11 @@ KBUILD_CFLAGS += $(warning)
else

ifdef CONFIG_CC_IS_CLANG
-KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides)
-KBUILD_CFLAGS += $(call cc-disable-warning, unused-value)
-KBUILD_CFLAGS += $(call cc-disable-warning, format)
-KBUILD_CFLAGS += $(call cc-disable-warning, sign-compare)
-KBUILD_CFLAGS += $(call cc-disable-warning, format-zero-length)
-KBUILD_CFLAGS += $(call cc-disable-warning, uninitialized)
+KBUILD_CFLAGS += -Wno-initializer-overrides
+KBUILD_CFLAGS += -Wno-unused-value
+KBUILD_CFLAGS += -Wno-format
+KBUILD_CFLAGS += -Wno-sign-compare
+KBUILD_CFLAGS += -Wno-format-zero-length
+KBUILD_CFLAGS += -Wno-uninitialized
endif
endif
--
2.17.1

[Sedat Dilek]

On Thu, May 9, 2019 at 8:45 AM Masahiro Yamada
<yamada....@socionext.com> wrote:
>
> We do not support old Clang versions. Upgrade your clang version
> if any of these flags is unsupported.
>
> Let's add flags within ifdef CONFIG_CC_IS_CLANG unconditionally,
> except -fcatch-undefined-behavior.
>
> Signed-off-by: Masahiro Yamada <yamada....@socionext.com>

Looks good to me.

Reviewed-by: Sedat Dilek <sedat...@gmail.com>

Just as sidenote:
I experimented with a snapshot version of clang-9 and lld-9 and could
build, link and boot on bare-metal with '-mglobal-merge' on
Debian/buster AMD64.
But forgot to document in [1].

[1] https://github.com/ClangBuiltLinux/linux/issues/431

> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-li...@googlegroups.com.
> To post to this group, send email to clang-bu...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20190509064455.1173-1-yamada.masahiro%40socionext.com.
> For more options, visit https://groups.google.com/d/optout.

[Nathan Chancellor]

I think we should just remove this, I'm fairly confident the kernel
can't be reliably compiled with anything earlier than Clang 4 (Pixel 2
was shipped with it but had some hacks, this commit is from Clang 3.5):

https://github.com/llvm/llvm-project/commit/cb3f812b6b9fab8f3b41414f24e90222170417b4

Otherwise:

Reviewed-by: Nathan Chancellor <natecha...@gmail.com>

[Masahiro Yamada]

Hi.

Oh, I did not know this was deprecated.

Could you send a patch to remove it
with filling the commit log ?


Thanks.



--
Best Regards
Masahiro Yamada

[Nathan Chancellor]

Sure!

>
>
> Thanks.
>
>
>
> --
> Best Regards
> Masahiro Yamada
>

> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-li...@googlegroups.com.
> To post to this group, send email to clang-bu...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAK7LNASpsid7_sh4rdRNSTwZ1YtW_%2BuH2eoarJNNUttntQZ-kg%40mail.gmail.com.

[Nathan Chancellor]

This is no longer a valid option in clang, it was removed in 3.5, which
we don't support.

https://github.com/llvm/llvm-project/commit/cb3f812b6b9fab8f3b41414f24e90222170417b4

Signed-off-by: Nathan Chancellor <natecha...@gmail.com>
---

Let me know if you want this incremental to your patch. I figured it
made more sense to remove this then do the cc-option/cc-disable-warning
removal because it will simplify the commit message.

Makefile | 1 -
1 file changed, 1 deletion(-)

diff --git a/Makefile b/Makefile
index d24f5a8009ee..e4788eb2c9b9 100644
--- a/Makefile
+++ b/Makefile
@@ -740,7 +740,6 @@ KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)

# 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.
--
2.21.0

[Masahiro Yamada]

On Thu, May 9, 2019 at 8:49 PM Nathan Chancellor
<natecha...@gmail.com> wrote:
>
> This is no longer a valid option in clang, it was removed in 3.5, which
> we don't support.
>
> https://github.com/llvm/llvm-project/commit/cb3f812b6b9fab8f3b41414f24e90222170417b4
>
> Signed-off-by: Nathan Chancellor <natecha...@gmail.com>
> ---
>
> Let me know if you want this incremental to your patch.

No problem. I will rebase mine on top of this.

> I figured it
> made more sense to remove this then do the cc-option/cc-disable-warning
> removal because it will simplify the commit message.

Yes, I agree!

[Sedat Dilek]

On Thu, May 9, 2019 at 1:49 PM Nathan Chancellor
<natecha...@gmail.com> wrote:
>
> This is no longer a valid option in clang, it was removed in 3.5, which
> we don't support.
>
> https://github.com/llvm/llvm-project/commit/cb3f812b6b9fab8f3b41414f24e90222170417b4
>

Cool.

Can you test with -mglobal-merge (inverted -mno-global-merge) which is
default for Clang?
I could build, link and boot with my llvm-toolchain (incl. lld-9) on x86-64.

Maybe this is also no more needed?

Thanks.

- Sedat -

> Signed-off-by: Nathan Chancellor <natecha...@gmail.com>
> ---
>
> Let me know if you want this incremental to your patch. I figured it
> made more sense to remove this then do the cc-option/cc-disable-warning
> removal because it will simplify the commit message.
>
> Makefile | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index d24f5a8009ee..e4788eb2c9b9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -740,7 +740,6 @@ KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
> # 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.
> --
> 2.21.0
>

> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-li...@googlegroups.com.
> To post to this group, send email to clang-bu...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20190509114824.25866-1-natechancellor%40gmail.com.

[Nick Desaulniers]

> On Thu, May 9, 2019 at 1:49 PM Nathan Chancellor
> <natecha...@gmail.com> wrote:
> >
> > This is no longer a valid option in clang, it was removed in 3.5, which
> > we don't support.
> >
> > https://github.com/llvm/llvm-project/commit/cb3f812b6b9fab8f3b41414f24e90222170417b4

lol, good catch.
Reviewed-by: Nick Desaulniers <ndesau...@google.com>

> >
>
> Cool.
>
> Can you test with -mglobal-merge (inverted -mno-global-merge) which is
> default for Clang?

Looks like these were added in r234668 in 2015, which would've been
Clang 3.6.2, according to:
http://releases.llvm.org/

I'd be fine with dropping the option check there.

There's a few here (-Qunused-arguments) that don't need cc-option
checks. I recommend someone audit all of them, rather than send a
bunch of little patches.
--
Thanks,
~Nick Desaulniers

[Nick Desaulniers]

Yeah, clang-4 is usually what I test these with, as that's the
earliest known version of Clang that I ever had success with.
Hopefully a lot of things will stabilize for the clang-9 release.

Anyways, I checked these all w/ godbolt w/ clang-4.
Tested-by: Nick Desaulniers <ndesau...@google.com>

--
Thanks,
~Nick Desaulniers

[Masahiro Yamada]

On Thu, May 9, 2019 at 4:06 PM Sedat Dilek <sedat...@gmail.com> wrote:
>
> On Thu, May 9, 2019 at 8:45 AM Masahiro Yamada
> <yamada....@socionext.com> wrote:
> >
> > We do not support old Clang versions. Upgrade your clang version
> > if any of these flags is unsupported.
> >
> > Let's add flags within ifdef CONFIG_CC_IS_CLANG unconditionally,
> > except -fcatch-undefined-behavior.
> >
> > Signed-off-by: Masahiro Yamada <yamada....@socionext.com>
>
> Looks good to me.
>
> Reviewed-by: Sedat Dilek <sedat...@gmail.com>
>
> Just as sidenote:
> I experimented with a snapshot version of clang-9 and lld-9 and could
> build, link and boot on bare-metal with '-mglobal-merge' on
> Debian/buster AMD64.

The comment says

# 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

So, it seems it is just a matter of modpost,
but I am not sure enough.

This flag has been here since the initial support.
(61163efae02040f66a95c8ed17f4407951ba58fa)


Perhaps, we should review clang flags one by one again?

> But forgot to document in [1].
>
> [1] https://github.com/ClangBuiltLinux/linux/issues/431

[Nick Desaulniers]

On Fri, May 10, 2019 at 6:54 AM Masahiro Yamada

Yes, it's always good to re-evaluate if something is just cruft and
can be removed.

+Behan

I don't quite understand the comment about _MergedGlobals, Behan, do
happen to have more context?
--
Thanks,
~Nick Desaulniers

[Masahiro Yamada]

On Thu, May 9, 2019 at 8:49 PM Nathan Chancellor
<natecha...@gmail.com> wrote:
>
> This is no longer a valid option in clang, it was removed in 3.5, which
> we don't support.
>
> https://github.com/llvm/llvm-project/commit/cb3f812b6b9fab8f3b41414f24e90222170417b4
>
> Signed-off-by: Nathan Chancellor <natecha...@gmail.com>
> ---
>
> Let me know if you want this incremental to your patch. I figured it
> made more sense to remove this then do the cc-option/cc-disable-warning
> removal because it will simplify the commit message.

Applied to linux-kbuild.
Thanks.

> Makefile | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index d24f5a8009ee..e4788eb2c9b9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -740,7 +740,6 @@ KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
> # 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.
> --
> 2.21.0
>