The kselftests may be built in a couple different ways:
make LLVM=1
make CC=clang
In order to handle both cases, set LLVM=1 if CC=clang. That way,the rest
of lib.mk, and any Makefiles that include lib.mk, can base decisions
solely on whether or not LLVM is set.
Then, build upon that to disable a pair of clang warnings that are
already silenced on gcc.
Doing it this way is much better than the piecemeal approach that I
started with in [1] and [2]. Thanks to Nathan Chancellor for the patch
reviews that led to this approach.
Changes since the first version:
1) Wrote a detailed explanation for suppressing two clang warnings, in
both a lib.mk comment, and the commit description.
2) Added a Reviewed-by tag to the first patch.
[1] https://lore.kernel.org/[email protected]
[2] https://lore.kernel.org/[email protected]
John Hubbard (2):
selftests/lib.mk: handle both LLVM=1 and CC=clang builds
selftests/lib.mk: silence some clang warnings that gcc already ignores
tools/testing/selftests/lib.mk | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
base-commit: e0cce98fe279b64f4a7d81b7f5c3a23d80b92fbc
--
2.45.1
The kselftests may be built in a couple different ways:
make LLVM=1
make CC=clang
In order to handle both cases, set LLVM=1 if CC=clang. That way,the rest
of lib.mk, and any Makefiles that include lib.mk, can base decisions
solely on whether or not LLVM is set.
Cc: Nathan Chancellor <[email protected]>
Cc: Ryan Roberts <[email protected]>
Reviewed-by: Muhammad Usama Anjum <[email protected]>
Signed-off-by: John Hubbard <[email protected]>
---
tools/testing/selftests/lib.mk | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 429535816dbd..2902787b89b2 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -1,5 +1,17 @@
# This mimics the top-level Makefile. We do it explicitly here so that this
# Makefile can operate with or without the kbuild infrastructure.
+
+# The kselftests may be built in a couple different ways:
+# make LLVM=1
+# make CC=clang
+#
+# In order to handle both cases, set LLVM=1 if CC=clang. That way,the rest of
+# lib.mk, and any Makefiles that include lib.mk, can base decisions solely on
+# whether or not LLVM is set.
+ifeq ($(CC),clang)
+ LLVM := 1
+endif
+
ifneq ($(LLVM),)
ifneq ($(filter %/,$(LLVM)),)
LLVM_PREFIX := $(LLVM)
--
2.45.1
On Fri, May 31, 2024 at 11:37:50AM -0700, John Hubbard wrote:
> The kselftests may be built in a couple different ways:
> make LLVM=1
> make CC=clang
>
> In order to handle both cases, set LLVM=1 if CC=clang. That way,the rest
> of lib.mk, and any Makefiles that include lib.mk, can base decisions
> solely on whether or not LLVM is set.
ICBW but I believe there are still some architectures with clang but not
lld support where there's a use case for using CC=clang.
On 6/3/24 8:32 AM, Mark Brown wrote:
> On Fri, May 31, 2024 at 11:37:50AM -0700, John Hubbard wrote:
>> The kselftests may be built in a couple different ways:
>> make LLVM=1
>> make CC=clang
>>
>> In order to handle both cases, set LLVM=1 if CC=clang. That way,the rest
>> of lib.mk, and any Makefiles that include lib.mk, can base decisions
>> solely on whether or not LLVM is set.
>
> ICBW but I believe there are still some architectures with clang but not
> lld support where there's a use case for using CC=clang.
I'm inclined to wait for those to make themselves known... :)
thanks,
--
John Hubbard
NVIDIA
On 6/3/24 10:09 AM, John Hubbard wrote:
> On 6/3/24 8:32 AM, Mark Brown wrote:
>> On Fri, May 31, 2024 at 11:37:50AM -0700, John Hubbard wrote:
>>> The kselftests may be built in a couple different ways:
>>> make LLVM=1
>>> make CC=clang
>>>
>>> In order to handle both cases, set LLVM=1 if CC=clang. That way,the rest
>>> of lib.mk, and any Makefiles that include lib.mk, can base decisions
>>> solely on whether or not LLVM is set.
>>
>> ICBW but I believe there are still some architectures with clang but not
>> lld support where there's a use case for using CC=clang.
>
> I'm inclined to wait for those to make themselves known... :)
>
...but thinking about this some more, maybe this patch is actually
a Bad Idea. Because it is encouraging weirdness and divergence from
how kbuild does it. And kbuild is very clear [1]:
Building with LLVM
Invoke make via:
make LLVM=1
to compile for the host target. For cross compiling:
make LLVM=1 ARCH=arm64
The LLVM= argument
LLVM has substitutes for GNU binutils utilities. They can be enabled individually. The full list of supported make variables:
make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
LLVM=1 expands to the above.
[1] https://docs.kernel.org/kbuild/llvm.html
thanks,
--
John Hubbard
NVIDIA
On Mon, Jun 03, 2024 at 04:32:30PM +0100, Mark Brown wrote:
> On Fri, May 31, 2024 at 11:37:50AM -0700, John Hubbard wrote:
> > The kselftests may be built in a couple different ways:
> > make LLVM=1
> > make CC=clang
> >
> > In order to handle both cases, set LLVM=1 if CC=clang. That way,the rest
> > of lib.mk, and any Makefiles that include lib.mk, can base decisions
> > solely on whether or not LLVM is set.
>
> ICBW but I believe there are still some architectures with clang but not
> lld support where there's a use case for using CC=clang.
Does CC=clang even work for the selftests? lib.mk here uses 'CC :=' so
won't CC=clang get overridden to CC=$(CROSS_COMPILE)gcc?
Cheers,
Nathan
On 6/3/24 3:47 PM, Nathan Chancellor wrote:
> On Mon, Jun 03, 2024 at 04:32:30PM +0100, Mark Brown wrote:
>> On Fri, May 31, 2024 at 11:37:50AM -0700, John Hubbard wrote:
>>> The kselftests may be built in a couple different ways:
>>> make LLVM=1
>>> make CC=clang
>>>
>>> In order to handle both cases, set LLVM=1 if CC=clang. That way,the rest
>>> of lib.mk, and any Makefiles that include lib.mk, can base decisions
>>> solely on whether or not LLVM is set.
>>
>> ICBW but I believe there are still some architectures with clang but not
>> lld support where there's a use case for using CC=clang.
>
> Does CC=clang even work for the selftests? lib.mk here uses 'CC :=' so
> won't CC=clang get overridden to CC=$(CROSS_COMPILE)gcc?
>
I received a report that someone (I forget who or what) was definitely
attempting to just set CC=clang. But yes, it definitely doesn't work
properly for CROSS_COMPILE.
And the more we talk it through, the less I like this direction that
I went off on. Let's just drop this patch and instead consider moving
kselftest builds closer to kbuild, instead of making it more different.
thanks,
--
John Hubbard
NVIDIA
On Mon, Jun 03, 2024 at 03:47:06PM -0700, Nathan Chancellor wrote:
> On Mon, Jun 03, 2024 at 04:32:30PM +0100, Mark Brown wrote:
> > ICBW but I believe there are still some architectures with clang but not
> > lld support where there's a use case for using CC=clang.
> Does CC=clang even work for the selftests? lib.mk here uses 'CC :=' so
> won't CC=clang get overridden to CC=$(CROSS_COMPILE)gcc?
No idea, it's not a configuration I'm interested in myself.
On 04/06/2024 05:55, John Hubbard wrote:
> On 6/3/24 3:47 PM, Nathan Chancellor wrote:
>> On Mon, Jun 03, 2024 at 04:32:30PM +0100, Mark Brown wrote:
>>> On Fri, May 31, 2024 at 11:37:50AM -0700, John Hubbard wrote:
>>>> The kselftests may be built in a couple different ways:
>>>> make LLVM=1
>>>> make CC=clang
>>>>
>>>> In order to handle both cases, set LLVM=1 if CC=clang. That way,the rest
>>>> of lib.mk, and any Makefiles that include lib.mk, can base decisions
>>>> solely on whether or not LLVM is set.
>>>
>>> ICBW but I believe there are still some architectures with clang but not
>>> lld support where there's a use case for using CC=clang.
>>
>> Does CC=clang even work for the selftests? lib.mk here uses 'CC :=' so
>> won't CC=clang get overridden to CC=$(CROSS_COMPILE)gcc?
>>
>
> I received a report that someone (I forget who or what) was definitely
> attempting to just set CC=clang. But yes, it definitely doesn't work
> properly for CROSS_COMPILE.
This history as I recall, is that I got a bug report [1] stating that:
# tools/testing/selftests/fchmodat2$ make CC=clang
and
# tools/testing/selftests/openat2$ make CC=clang
were both failing due to the -static-libsan / -static-libasan difference between
gcc and clang. I attempted to fix that with [2], which used cc-option to
determine which variant to use. That never got picked up, and John
coincidentally did a similar fix, but relying on LLVM=1 instead.
If we are concluding that CC=clang is an invalid way to do this, then I guess we
should report that back to [1]?
[1] https://lore.kernel.org/all/[email protected]/
[2]
https://lore.kernel.org/linux-kselftest/[email protected]/
Thanks,
Ryan
>
> And the more we talk it through, the less I like this direction that
> I went off on. Let's just drop this patch and instead consider moving
> kselftest builds closer to kbuild, instead of making it more different.
>
>
> thanks,
On Fri, Jun 07, 2024 at 12:12:19PM +0100, Ryan Roberts wrote:
> On 04/06/2024 05:55, John Hubbard wrote:
> > On 6/3/24 3:47 PM, Nathan Chancellor wrote:
> >> Does CC=clang even work for the selftests? lib.mk here uses 'CC :=' so
> >> won't CC=clang get overridden to CC=$(CROSS_COMPILE)gcc?
> >>
> >
> > I received a report that someone (I forget who or what) was definitely
> > attempting to just set CC=clang. But yes, it definitely doesn't work
> > properly for CROSS_COMPILE.
>
> This history as I recall, is that I got a bug report [1] stating that:
>
> # tools/testing/selftests/fchmodat2$ make CC=clang
>
> and
>
> # tools/testing/selftests/openat2$ make CC=clang
>
> were both failing due to the -static-libsan / -static-libasan difference between
> gcc and clang. I attempted to fix that with [2], which used cc-option to
> determine which variant to use. That never got picked up, and John
> coincidentally did a similar fix, but relying on LLVM=1 instead.
>
> If we are concluding that CC=clang is an invalid way to do this, then I guess we
> should report that back to [1]?
>
> [1] https://lore.kernel.org/all/[email protected]/
> [2]
> https://lore.kernel.org/linux-kselftest/[email protected]/
I can only speak from the perspective of the main kernel build, as I
don't really know much if anything about the selftests, but I think
CC=clang and LLVM=1 should both be valid. Ideally, they would behave as
they do for the main kernel build (i.e., CC=clang just uses clang for
the compiler and LLVM=1 uses the entire LLVM tools). I realize that for
the selftests, there is probably little use for tools other than the
compiler, assembler, and linker but I think consistency is desirable
here.
I am not at all familiar with the selftests build system, which is
completely different from Kbuild, but I would ack a patch that does
that. Otherwise, I think having a different meaning or handling of
CC=clang or LLVM=1 is the end of the world, but I do think that it
should be documented.
Cheers,
Nathan
On 6/7/24 10:15 AM, Nathan Chancellor wrote:
> On Fri, Jun 07, 2024 at 12:12:19PM +0100, Ryan Roberts wrote:
>> On 04/06/2024 05:55, John Hubbard wrote:
>>> On 6/3/24 3:47 PM, Nathan Chancellor wrote:
...
>> If we are concluding that CC=clang is an invalid way to do this, then I guess we
>> should report that back to [1]?
>>
>> [1] https://lore.kernel.org/all/[email protected]/
>> [2]
>> https://lore.kernel.org/linux-kselftest/[email protected]/
>
> I can only speak from the perspective of the main kernel build, as I
> don't really know much if anything about the selftests, but I think
> CC=clang and LLVM=1 should both be valid. Ideally, they would behave as
> they do for the main kernel build (i.e., CC=clang just uses clang for
> the compiler and LLVM=1 uses the entire LLVM tools). I realize that for
> the selftests, there is probably little use for tools other than the
> compiler, assembler, and linker but I think consistency is desirable
> here.
>
> I am not at all familiar with the selftests build system, which is
> completely different from Kbuild, but I would ack a patch that does
> that. Otherwise, I think having a different meaning or handling of
> CC=clang or LLVM=1 is the end of the world, but I do think that it
> should be documented.
>
OK, that can be easily done, as shown below. And there are so far only
a handful of selftests that key off of LLVM (plus a few of my pending
patches). I can post this, plus a few patches (and patch updates for
pending patches) to use the new CC_IS_CLANG where appropriate:
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 429535816dbd..ea643d1a65dc 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -43,6 +43,15 @@ else
CC := $(CROSS_COMPILE)gcc
endif # LLVM
+# CC might have been set above (by inferring it from LLVM==1), or CC might have
+# been set from the environment. In either case, if CC is an invocation of clang
+# in any form, set CC_IS_CLANG. This allows subsystem selftests to selectively
+# control clang-specific behavior, such as, in particular, compiler warnings.
+CC_IS_CLANG := 0
+ifeq ($(findstring clang,$(CC)),clang)
+ CC_IS_CLANG := 1
+endif
+
ifeq (0,$(MAKELEVEL))
ifeq ($(OUTPUT),)
OUTPUT := $(shell pwd)
thanks,
--
John Hubbard
NVIDIA