* [PATCH] Makefile: remove stale cc-option checks
@ 2021-08-10 20:42 Nick Desaulniers
2021-08-10 23:01 ` Nathan Chancellor
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Nick Desaulniers @ 2021-08-10 20:42 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Nick Desaulniers, Michal Marek, Nathan Chancellor, Andrew Morton,
Paul E. McKenney, Peter Zijlstra, Miguel Ojeda, Tetsuo Handa,
Vitor Massaru Iha, Sedat Dilek, Daniel Latypov, linux-kbuild,
linux-kernel, clang-built-linux
cc-option, cc-option-yn, and cc-disable-warning all invoke the compiler
during build time, and can slow down the build when these checks become
stale for our supported compilers, whose minimally supported versions
increases over time. See Documentation/process/changes.rst for the
current supported minimal versions (GCC 4.9+, clang 10.0.1+). Compiler
version support for these flags may be verified on godbolt.org.
The following flags are GCC only and supported since at least GCC 4.9.
Remove cc-option and cc-disable-warning tests.
* -fno-tree-loop-im
* -Wno-maybe-uninitialized
* -fno-reorder-blocks
* -fno-ipa-cp-clone
* -fno-partial-inlining
* -femit-struct-debug-baseonly
* -fno-inline-functions-called-once
* -fconserve-stack
The following flags are supported by all supported versions of GCC and
Clang. Remove their cc-option, cc-option-yn, and cc-disable-warning tests.
* -fno-delete-null-pointer-checks
* -fno-var-tracking
* -mfentry
* -Wno-array-bounds
The following configs are made dependent on GCC, since they use GCC
specific flags.
* READABLE_ASM
* DEBUG_SECTION_MISMATCH
--param=allow-store-data-races=0 was renamed to --allow-store-data-races
in the GCC 10 release.
Also, base RETPOLINE_CFLAGS and RETPOLINE_VDSO_CFLAGS on CONFIC_CC_IS_*
then remove cc-option tests for Clang.
Link: https://github.com/ClangBuiltLinux/linux/issues/1436
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Note: It may be preferred to move the test for
-fno-inline-functions-called-once for DEBUG_SECTION_MISMATCH into
Kconfig. That one does seem relatively more reasonable to implement in
Clang.
Makefile | 55 ++++++++++++++++++++++++++---------------------
lib/Kconfig.debug | 2 ++
2 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/Makefile b/Makefile
index 027fdf2a14fe..3e3fb4affba1 100644
--- a/Makefile
+++ b/Makefile
@@ -730,9 +730,10 @@ endif # KBUILD_EXTMOD
# Defaults to vmlinux, but the arch makefile usually adds further targets
all: vmlinux
-CFLAGS_GCOV := -fprofile-arcs -ftest-coverage \
- $(call cc-option,-fno-tree-loop-im) \
- $(call cc-disable-warning,maybe-uninitialized,)
+CFLAGS_GCOV := -fprofile-arcs -ftest-coverage
+ifdef CONFIG_CC_IS_GCC
+CFLAGS_GCOV += -fno-tree-loop-im
+endif
export CFLAGS_GCOV
# The arch Makefiles can override CC_FLAGS_FTRACE. We may also append it later.
@@ -740,12 +741,14 @@ ifdef CONFIG_FUNCTION_TRACER
CC_FLAGS_FTRACE := -pg
endif
-RETPOLINE_CFLAGS_GCC := -mindirect-branch=thunk-extern -mindirect-branch-register
-RETPOLINE_VDSO_CFLAGS_GCC := -mindirect-branch=thunk-inline -mindirect-branch-register
-RETPOLINE_CFLAGS_CLANG := -mretpoline-external-thunk
-RETPOLINE_VDSO_CFLAGS_CLANG := -mretpoline
-RETPOLINE_CFLAGS := $(call cc-option,$(RETPOLINE_CFLAGS_GCC),$(call cc-option,$(RETPOLINE_CFLAGS_CLANG)))
-RETPOLINE_VDSO_CFLAGS := $(call cc-option,$(RETPOLINE_VDSO_CFLAGS_GCC),$(call cc-option,$(RETPOLINE_VDSO_CFLAGS_CLANG)))
+ifdef CONFIG_CC_IS_GCC
+RETPOLINE_CFLAGS := $(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register)
+RETPOLINE_VDSO_CFLAGS := $(call cc-option,-mindirect-branch=thunk-inline -mindirect-branch-register)
+endif
+ifdef CONFIG_CC_IS_CLANG
+RETPOLINE_CFLAGS := -mretpoline-external-thunk
+RETPOLINE_VDSO_CFLAGS := -mretpoline
+endif
export RETPOLINE_CFLAGS
export RETPOLINE_VDSO_CFLAGS
@@ -798,7 +801,7 @@ include/config/auto.conf:
endif # may-sync-config
endif # need-config
-KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)
+KBUILD_CFLAGS += -fno-delete-null-pointer-checks
KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
KBUILD_CFLAGS += $(call cc-disable-warning, format-truncation)
KBUILD_CFLAGS += $(call cc-disable-warning, format-overflow)
@@ -844,17 +847,17 @@ KBUILD_RUSTFLAGS += -Copt-level=z
endif
# Tell gcc to never replace conditional load with a non-conditional one
-KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
+ifdef CONFIG_CC_IS_GCC
+KBUILD_CFLAGS += $(call cc-option,--allow-store-data-races,--param=allow-store-data-races=0)
KBUILD_CFLAGS += $(call cc-option,-fno-allow-store-data-races)
+endif
ifdef CONFIG_READABLE_ASM
# Disable optimizations that make assembler listings hard to read.
# reorder blocks reorders the control in the function
# ipa clone creates specialized cloned functions
# partial inlining inlines only parts of functions
-KBUILD_CFLAGS += $(call cc-option,-fno-reorder-blocks,) \
- $(call cc-option,-fno-ipa-cp-clone,) \
- $(call cc-option,-fno-partial-inlining)
+KBUILD_CFLAGS += -fno-reorder-blocks -fno-ipa-cp-clone -fno-partial-inlining
endif
ifneq ($(CONFIG_FRAME_WARN),0)
@@ -959,8 +962,10 @@ DEBUG_CFLAGS += -gdwarf-$(dwarf-version-y)
endif
ifdef CONFIG_DEBUG_INFO_REDUCED
-DEBUG_CFLAGS += $(call cc-option, -femit-struct-debug-baseonly) \
- $(call cc-option,-fno-var-tracking)
+DEBUG_CFLAGS += -fno-var-tracking
+ifdef CONFIG_CC_IS_GCC
+DEBUG_CFLAGS += -femit-struct-debug-baseonly
+endif
endif
ifdef CONFIG_DEBUG_INFO_COMPRESSED
@@ -997,10 +1002,8 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
endif
endif
ifdef CONFIG_HAVE_FENTRY
- ifeq ($(call cc-option-yn, -mfentry),y)
- CC_FLAGS_FTRACE += -mfentry
- CC_FLAGS_USING += -DCC_USING_FENTRY
- endif
+ CC_FLAGS_FTRACE += -mfentry
+ CC_FLAGS_USING += -DCC_USING_FENTRY
endif
export CC_FLAGS_FTRACE
KBUILD_CFLAGS += $(CC_FLAGS_FTRACE) $(CC_FLAGS_USING)
@@ -1009,7 +1012,7 @@ endif
# We trigger additional mismatches with less inlining
ifdef CONFIG_DEBUG_SECTION_MISMATCH
-KBUILD_CFLAGS += $(call cc-option, -fno-inline-functions-called-once)
+KBUILD_CFLAGS += -fno-inline-functions-called-once
endif
ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
@@ -1088,14 +1091,16 @@ KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation)
# We'll want to enable this eventually, but it's not going away for 5.7 at least
KBUILD_CFLAGS += $(call cc-disable-warning, zero-length-bounds)
-KBUILD_CFLAGS += $(call cc-disable-warning, array-bounds)
+KBUILD_CFLAGS += -Wno-array-bounds
KBUILD_CFLAGS += $(call cc-disable-warning, stringop-overflow)
# Another good warning that we'll want to enable eventually
KBUILD_CFLAGS += $(call cc-disable-warning, restrict)
# Enabled with W=2, disabled by default as noisy
-KBUILD_CFLAGS += $(call cc-disable-warning, maybe-uninitialized)
+ifdef CONFIG_CC_IS_GCC
+KBUILD_CFLAGS += -Wno-maybe-uninitialized
+endif
# disable invalid "can't wrap" optimizations for signed / pointers
KBUILD_CFLAGS += -fno-strict-overflow
@@ -1104,7 +1109,9 @@ KBUILD_CFLAGS += -fno-strict-overflow
KBUILD_CFLAGS += -fno-stack-check
# conserve stack if available
-KBUILD_CFLAGS += $(call cc-option,-fconserve-stack)
+ifdef CONFIG_CC_IS_GCC
+KBUILD_CFLAGS += -fconserve-stack
+endif
# Prohibit date/time macros, which would make the build non-deterministic
KBUILD_CFLAGS += -Werror=date-time
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index b6b951b0ed46..a4a431606be2 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -364,6 +364,7 @@ config STRIP_ASM_SYMS
config READABLE_ASM
bool "Generate readable assembler code"
depends on DEBUG_KERNEL
+ depends on CC_IS_GCC
help
Disable some compiler optimizations that tend to generate human unreadable
assembler output. This may make the kernel slightly slower, but it helps
@@ -382,6 +383,7 @@ config HEADERS_INSTALL
config DEBUG_SECTION_MISMATCH
bool "Enable full Section mismatch analysis"
+ depends on CC_IS_GCC
help
The section mismatch analysis checks if there are illegal
references from one section to another section.
--
2.32.0.605.g8dce9f2422-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] Makefile: remove stale cc-option checks
2021-08-10 20:42 [PATCH] Makefile: remove stale cc-option checks Nick Desaulniers
@ 2021-08-10 23:01 ` Nathan Chancellor
2021-08-14 1:33 ` Masahiro Yamada
2021-08-11 10:41 ` Miguel Ojeda
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Nathan Chancellor @ 2021-08-10 23:01 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Masahiro Yamada, Michal Marek, Andrew Morton, Paul E. McKenney,
Peter Zijlstra, Miguel Ojeda, Tetsuo Handa, Vitor Massaru Iha,
Sedat Dilek, Daniel Latypov, linux-kbuild, linux-kernel,
clang-built-linux
On Tue, Aug 10, 2021 at 01:42:37PM -0700, Nick Desaulniers wrote:
> cc-option, cc-option-yn, and cc-disable-warning all invoke the compiler
> during build time, and can slow down the build when these checks become
> stale for our supported compilers, whose minimally supported versions
> increases over time. See Documentation/process/changes.rst for the
> current supported minimal versions (GCC 4.9+, clang 10.0.1+). Compiler
> version support for these flags may be verified on godbolt.org.
>
> The following flags are GCC only and supported since at least GCC 4.9.
> Remove cc-option and cc-disable-warning tests.
> * -fno-tree-loop-im
> * -Wno-maybe-uninitialized
> * -fno-reorder-blocks
> * -fno-ipa-cp-clone
> * -fno-partial-inlining
> * -femit-struct-debug-baseonly
> * -fno-inline-functions-called-once
> * -fconserve-stack
>
> The following flags are supported by all supported versions of GCC and
> Clang. Remove their cc-option, cc-option-yn, and cc-disable-warning tests.
> * -fno-delete-null-pointer-checks
> * -fno-var-tracking
> * -mfentry
> * -Wno-array-bounds
>
> The following configs are made dependent on GCC, since they use GCC
> specific flags.
> * READABLE_ASM
> * DEBUG_SECTION_MISMATCH
>
> --param=allow-store-data-races=0 was renamed to --allow-store-data-races
> in the GCC 10 release.
>
> Also, base RETPOLINE_CFLAGS and RETPOLINE_VDSO_CFLAGS on CONFIC_CC_IS_*
> then remove cc-option tests for Clang.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1436
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Fewer pointless calls to the compiler is always a good thing :)
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Small comments inline.
> ---
> Note: It may be preferred to move the test for
> -fno-inline-functions-called-once for DEBUG_SECTION_MISMATCH into
> Kconfig. That one does seem relatively more reasonable to implement in
> Clang.
>
> Makefile | 55 ++++++++++++++++++++++++++---------------------
> lib/Kconfig.debug | 2 ++
> 2 files changed, 33 insertions(+), 24 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 027fdf2a14fe..3e3fb4affba1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -730,9 +730,10 @@ endif # KBUILD_EXTMOD
> # Defaults to vmlinux, but the arch makefile usually adds further targets
> all: vmlinux
>
> -CFLAGS_GCOV := -fprofile-arcs -ftest-coverage \
> - $(call cc-option,-fno-tree-loop-im) \
> - $(call cc-disable-warning,maybe-uninitialized,)
> +CFLAGS_GCOV := -fprofile-arcs -ftest-coverage
> +ifdef CONFIG_CC_IS_GCC
> +CFLAGS_GCOV += -fno-tree-loop-im
> +endif
Eliminating -Wno-maybe-uninitialized might warrant a comment in the
commit message as I was initially confused then I realized that it is
unconditionally added later.
> export CFLAGS_GCOV
>
> # The arch Makefiles can override CC_FLAGS_FTRACE. We may also append it later.
> @@ -740,12 +741,14 @@ ifdef CONFIG_FUNCTION_TRACER
> CC_FLAGS_FTRACE := -pg
> endif
>
> -RETPOLINE_CFLAGS_GCC := -mindirect-branch=thunk-extern -mindirect-branch-register
> -RETPOLINE_VDSO_CFLAGS_GCC := -mindirect-branch=thunk-inline -mindirect-branch-register
> -RETPOLINE_CFLAGS_CLANG := -mretpoline-external-thunk
> -RETPOLINE_VDSO_CFLAGS_CLANG := -mretpoline
> -RETPOLINE_CFLAGS := $(call cc-option,$(RETPOLINE_CFLAGS_GCC),$(call cc-option,$(RETPOLINE_CFLAGS_CLANG)))
> -RETPOLINE_VDSO_CFLAGS := $(call cc-option,$(RETPOLINE_VDSO_CFLAGS_GCC),$(call cc-option,$(RETPOLINE_VDSO_CFLAGS_CLANG)))
> +ifdef CONFIG_CC_IS_GCC
> +RETPOLINE_CFLAGS := $(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register)
> +RETPOLINE_VDSO_CFLAGS := $(call cc-option,-mindirect-branch=thunk-inline -mindirect-branch-register)
> +endif
> +ifdef CONFIG_CC_IS_CLANG
> +RETPOLINE_CFLAGS := -mretpoline-external-thunk
> +RETPOLINE_VDSO_CFLAGS := -mretpoline
> +endif
> export RETPOLINE_CFLAGS
> export RETPOLINE_VDSO_CFLAGS
>
> @@ -798,7 +801,7 @@ include/config/auto.conf:
> endif # may-sync-config
> endif # need-config
>
> -KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)
> +KBUILD_CFLAGS += -fno-delete-null-pointer-checks
> KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
> KBUILD_CFLAGS += $(call cc-disable-warning, format-truncation)
> KBUILD_CFLAGS += $(call cc-disable-warning, format-overflow)
> @@ -844,17 +847,17 @@ KBUILD_RUSTFLAGS += -Copt-level=z
Looks like this was generated against -next as it includes the rust
patchset. I was able to apply it to mainline without any complaints but
any follow ups should probably be based on Masahiro's tree.
> endif
>
> # Tell gcc to never replace conditional load with a non-conditional one
> -KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
> +ifdef CONFIG_CC_IS_GCC
> +KBUILD_CFLAGS += $(call cc-option,--allow-store-data-races,--param=allow-store-data-races=0)
> KBUILD_CFLAGS += $(call cc-option,-fno-allow-store-data-races)
> +endif
>
> ifdef CONFIG_READABLE_ASM
> # Disable optimizations that make assembler listings hard to read.
> # reorder blocks reorders the control in the function
> # ipa clone creates specialized cloned functions
> # partial inlining inlines only parts of functions
> -KBUILD_CFLAGS += $(call cc-option,-fno-reorder-blocks,) \
> - $(call cc-option,-fno-ipa-cp-clone,) \
> - $(call cc-option,-fno-partial-inlining)
> +KBUILD_CFLAGS += -fno-reorder-blocks -fno-ipa-cp-clone -fno-partial-inlining
> endif
>
> ifneq ($(CONFIG_FRAME_WARN),0)
> @@ -959,8 +962,10 @@ DEBUG_CFLAGS += -gdwarf-$(dwarf-version-y)
> endif
>
> ifdef CONFIG_DEBUG_INFO_REDUCED
> -DEBUG_CFLAGS += $(call cc-option, -femit-struct-debug-baseonly) \
> - $(call cc-option,-fno-var-tracking)
> +DEBUG_CFLAGS += -fno-var-tracking
> +ifdef CONFIG_CC_IS_GCC
> +DEBUG_CFLAGS += -femit-struct-debug-baseonly
> +endif
> endif
>
> ifdef CONFIG_DEBUG_INFO_COMPRESSED
> @@ -997,10 +1002,8 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
> endif
> endif
> ifdef CONFIG_HAVE_FENTRY
> - ifeq ($(call cc-option-yn, -mfentry),y)
> - CC_FLAGS_FTRACE += -mfentry
> - CC_FLAGS_USING += -DCC_USING_FENTRY
> - endif
> + CC_FLAGS_FTRACE += -mfentry
> + CC_FLAGS_USING += -DCC_USING_FENTRY
> endif
> export CC_FLAGS_FTRACE
> KBUILD_CFLAGS += $(CC_FLAGS_FTRACE) $(CC_FLAGS_USING)
> @@ -1009,7 +1012,7 @@ endif
>
> # We trigger additional mismatches with less inlining
> ifdef CONFIG_DEBUG_SECTION_MISMATCH
> -KBUILD_CFLAGS += $(call cc-option, -fno-inline-functions-called-once)
> +KBUILD_CFLAGS += -fno-inline-functions-called-once
> endif
>
> ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
> @@ -1088,14 +1091,16 @@ KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation)
>
> # We'll want to enable this eventually, but it's not going away for 5.7 at least
> KBUILD_CFLAGS += $(call cc-disable-warning, zero-length-bounds)
> -KBUILD_CFLAGS += $(call cc-disable-warning, array-bounds)
> +KBUILD_CFLAGS += -Wno-array-bounds
> KBUILD_CFLAGS += $(call cc-disable-warning, stringop-overflow)
>
> # Another good warning that we'll want to enable eventually
> KBUILD_CFLAGS += $(call cc-disable-warning, restrict)
>
> # Enabled with W=2, disabled by default as noisy
> -KBUILD_CFLAGS += $(call cc-disable-warning, maybe-uninitialized)
> +ifdef CONFIG_CC_IS_GCC
> +KBUILD_CFLAGS += -Wno-maybe-uninitialized
> +endif
>
> # disable invalid "can't wrap" optimizations for signed / pointers
> KBUILD_CFLAGS += -fno-strict-overflow
> @@ -1104,7 +1109,9 @@ KBUILD_CFLAGS += -fno-strict-overflow
> KBUILD_CFLAGS += -fno-stack-check
>
> # conserve stack if available
> -KBUILD_CFLAGS += $(call cc-option,-fconserve-stack)
> +ifdef CONFIG_CC_IS_GCC
> +KBUILD_CFLAGS += -fconserve-stack
> +endif
>
> # Prohibit date/time macros, which would make the build non-deterministic
> KBUILD_CFLAGS += -Werror=date-time
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index b6b951b0ed46..a4a431606be2 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -364,6 +364,7 @@ config STRIP_ASM_SYMS
> config READABLE_ASM
> bool "Generate readable assembler code"
> depends on DEBUG_KERNEL
> + depends on CC_IS_GCC
> help
> Disable some compiler optimizations that tend to generate human unreadable
> assembler output. This may make the kernel slightly slower, but it helps
> @@ -382,6 +383,7 @@ config HEADERS_INSTALL
>
> config DEBUG_SECTION_MISMATCH
> bool "Enable full Section mismatch analysis"
> + depends on CC_IS_GCC
> help
> The section mismatch analysis checks if there are illegal
> references from one section to another section.
> --
> 2.32.0.605.g8dce9f2422-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Makefile: remove stale cc-option checks
2021-08-10 20:42 [PATCH] Makefile: remove stale cc-option checks Nick Desaulniers
2021-08-10 23:01 ` Nathan Chancellor
@ 2021-08-11 10:41 ` Miguel Ojeda
2021-08-14 1:42 ` Masahiro Yamada
2021-08-14 11:02 ` Naresh Kamboju
3 siblings, 0 replies; 13+ messages in thread
From: Miguel Ojeda @ 2021-08-11 10:41 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Masahiro Yamada, Michal Marek, Nathan Chancellor, Andrew Morton,
Paul E. McKenney, Peter Zijlstra, Miguel Ojeda, Tetsuo Handa,
Vitor Massaru Iha, Sedat Dilek, Daniel Latypov,
Linux Kbuild mailing list, linux-kernel, clang-built-linux
On Tue, Aug 10, 2021 at 10:42 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> cc-option, cc-option-yn, and cc-disable-warning all invoke the compiler
> during build time, and can slow down the build when these checks become
> stale for our supported compilers, whose minimally supported versions
> increases over time. See Documentation/process/changes.rst for the
> current supported minimal versions (GCC 4.9+, clang 10.0.1+). Compiler
> version support for these flags may be verified on godbolt.org.
Always nice to see cleanups due to raised versions!
Shouldn't `--allow-store-data-races` have been on its own patch, though?
Acked-by: Miguel Ojeda <ojeda@kernel.org>
Cheers,
Miguel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Makefile: remove stale cc-option checks
2021-08-10 23:01 ` Nathan Chancellor
@ 2021-08-14 1:33 ` Masahiro Yamada
0 siblings, 0 replies; 13+ messages in thread
From: Masahiro Yamada @ 2021-08-14 1:33 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Nick Desaulniers, Michal Marek, Andrew Morton, Paul E. McKenney,
Peter Zijlstra, Miguel Ojeda, Tetsuo Handa, Vitor Massaru Iha,
Sedat Dilek, Daniel Latypov, Linux Kbuild mailing list,
Linux Kernel Mailing List, clang-built-linux
On Wed, Aug 11, 2021 at 8:01 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Tue, Aug 10, 2021 at 01:42:37PM -0700, Nick Desaulniers wrote:
> > cc-option, cc-option-yn, and cc-disable-warning all invoke the compiler
> > during build time, and can slow down the build when these checks become
> > stale for our supported compilers, whose minimally supported versions
> > increases over time. See Documentation/process/changes.rst for the
> > current supported minimal versions (GCC 4.9+, clang 10.0.1+). Compiler
> > version support for these flags may be verified on godbolt.org.
> >
> > The following flags are GCC only and supported since at least GCC 4.9.
> > Remove cc-option and cc-disable-warning tests.
> > * -fno-tree-loop-im
> > * -Wno-maybe-uninitialized
> > * -fno-reorder-blocks
> > * -fno-ipa-cp-clone
> > * -fno-partial-inlining
> > * -femit-struct-debug-baseonly
> > * -fno-inline-functions-called-once
> > * -fconserve-stack
> >
> > The following flags are supported by all supported versions of GCC and
> > Clang. Remove their cc-option, cc-option-yn, and cc-disable-warning tests.
> > * -fno-delete-null-pointer-checks
> > * -fno-var-tracking
> > * -mfentry
> > * -Wno-array-bounds
> >
> > The following configs are made dependent on GCC, since they use GCC
> > specific flags.
> > * READABLE_ASM
> > * DEBUG_SECTION_MISMATCH
> >
> > --param=allow-store-data-races=0 was renamed to --allow-store-data-races
> > in the GCC 10 release.
> >
> > Also, base RETPOLINE_CFLAGS and RETPOLINE_VDSO_CFLAGS on CONFIC_CC_IS_*
> > then remove cc-option tests for Clang.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1436
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Fewer pointless calls to the compiler is always a good thing :)
>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>
> Small comments inline.
>
> > ---
> > Note: It may be preferred to move the test for
> > -fno-inline-functions-called-once for DEBUG_SECTION_MISMATCH into
> > Kconfig. That one does seem relatively more reasonable to implement in
> > Clang.
> >
> > Makefile | 55 ++++++++++++++++++++++++++---------------------
> > lib/Kconfig.debug | 2 ++
> > 2 files changed, 33 insertions(+), 24 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 027fdf2a14fe..3e3fb4affba1 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -730,9 +730,10 @@ endif # KBUILD_EXTMOD
> > # Defaults to vmlinux, but the arch makefile usually adds further targets
> > all: vmlinux
> >
> > -CFLAGS_GCOV := -fprofile-arcs -ftest-coverage \
> > - $(call cc-option,-fno-tree-loop-im) \
> > - $(call cc-disable-warning,maybe-uninitialized,)
> > +CFLAGS_GCOV := -fprofile-arcs -ftest-coverage
> > +ifdef CONFIG_CC_IS_GCC
> > +CFLAGS_GCOV += -fno-tree-loop-im
> > +endif
>
> Eliminating -Wno-maybe-uninitialized might warrant a comment in the
> commit message as I was initially confused then I realized that it is
> unconditionally added later.
Indeed.
Commit 78a5255ffb6a1af189a83e493d916ba1c54d8c75
could have removed -Wno-maybe-initialized
from CFLAGS_GCOV as well, but somehow it has been
left over here...
Comments in the commit log, or perhaps
splitting this as 1/2 will be less confusing.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Makefile: remove stale cc-option checks
2021-08-10 20:42 [PATCH] Makefile: remove stale cc-option checks Nick Desaulniers
2021-08-10 23:01 ` Nathan Chancellor
2021-08-11 10:41 ` Miguel Ojeda
@ 2021-08-14 1:42 ` Masahiro Yamada
2021-08-16 18:35 ` Nick Desaulniers
2021-08-16 20:06 ` Nick Desaulniers
2021-08-14 11:02 ` Naresh Kamboju
3 siblings, 2 replies; 13+ messages in thread
From: Masahiro Yamada @ 2021-08-14 1:42 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Michal Marek, Nathan Chancellor, Andrew Morton, Paul E. McKenney,
Peter Zijlstra, Miguel Ojeda, Tetsuo Handa, Vitor Massaru Iha,
Sedat Dilek, Daniel Latypov, Linux Kbuild mailing list,
Linux Kernel Mailing List, clang-built-linux
On Wed, Aug 11, 2021 at 5:42 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> cc-option, cc-option-yn, and cc-disable-warning all invoke the compiler
> during build time, and can slow down the build when these checks become
> stale for our supported compilers, whose minimally supported versions
> increases over time. See Documentation/process/changes.rst for the
> current supported minimal versions (GCC 4.9+, clang 10.0.1+). Compiler
> version support for these flags may be verified on godbolt.org.
>
> The following flags are GCC only and supported since at least GCC 4.9.
> Remove cc-option and cc-disable-warning tests.
> * -fno-tree-loop-im
> * -Wno-maybe-uninitialized
> * -fno-reorder-blocks
> * -fno-ipa-cp-clone
> * -fno-partial-inlining
> * -femit-struct-debug-baseonly
> * -fno-inline-functions-called-once
> * -fconserve-stack
>
> The following flags are supported by all supported versions of GCC and
> Clang. Remove their cc-option, cc-option-yn, and cc-disable-warning tests.
> * -fno-delete-null-pointer-checks
> * -fno-var-tracking
> * -mfentry
> * -Wno-array-bounds
>
> The following configs are made dependent on GCC, since they use GCC
> specific flags.
> * READABLE_ASM
> * DEBUG_SECTION_MISMATCH
>
> --param=allow-store-data-races=0 was renamed to --allow-store-data-races
> in the GCC 10 release.
>
> Also, base RETPOLINE_CFLAGS and RETPOLINE_VDSO_CFLAGS on CONFIC_CC_IS_*
> then remove cc-option tests for Clang.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1436
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Note: It may be preferred to move the test for
> -fno-inline-functions-called-once for DEBUG_SECTION_MISMATCH into
> Kconfig. That one does seem relatively more reasonable to implement in
> Clang.
>
> Makefile | 55 ++++++++++++++++++++++++++---------------------
> lib/Kconfig.debug | 2 ++
> 2 files changed, 33 insertions(+), 24 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 027fdf2a14fe..3e3fb4affba1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -730,9 +730,10 @@ endif # KBUILD_EXTMOD
> # Defaults to vmlinux, but the arch makefile usually adds further targets
> all: vmlinux
>
> -CFLAGS_GCOV := -fprofile-arcs -ftest-coverage \
> - $(call cc-option,-fno-tree-loop-im) \
> - $(call cc-disable-warning,maybe-uninitialized,)
> +CFLAGS_GCOV := -fprofile-arcs -ftest-coverage
> +ifdef CONFIG_CC_IS_GCC
> +CFLAGS_GCOV += -fno-tree-loop-im
> +endif
> export CFLAGS_GCOV
>
> # The arch Makefiles can override CC_FLAGS_FTRACE. We may also append it later.
> @@ -740,12 +741,14 @@ ifdef CONFIG_FUNCTION_TRACER
> CC_FLAGS_FTRACE := -pg
> endif
>
> -RETPOLINE_CFLAGS_GCC := -mindirect-branch=thunk-extern -mindirect-branch-register
> -RETPOLINE_VDSO_CFLAGS_GCC := -mindirect-branch=thunk-inline -mindirect-branch-register
> -RETPOLINE_CFLAGS_CLANG := -mretpoline-external-thunk
> -RETPOLINE_VDSO_CFLAGS_CLANG := -mretpoline
> -RETPOLINE_CFLAGS := $(call cc-option,$(RETPOLINE_CFLAGS_GCC),$(call cc-option,$(RETPOLINE_CFLAGS_CLANG)))
> -RETPOLINE_VDSO_CFLAGS := $(call cc-option,$(RETPOLINE_VDSO_CFLAGS_GCC),$(call cc-option,$(RETPOLINE_VDSO_CFLAGS_CLANG)))
> +ifdef CONFIG_CC_IS_GCC
> +RETPOLINE_CFLAGS := $(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register)
> +RETPOLINE_VDSO_CFLAGS := $(call cc-option,-mindirect-branch=thunk-inline -mindirect-branch-register)
> +endif
> +ifdef CONFIG_CC_IS_CLANG
> +RETPOLINE_CFLAGS := -mretpoline-external-thunk
> +RETPOLINE_VDSO_CFLAGS := -mretpoline
> +endif
> export RETPOLINE_CFLAGS
> export RETPOLINE_VDSO_CFLAGS
>
> @@ -798,7 +801,7 @@ include/config/auto.conf:
> endif # may-sync-config
> endif # need-config
>
> -KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)
> +KBUILD_CFLAGS += -fno-delete-null-pointer-checks
> KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
> KBUILD_CFLAGS += $(call cc-disable-warning, format-truncation)
> KBUILD_CFLAGS += $(call cc-disable-warning, format-overflow)
> @@ -844,17 +847,17 @@ KBUILD_RUSTFLAGS += -Copt-level=z
> endif
>
> # Tell gcc to never replace conditional load with a non-conditional one
> -KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
> +ifdef CONFIG_CC_IS_GCC
Can you insert a comment here?
# GCC 10 renamed --param=allow-store-data-races=0 to --allow-store-data-races
It will remind us of dropping this conditional
in the (long long distant) future.
> +KBUILD_CFLAGS += $(call cc-option,--allow-store-data-races,--param=allow-store-data-races=0)
> KBUILD_CFLAGS += $(call cc-option,-fno-allow-store-data-races)
> +endif
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Makefile: remove stale cc-option checks
2021-08-10 20:42 [PATCH] Makefile: remove stale cc-option checks Nick Desaulniers
` (2 preceding siblings ...)
2021-08-14 1:42 ` Masahiro Yamada
@ 2021-08-14 11:02 ` Naresh Kamboju
2021-08-16 18:37 ` Nick Desaulniers
3 siblings, 1 reply; 13+ messages in thread
From: Naresh Kamboju @ 2021-08-14 11:02 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Masahiro Yamada, Michal Marek, Nathan Chancellor, Andrew Morton,
Paul E. McKenney, Peter Zijlstra, Miguel Ojeda, Tetsuo Handa,
Vitor Massaru Iha, Sedat Dilek, Daniel Latypov, linux-kbuild,
open list, clang-built-linux, Linux-Next Mailing List,
Stephen Rothwell, Mark Brown, lkft-triage
On Wed, 11 Aug 2021 at 02:12, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> cc-option, cc-option-yn, and cc-disable-warning all invoke the compiler
> during build time, and can slow down the build when these checks become
> stale for our supported compilers, whose minimally supported versions
> increases over time. See Documentation/process/changes.rst for the
> current supported minimal versions (GCC 4.9+, clang 10.0.1+). Compiler
> version support for these flags may be verified on godbolt.org.
>
> The following flags are GCC only and supported since at least GCC 4.9.
> Remove cc-option and cc-disable-warning tests.
> * -fno-tree-loop-im
> * -Wno-maybe-uninitialized
> * -fno-reorder-blocks
> * -fno-ipa-cp-clone
> * -fno-partial-inlining
> * -femit-struct-debug-baseonly
> * -fno-inline-functions-called-once
> * -fconserve-stack
>
> The following flags are supported by all supported versions of GCC and
> Clang. Remove their cc-option, cc-option-yn, and cc-disable-warning tests.
> * -fno-delete-null-pointer-checks
> * -fno-var-tracking
> * -mfentry
> * -Wno-array-bounds
>
> The following configs are made dependent on GCC, since they use GCC
> specific flags.
> * READABLE_ASM
> * DEBUG_SECTION_MISMATCH
>
> --param=allow-store-data-races=0 was renamed to --allow-store-data-races
> in the GCC 10 release.
[Please ignore this if it is already reported]
Linux next 20210813 tag s390 build failed with gcc-8 but pass with
gcc-9 and gcc-10.
s390 (defconfig) with gcc-8 FAILED
s390 (defconfig) with gcc-9 PASS
s390 (defconfig) with gcc-10 PASS
Build error:
-----------
s390x-linux-gnu-gcc: error: unrecognized command line option
'-mfentry'; did you mean '--entry'?
make[2]: *** [/builds/linux/scripts/Makefile.build:272:
scripts/mod/empty.o] Error 1
s390x-linux-gnu-gcc: error: unrecognized command line option
'-mfentry'; did you mean '--entry'?
make[2]: *** [/builds/linux/scripts/Makefile.build:118:
scripts/mod/devicetable-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Build log,
https://builds.tuxbuild.com/1wfNcaYbsp29k3RvYuPXzxrM4vs/
metadata:
--------
git_describe: next-20210813
git_repo: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next
git_short_log: 4b358aabb93a (\Add linux-next specific files for 20210813\)
target_arch: s390
toolchain: gcc-8
Steps to reproduce:
-------------------
# TuxMake is a command line tool and Python library that provides
# portable and repeatable Linux kernel builds across a variety of
# architectures, toolchains, kernel configurations, and make targets.
#
# TuxMake supports the concept of runtimes.
# See https://docs.tuxmake.org/runtimes/, for that to work it requires
# that you install podman or docker on your system.
#
# To install tuxmake on your system globally:
# sudo pip3 install -U tuxmake
#
# See https://docs.tuxmake.org/ for complete documentation.
tuxmake --runtime podman --target-arch s390 --toolchain gcc-8
--kconfig defconfig
--
Linaro LKFT
https://lkft.linaro.org
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Makefile: remove stale cc-option checks
2021-08-14 1:42 ` Masahiro Yamada
@ 2021-08-16 18:35 ` Nick Desaulniers
2021-08-17 0:16 ` Masahiro Yamada
2021-08-16 20:06 ` Nick Desaulniers
1 sibling, 1 reply; 13+ messages in thread
From: Nick Desaulniers @ 2021-08-16 18:35 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Michal Marek, Nathan Chancellor, Andrew Morton, Paul E. McKenney,
Peter Zijlstra, Miguel Ojeda, Tetsuo Handa, Vitor Massaru Iha,
Sedat Dilek, Daniel Latypov, Linux Kbuild mailing list,
Linux Kernel Mailing List, clang-built-linux, Naresh Kamboju
On Fri, Aug 13, 2021 at 6:43 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Aug 11, 2021 at 5:42 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > --param=allow-store-data-races=0 was renamed to --allow-store-data-races
> > in the GCC 10 release.
> >
> > diff --git a/Makefile b/Makefile
> > index 027fdf2a14fe..3e3fb4affba1 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -844,17 +847,17 @@ KBUILD_RUSTFLAGS += -Copt-level=z
> > endif
> >
> > # Tell gcc to never replace conditional load with a non-conditional one
> > -KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
> > +ifdef CONFIG_CC_IS_GCC
>
>
> Can you insert a comment here?
>
> # GCC 10 renamed --param=allow-store-data-races=0 to --allow-store-data-races
>
>
> It will remind us of dropping this conditional
> in the (long long distant) future.
>
>
>
>
> > +KBUILD_CFLAGS += $(call cc-option,--allow-store-data-races,--param=allow-store-data-races=0)
> > KBUILD_CFLAGS += $(call cc-option,-fno-allow-store-data-races)
> > +endif
This report is confusing:
https://lore.kernel.org/linux-mm/202108160729.Lx0IJzq3-lkp@intel.com/
(csky gcc-11)
>> csky-linux-gcc: error: unrecognized command-line option '--param=allow-store-data-races=0'; did you mean '--allow-store-data-races'?
I wonder if cc-option detection for these is broken? Perhaps I should
not touch these other than to wrap them in the CONFIG_CC_IS_GCC guard?
(Either way, I need to send a v2 in response to Naresh's report as
well. https://lore.kernel.org/lkml/CA+G9fYtPBp_-Ko_P7NuOX6vN9-66rjJuBt21h3arrLqEaQQn6w@mail.gmail.com/
It seems that -mfentry wasn't implemented for s390-linux-gnu-gcc until
gcc-9; so rather than remove top level support, perhaps a comment
about gcc-9+ s390 having support will make grepping for it easier in
the future).
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Makefile: remove stale cc-option checks
2021-08-14 11:02 ` Naresh Kamboju
@ 2021-08-16 18:37 ` Nick Desaulniers
2021-08-16 23:31 ` Stephen Rothwell
0 siblings, 1 reply; 13+ messages in thread
From: Nick Desaulniers @ 2021-08-16 18:37 UTC (permalink / raw)
To: Naresh Kamboju
Cc: Masahiro Yamada, Michal Marek, Nathan Chancellor, Andrew Morton,
Paul E. McKenney, Peter Zijlstra, Miguel Ojeda, Tetsuo Handa,
Vitor Massaru Iha, Sedat Dilek, Daniel Latypov, linux-kbuild,
open list, clang-built-linux, Linux-Next Mailing List,
Stephen Rothwell, Mark Brown, lkft-triage
On Sat, Aug 14, 2021 at 4:02 AM Naresh Kamboju
<naresh.kamboju@linaro.org> wrote:
>
> On Wed, 11 Aug 2021 at 02:12, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > cc-option, cc-option-yn, and cc-disable-warning all invoke the compiler
> > during build time, and can slow down the build when these checks become
> > stale for our supported compilers, whose minimally supported versions
> > increases over time. See Documentation/process/changes.rst for the
> > current supported minimal versions (GCC 4.9+, clang 10.0.1+). Compiler
> > version support for these flags may be verified on godbolt.org.
> >
> > The following flags are GCC only and supported since at least GCC 4.9.
> > Remove cc-option and cc-disable-warning tests.
> > * -fno-tree-loop-im
> > * -Wno-maybe-uninitialized
> > * -fno-reorder-blocks
> > * -fno-ipa-cp-clone
> > * -fno-partial-inlining
> > * -femit-struct-debug-baseonly
> > * -fno-inline-functions-called-once
> > * -fconserve-stack
> >
> > The following flags are supported by all supported versions of GCC and
> > Clang. Remove their cc-option, cc-option-yn, and cc-disable-warning tests.
> > * -fno-delete-null-pointer-checks
> > * -fno-var-tracking
> > * -mfentry
> > * -Wno-array-bounds
> >
> > The following configs are made dependent on GCC, since they use GCC
> > specific flags.
> > * READABLE_ASM
> > * DEBUG_SECTION_MISMATCH
> >
> > --param=allow-store-data-races=0 was renamed to --allow-store-data-races
> > in the GCC 10 release.
>
> [Please ignore this if it is already reported]
>
> Linux next 20210813 tag s390 build failed with gcc-8 but pass with
> gcc-9 and gcc-10.
>
> s390 (defconfig) with gcc-8 FAILED
> s390 (defconfig) with gcc-9 PASS
> s390 (defconfig) with gcc-10 PASS
Thanks for the report. Andrew has dropped the patch from mm-next.
Looks like it's too soon to remove build configuration tests for
-mfentry.
>
> Build error:
> -----------
> s390x-linux-gnu-gcc: error: unrecognized command line option
> '-mfentry'; did you mean '--entry'?
> make[2]: *** [/builds/linux/scripts/Makefile.build:272:
> scripts/mod/empty.o] Error 1
> s390x-linux-gnu-gcc: error: unrecognized command line option
> '-mfentry'; did you mean '--entry'?
> make[2]: *** [/builds/linux/scripts/Makefile.build:118:
> scripts/mod/devicetable-offsets.s] Error 1
> make[2]: Target '__build' not remade because of errors.
>
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
>
> Build log,
> https://builds.tuxbuild.com/1wfNcaYbsp29k3RvYuPXzxrM4vs/
>
> metadata:
> --------
> git_describe: next-20210813
> git_repo: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next
> git_short_log: 4b358aabb93a (\Add linux-next specific files for 20210813\)
> target_arch: s390
> toolchain: gcc-8
>
>
> Steps to reproduce:
> -------------------
> # TuxMake is a command line tool and Python library that provides
> # portable and repeatable Linux kernel builds across a variety of
> # architectures, toolchains, kernel configurations, and make targets.
> #
> # TuxMake supports the concept of runtimes.
> # See https://docs.tuxmake.org/runtimes/, for that to work it requires
> # that you install podman or docker on your system.
> #
> # To install tuxmake on your system globally:
> # sudo pip3 install -U tuxmake
> #
> # See https://docs.tuxmake.org/ for complete documentation.
>
>
> tuxmake --runtime podman --target-arch s390 --toolchain gcc-8
> --kconfig defconfig
>
>
> --
> Linaro LKFT
> https://lkft.linaro.org
>
> --
> 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-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CA%2BG9fYtPBp_-Ko_P7NuOX6vN9-66rjJuBt21h3arrLqEaQQn6w%40mail.gmail.com.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Makefile: remove stale cc-option checks
2021-08-14 1:42 ` Masahiro Yamada
2021-08-16 18:35 ` Nick Desaulniers
@ 2021-08-16 20:06 ` Nick Desaulniers
2021-08-16 20:11 ` Nick Desaulniers
1 sibling, 1 reply; 13+ messages in thread
From: Nick Desaulniers @ 2021-08-16 20:06 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Michal Marek, Nathan Chancellor, Andrew Morton, Paul E. McKenney,
Peter Zijlstra, Miguel Ojeda, Tetsuo Handa, Vitor Massaru Iha,
Sedat Dilek, Daniel Latypov, Linux Kbuild mailing list,
Linux Kernel Mailing List, clang-built-linux
On Fri, Aug 13, 2021 at 6:43 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Aug 11, 2021 at 5:42 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > diff --git a/Makefile b/Makefile
> > index 027fdf2a14fe..3e3fb4affba1 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -798,7 +801,7 @@ include/config/auto.conf:
> > endif # may-sync-config
> > endif # need-config
> >
> > -KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)
> > +KBUILD_CFLAGS += -fno-delete-null-pointer-checks
> > KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
> > KBUILD_CFLAGS += $(call cc-disable-warning, format-truncation)
> > KBUILD_CFLAGS += $(call cc-disable-warning, format-overflow)
> > @@ -844,17 +847,17 @@ KBUILD_RUSTFLAGS += -Copt-level=z
> > endif
> >
> > # Tell gcc to never replace conditional load with a non-conditional one
> > -KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
> > +ifdef CONFIG_CC_IS_GCC
>
>
> Can you insert a comment here?
>
> # GCC 10 renamed --param=allow-store-data-races=0 to --allow-store-data-races
>
>
> It will remind us of dropping this conditional
> in the (long long distant) future.
>
>
>
>
> > +KBUILD_CFLAGS += $(call cc-option,--allow-store-data-races,--param=allow-store-data-races=0)
> > KBUILD_CFLAGS += $(call cc-option,-fno-allow-store-data-races)
> > +endif
So it looks like `-fallow-store-data-races` was also supported. Reading through
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97309
https://gcc.gnu.org/wiki/Atomic/GCCMM/ExecutiveSummary
it seems that the default is not to allow "store data races." So I
should not be adding `--allow-store-data-races` or
`-fallow-store-data-races`; so this just should be the existing test,
with a comment that it can be removed once gcc-10+ is the minimum.
Will update that in v2.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Makefile: remove stale cc-option checks
2021-08-16 20:06 ` Nick Desaulniers
@ 2021-08-16 20:11 ` Nick Desaulniers
0 siblings, 0 replies; 13+ messages in thread
From: Nick Desaulniers @ 2021-08-16 20:11 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Michal Marek, Nathan Chancellor, Andrew Morton, Paul E. McKenney,
Peter Zijlstra, Miguel Ojeda, Tetsuo Handa, Vitor Massaru Iha,
Sedat Dilek, Daniel Latypov, Linux Kbuild mailing list,
Linux Kernel Mailing List, clang-built-linux
On Mon, Aug 16, 2021 at 1:06 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Aug 13, 2021 at 6:43 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Wed, Aug 11, 2021 at 5:42 AM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 027fdf2a14fe..3e3fb4affba1 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -798,7 +801,7 @@ include/config/auto.conf:
> > > endif # may-sync-config
> > > endif # need-config
> > >
> > > -KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)
> > > +KBUILD_CFLAGS += -fno-delete-null-pointer-checks
> > > KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
> > > KBUILD_CFLAGS += $(call cc-disable-warning, format-truncation)
> > > KBUILD_CFLAGS += $(call cc-disable-warning, format-overflow)
> > > @@ -844,17 +847,17 @@ KBUILD_RUSTFLAGS += -Copt-level=z
> > > endif
> > >
> > > # Tell gcc to never replace conditional load with a non-conditional one
> > > -KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
> > > +ifdef CONFIG_CC_IS_GCC
> >
> >
> > Can you insert a comment here?
> >
> > # GCC 10 renamed --param=allow-store-data-races=0 to --allow-store-data-races
> >
> >
> > It will remind us of dropping this conditional
> > in the (long long distant) future.
> >
> >
> >
> >
> > > +KBUILD_CFLAGS += $(call cc-option,--allow-store-data-races,--param=allow-store-data-races=0)
> > > KBUILD_CFLAGS += $(call cc-option,-fno-allow-store-data-races)
> > > +endif
>
> So it looks like `-fallow-store-data-races` was also supported. Reading through
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97309
> https://gcc.gnu.org/wiki/Atomic/GCCMM/ExecutiveSummary
> it seems that the default is not to allow "store data races." So I
> should not be adding `--allow-store-data-races` or
> `-fallow-store-data-races`; so this just should be the existing test,
> with a comment that it can be removed once gcc-10+ is the minimum.
> Will update that in v2.
Ah, perhaps I should combine the following line:
KBUILD_CFLAGS += $(call
cc-option,-fallow-store-data-races,--param=allow-store-data-races=0)
Though I don't understand the csky failure. cc-option appends to a
running list of flags; if an unsupported flag gets added to CFLAGS, it
can cause cc-option to failure subsequent checks in spooky ways. I
usually debug those by adding print statements to cc-option to see
what the full command it's testing; then invoke the compiler manually
with those to see what went wrong. (bookmarked and referenced often:
https://stackoverflow.com/questions/16467718/how-to-print-out-a-variable-in-makefile)
Though my distro doesn't package csky-linux-gnu-gcc...
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Makefile: remove stale cc-option checks
2021-08-16 18:37 ` Nick Desaulniers
@ 2021-08-16 23:31 ` Stephen Rothwell
0 siblings, 0 replies; 13+ messages in thread
From: Stephen Rothwell @ 2021-08-16 23:31 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Naresh Kamboju, Masahiro Yamada, Michal Marek, Nathan Chancellor,
Andrew Morton, Paul E. McKenney, Peter Zijlstra, Miguel Ojeda,
Tetsuo Handa, Vitor Massaru Iha, Sedat Dilek, Daniel Latypov,
linux-kbuild, open list, clang-built-linux,
Linux-Next Mailing List, Mark Brown, lkft-triage
[-- Attachment #1: Type: text/plain, Size: 344 bytes --]
Hi all,
On Mon, 16 Aug 2021 11:37:23 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> Thanks for the report. Andrew has dropped the patch from mm-next.
> Looks like it's too soon to remove build configuration tests for
> -mfentry.
I have removed that patch from linux-next today as well.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Makefile: remove stale cc-option checks
2021-08-16 18:35 ` Nick Desaulniers
@ 2021-08-17 0:16 ` Masahiro Yamada
2021-08-17 1:38 ` Masahiro Yamada
0 siblings, 1 reply; 13+ messages in thread
From: Masahiro Yamada @ 2021-08-17 0:16 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Michal Marek, Nathan Chancellor, Andrew Morton, Paul E. McKenney,
Peter Zijlstra, Miguel Ojeda, Tetsuo Handa, Vitor Massaru Iha,
Sedat Dilek, Daniel Latypov, Linux Kbuild mailing list,
Linux Kernel Mailing List, clang-built-linux, Naresh Kamboju
On Tue, Aug 17, 2021 at 3:36 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Aug 13, 2021 at 6:43 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Wed, Aug 11, 2021 at 5:42 AM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > --param=allow-store-data-races=0 was renamed to --allow-store-data-races
> > > in the GCC 10 release.
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 027fdf2a14fe..3e3fb4affba1 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -844,17 +847,17 @@ KBUILD_RUSTFLAGS += -Copt-level=z
> > > endif
> > >
> > > # Tell gcc to never replace conditional load with a non-conditional one
> > > -KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
> > > +ifdef CONFIG_CC_IS_GCC
> >
> >
> > Can you insert a comment here?
> >
> > # GCC 10 renamed --param=allow-store-data-races=0 to --allow-store-data-races
> >
> >
> > It will remind us of dropping this conditional
> > in the (long long distant) future.
> >
> >
> >
> >
> > > +KBUILD_CFLAGS += $(call cc-option,--allow-store-data-races,--param=allow-store-data-races=0)
> > > KBUILD_CFLAGS += $(call cc-option,-fno-allow-store-data-races)
> > > +endif
>
> This report is confusing:
> https://lore.kernel.org/linux-mm/202108160729.Lx0IJzq3-lkp@intel.com/
> (csky gcc-11)
>
> >> csky-linux-gcc: error: unrecognized command-line option '--param=allow-store-data-races=0'; did you mean '--allow-store-data-races'?
>
> I wonder if cc-option detection for these is broken?
I do not say it is broken...
cc-option is defined like this:
cc-option = $(call __cc-option, $(CC),\
$(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS),$(1),$(2))
It is checking
$(KBUILD_CPPFLAGS) + $(KBUILD_CFLAGS)
+ --allow-store-data-races
A few lines above, I see
csky-linux-gcc: error: unrecognized argument in option '-mcpu=ck860'
It makes all the cc-option tests fail after this line:
KBUILD_CFLAGS += -mcpu=$(CPUTYPE) -Wa,-mcpu=$(MCPU_STR)
Then,
$(call cc-option,--allow-store-data-races,--param=allow-store-data-races=0)
falls back to --param=allow-store-data-races=0
> Perhaps I should
> not touch these other than to wrap them in the CONFIG_CC_IS_GCC guard?
I do not think so.
If an unrecognized argument appears,
all the cc-option tests that follow are unreliable.
If you are not comfortable with it,
you can split it.
KBUILD_CFLAGS += $(call cc-option,--allow-store-data-races)
KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
Or, another way of implementation is
KBUILD_CFLAGS += $(call cc-ifversion, -lt, 1000,
--allow-store-data-races, --param=allow-store-data-races=0)
>
> (Either way, I need to send a v2 in response to Naresh's report as
> well. https://lore.kernel.org/lkml/CA+G9fYtPBp_-Ko_P7NuOX6vN9-66rjJuBt21h3arrLqEaQQn6w@mail.gmail.com/
> It seems that -mfentry wasn't implemented for s390-linux-gnu-gcc until
> gcc-9; so rather than remove top level support, perhaps a comment
> about gcc-9+ s390 having support will make grepping for it easier in
> the future).
> --
> Thanks,
> ~Nick Desaulniers
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Makefile: remove stale cc-option checks
2021-08-17 0:16 ` Masahiro Yamada
@ 2021-08-17 1:38 ` Masahiro Yamada
0 siblings, 0 replies; 13+ messages in thread
From: Masahiro Yamada @ 2021-08-17 1:38 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Michal Marek, Nathan Chancellor, Andrew Morton, Paul E. McKenney,
Peter Zijlstra, Miguel Ojeda, Tetsuo Handa, Vitor Massaru Iha,
Sedat Dilek, Daniel Latypov, Linux Kbuild mailing list,
Linux Kernel Mailing List, clang-built-linux, Naresh Kamboju
On Tue, Aug 17, 2021 at 9:16 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Tue, Aug 17, 2021 at 3:36 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Fri, Aug 13, 2021 at 6:43 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > On Wed, Aug 11, 2021 at 5:42 AM Nick Desaulniers
> > > <ndesaulniers@google.com> wrote:
> > > >
> > > > --param=allow-store-data-races=0 was renamed to --allow-store-data-races
> > > > in the GCC 10 release.
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index 027fdf2a14fe..3e3fb4affba1 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -844,17 +847,17 @@ KBUILD_RUSTFLAGS += -Copt-level=z
> > > > endif
> > > >
> > > > # Tell gcc to never replace conditional load with a non-conditional one
> > > > -KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
> > > > +ifdef CONFIG_CC_IS_GCC
> > >
> > >
> > > Can you insert a comment here?
> > >
> > > # GCC 10 renamed --param=allow-store-data-races=0 to --allow-store-data-races
> > >
> > >
> > > It will remind us of dropping this conditional
> > > in the (long long distant) future.
> > >
> > >
> > >
> > >
> > > > +KBUILD_CFLAGS += $(call cc-option,--allow-store-data-races,--param=allow-store-data-races=0)
> > > > KBUILD_CFLAGS += $(call cc-option,-fno-allow-store-data-races)
> > > > +endif
> >
> > This report is confusing:
> > https://lore.kernel.org/linux-mm/202108160729.Lx0IJzq3-lkp@intel.com/
> > (csky gcc-11)
> >
> > >> csky-linux-gcc: error: unrecognized command-line option '--param=allow-store-data-races=0'; did you mean '--allow-store-data-races'?
> >
> > I wonder if cc-option detection for these is broken?
>
> I do not say it is broken...
>
>
> cc-option is defined like this:
>
> cc-option = $(call __cc-option, $(CC),\
> $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS),$(1),$(2))
>
>
> It is checking
> $(KBUILD_CPPFLAGS) + $(KBUILD_CFLAGS)
> + --allow-store-data-races
>
>
> A few lines above, I see
>
> csky-linux-gcc: error: unrecognized argument in option '-mcpu=ck860'
>
>
> It makes all the cc-option tests fail after this line:
> KBUILD_CFLAGS += -mcpu=$(CPUTYPE) -Wa,-mcpu=$(MCPU_STR)
>
>
> Then,
>
> $(call cc-option,--allow-store-data-races,--param=allow-store-data-races=0)
>
> falls back to --param=allow-store-data-races=0
>
>
>
>
> > Perhaps I should
> > not touch these other than to wrap them in the CONFIG_CC_IS_GCC guard?
>
> I do not think so.
>
> If an unrecognized argument appears,
> all the cc-option tests that follow are unreliable.
>
>
>
> If you are not comfortable with it,
> you can split it.
>
> KBUILD_CFLAGS += $(call cc-option,--allow-store-data-races)
> KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
I read the patch in the wrong way.
This is the same as the current code.
So, your v2 is fine with me.
I will pick it up soon.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-08-17 1:39 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10 20:42 [PATCH] Makefile: remove stale cc-option checks Nick Desaulniers
2021-08-10 23:01 ` Nathan Chancellor
2021-08-14 1:33 ` Masahiro Yamada
2021-08-11 10:41 ` Miguel Ojeda
2021-08-14 1:42 ` Masahiro Yamada
2021-08-16 18:35 ` Nick Desaulniers
2021-08-17 0:16 ` Masahiro Yamada
2021-08-17 1:38 ` Masahiro Yamada
2021-08-16 20:06 ` Nick Desaulniers
2021-08-16 20:11 ` Nick Desaulniers
2021-08-14 11:02 ` Naresh Kamboju
2021-08-16 18:37 ` Nick Desaulniers
2021-08-16 23:31 ` Stephen Rothwell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).