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
Yes, I agree that the title should contain the word 'fix' somewhere. For me its okay if maintainers reword the title when applying the patch, alternatively I can send a v2. (Is it still a v2 if I change the title, or rather a new patch?).
Any thoughts on whether this also needs a 'Cc stable'? Its not quite clear to me if this fix meets the requirements. As above, no objections if maintainers should decide to add it.
?
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!
- Best Valentin
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.
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