On 3/29/24 3:49 AM, Valentin Obst wrote:
Align the behavior for gcc and clang builds by interpreting unset
`ARCH` and `CROSS_COMPILE` variables in `LLVM` builds as a sign that the
user wants to build for the host architecture.
This patch preserves the properties that setting the `ARCH` variable to an
unknown value will trigger an error that complains about insufficient
information, and that a set `CROSS_COMPILE` variable will override the
target triple that is determined based on presence/absence of `ARCH`.
When compiling with clang, i.e., `LLVM` is set, an unset `ARCH` variable in
combination with an unset `CROSS_COMPILE` variable, i.e., compiling for
the host architecture, leads to compilation failures since `lib.mk` can
not determine the clang target triple. In this case, the following error
message is displayed for each subsystem that does not set `ARCH` in its
own Makefile before including `lib.mk` (lines wrapped at 75 chrs):
make[1]: Entering directory '/mnt/build/linux/tools/testing/selftests/
sysctl'
../lib.mk:33: *** Specify CROSS_COMPILE or add '--target=' option to
lib.mk. Stop.
make[1]: Leaving directory '/mnt/build/linux/tools/testing/selftests/
sysctl'
Thanks for fixing this.
And yes, the selftests "normal" (non-cross-compile) build is *broken*
right now, for clang. I didn't realize from the patch title that this is
actually a significant fix. Maybe we should change the subject line (patch
title) to something like:
[PATCH] selftests: fix the clang build: default to host arch for LLVM builds
?
Just a thought. The "Fixes:" tag covers it already, I realize.
Anyway, this looks correct, and fixes that aspect of the build for me, so
either way, please feel free to add:
Reviewed-by: John Hubbard jhubbard@nvidia.com
thanks,
--
John Hubbard
NVIDIA
>
> In the same scenario a gcc build would default to the host architecture,
> i.e., it would use plain `gcc`.
>
> Fixes: 795285ef2425 ("selftests: Fix clang cross compilation")
> Reviewed-by: Mark Brown
broonie@kernel.org
> Signed-off-by: Valentin Obst
kernel@valentinobst.de
> ---
> I am not entirely sure whether this behavior is in fact known and intended
> and whether the way to obtain the host target triple is sufficiently
> general. The flag was introduced in llvm-8 with [1], it will be an error in
> older clang versions.
>
> The target triple you get with `-print-target-triple` may not be the
> same that you would get when explicitly setting ARCH to you host
> architecture. For example on my x86_64 system it get
> `x86_64-pc-linux-gnu` instead of `x86_64-linux-gnu`, similar deviations
> were observed when testing other clang binaries on compiler-explorer,
> e.g., [2].
>
> An alternative could be to simply do:
>
> ARCH ?= $(shell uname -m)
>
> before using it to select the target. Possibly with some post processing,
> but at that point we would likely be replicating `scripts/subarch.include`.
> This is what some subsystem Makefiles do before including `lib.mk`. This
> change might make it possible to remove the explicit setting of `ARCH` from
> the few subsystem Makefiles that do it.
>
> [1]:
https://reviews.llvm.org/D50755
> [2]:
https://godbolt.org/z/r7Gn9bvv1
>
> Changes in v1:
> - Shortened commit message.
> - Link to RFC:
https://lore.kernel.org/r/20240303-selftests-libmk-llvm-rfc-v1-1-9ab53e365e3...
> ---
> tools/testing/selftests/lib.mk | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index da2cade3bab0..8ae203d8ed7f 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -7,6 +7,8 @@ else ifneq ($(filter -%,$(LLVM)),)
> LLVM_SUFFIX := $(LLVM)
> endif
>
> +CLANG := $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
> +
> CLANG_TARGET_FLAGS_arm := arm-linux-gnueabi
> CLANG_TARGET_FLAGS_arm64 := aarch64-linux-gnu
> CLANG_TARGET_FLAGS_hexagon := hexagon-linux-musl
> @@ -18,7 +20,13 @@ CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu
> CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu
> CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu
> CLANG_TARGET_FLAGS_x86_64 := x86_64-linux-gnu
> -CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(ARCH))
> +
> +# Default to host architecture if ARCH is not explicitly given.
> +ifeq ($(ARCH),)
> +CLANG_TARGET_FLAGS := $(shell $(CLANG) -print-target-triple)
> +else
> +CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(ARCH))
> +endif
>
> ifeq ($(CROSS_COMPILE),)
> ifeq ($(CLANG_TARGET_FLAGS),)
> @@ -30,7 +38,7 @@ else
> CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
> endif # CROSS_COMPILE
>
> -CC := $(LLVM_PREFIX)clang$(LLVM_SUFFIX) $(CLANG_FLAGS) -fintegrated-as
> +CC := $(CLANG) $(CLANG_FLAGS) -fintegrated-as
> else
> CC := $(CROSS_COMPILE)gcc
> endif # LLVM
>
> ---
> base-commit: 4cece764965020c22cff7665b18a012006359095
> change-id: 20240303-selftests-libmk-llvm-rfc-5fe3cfa9f094
>
> Best regards,
> --
> Valentin Obst
kernel@valentinobst.de
>
