2020-03-10 10:14:27

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 1/2] kconfig: introduce m32-flag and m64-flag

When a compiler supports multiple architectures, some compiler features
can be dependent on the target architecture.

This is typical for Clang, which supports multiple LLVM backends.
Even for GCC, we need to take care of biarch compiler cases.

It is not a problem when we evaluate cc-option in Makefiles because
cc-option is tested against the flag in question + $(KBUILD_CFLAGS).

The cc-option in Kconfig, on the other hand, does not accumulate
tested flags. Due to this simplification, it could potentially test
cc-option against a different target.

At first, Kconfig always evaluated cc-option against the host
architecture.

Since commit e8de12fb7cde ("kbuild: Check for unknown options with
cc-option usage in Kconfig and clang"), in case of cross-compiling
with Clang, the target triple is correctly passed to Kconfig.

The case with biarch GCC (and native build with Clang) is still not
handled properly. We need to pass some flags to specify the target
machine bit.

Due to the design, all the macros in Kconfig are expanded in the
parse stage, where we do not know the target bit size yet.

For example, arch/x86/Kconfig allows a user to toggle CONFIG_64BIT.
If a compiler flag -foo depends on the machine bit, it must be tested
twice, one with -m32 and the other with -m64.

However, -m32/-m64 are not always recognized. So, this commits adds
m64-flag and m32-flag macros. They expand to -m32, -m64, respectively
if supported. Or, they expand to an empty string if unsupported.

The typical usage is like this:

config FOO
bool
default $(cc-option,$(m64-flag) -foo) if 64BIT
default $(cc-option,$(m32-flag) -foo)

This is clumsy, but there is no elegant way to handle this in the
current static macro expansion.

There was discussion for static functions vs dynamic functions.
The consensus was to go as far as possible with the static functions.
(https://lkml.org/lkml/2018/3/2/22)

Signed-off-by: Masahiro Yamada <[email protected]>
---

scripts/Kconfig.include | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
index 85334dc8c997..496d11c92c97 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -44,3 +44,10 @@ $(error-if,$(success, $(LD) -v | grep -q gold), gold linker '$(LD)' not supporte

# gcc version including patch level
gcc-version := $(shell,$(srctree)/scripts/gcc-version.sh $(CC))
+
+# machine bit flags
+# $(m32-flag): -m32 if the compiler supports it, or an empty string otherwise.
+# $(m64-flag): -m64 if the compiler supports it, or an empty string otherwise.
+cc-option-bit = $(if-success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null,$(1))
+m32-flag := $(cc-option-bit,-m32)
+m64-flag := $(cc-option-bit,-m64)
--
2.17.1


2020-03-10 10:16:56

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 2/2] int128: fix __uint128_t compiler test in Kconfig

The support for __uint128_t is dependent on the target bit size.

GCC that defaults to the 32-bit can still build the 64-bit kernel
with -m64 flag passed.

However, $(cc-option,-D__SIZEOF_INT128__=0) is evaluated against the
default machine bit, which may not match to the kernel it is building.

Theoretically, this could be evaluated separately for 64BIT/32BIT.

config CC_HAS_INT128
bool
default !$(cc-option,$(m64-flag) -D__SIZEOF_INT128__=0) if 64BIT
default !$(cc-option,$(m32-flag) -D__SIZEOF_INT128__=0)

I simplified it more because the 32-bit compiler is unlikely to support
__uint128_t.

Fixes: c12d3362a74b ("int128: move __uint128_t compiler test to Kconfig")
Reported-by: George Spelvin <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---

init/Kconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 20a6ac33761c..4f717bfdbfe2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -767,8 +767,7 @@ config ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
bool

config CC_HAS_INT128
- def_bool y
- depends on !$(cc-option,-D__SIZEOF_INT128__=0)
+ def_bool !$(cc-option,$(m64-flag) -D__SIZEOF_INT128__=0) && 64BIT

#
# For architectures that know their GCC __int128 support is sound
--
2.17.1

2020-03-10 11:05:14

by George Spelvin

[permalink] [raw]
Subject: Re: [PATCH 2/2] int128: fix __uint128_t compiler test in Kconfig

Thank you! As your 1/2 commit message explained, the best way to fix
this was not entirely obvious.

FWIW, this series
Tested-by: George Spelvin <[email protected]>

2020-03-10 15:48:11

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: introduce m32-flag and m64-flag

On Tue, Mar 10, 2020 at 07:12:49PM +0900, Masahiro Yamada wrote:
> When a compiler supports multiple architectures, some compiler features
> can be dependent on the target architecture.
>
> This is typical for Clang, which supports multiple LLVM backends.
> Even for GCC, we need to take care of biarch compiler cases.
>
> It is not a problem when we evaluate cc-option in Makefiles because
> cc-option is tested against the flag in question + $(KBUILD_CFLAGS).
>
> The cc-option in Kconfig, on the other hand, does not accumulate
> tested flags. Due to this simplification, it could potentially test
> cc-option against a different target.
>
> At first, Kconfig always evaluated cc-option against the host
> architecture.
>
> Since commit e8de12fb7cde ("kbuild: Check for unknown options with
> cc-option usage in Kconfig and clang"), in case of cross-compiling
> with Clang, the target triple is correctly passed to Kconfig.
>
> The case with biarch GCC (and native build with Clang) is still not
> handled properly. We need to pass some flags to specify the target
> machine bit.
>
> Due to the design, all the macros in Kconfig are expanded in the
> parse stage, where we do not know the target bit size yet.
>
> For example, arch/x86/Kconfig allows a user to toggle CONFIG_64BIT.
> If a compiler flag -foo depends on the machine bit, it must be tested
> twice, one with -m32 and the other with -m64.
>
> However, -m32/-m64 are not always recognized. So, this commits adds
> m64-flag and m32-flag macros. They expand to -m32, -m64, respectively
> if supported. Or, they expand to an empty string if unsupported.
>
> The typical usage is like this:
>
> config FOO
> bool
> default $(cc-option,$(m64-flag) -foo) if 64BIT
> default $(cc-option,$(m32-flag) -foo)
>
> This is clumsy, but there is no elegant way to handle this in the
> current static macro expansion.
>
> There was discussion for static functions vs dynamic functions.
> The consensus was to go as far as possible with the static functions.
> (https://lkml.org/lkml/2018/3/2/22)
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> scripts/Kconfig.include | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
> index 85334dc8c997..496d11c92c97 100644
> --- a/scripts/Kconfig.include
> +++ b/scripts/Kconfig.include
> @@ -44,3 +44,10 @@ $(error-if,$(success, $(LD) -v | grep -q gold), gold linker '$(LD)' not supporte
>
> # gcc version including patch level
> gcc-version := $(shell,$(srctree)/scripts/gcc-version.sh $(CC))
> +
> +# machine bit flags
> +# $(m32-flag): -m32 if the compiler supports it, or an empty string otherwise.
> +# $(m64-flag): -m64 if the compiler supports it, or an empty string otherwise.
> +cc-option-bit = $(if-success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null,$(1))
> +m32-flag := $(cc-option-bit,-m32)
> +m64-flag := $(cc-option-bit,-m64)
> --
> 2.17.1

Reviewed-by: Nathan Chancellor <[email protected]>