On Fri, Mar 4, 2022 at 1:30 AM Nathan Chancellor nathan@kernel.org wrote:
Hi David,
On Thu, Mar 03, 2022 at 05:06:42PM +0800, David Gow wrote:
The things built with USER_CFLAGS don't seem to recognise it as a compiler option, and print a warning: clang: warning: argument unused during compilation: '-mno-global-merge' [-Wunused-command-line-argument]
Fixes: 744814d2fa ("um: Allow builds with Clang") Signed-off-by: David Gow davidgow@google.com
This warning shows up after merging: https://lore.kernel.org/lkml/20220227184517.504931-6-keescook@chromium.org/
I'm not 100% sure why this is necessary, but it does seem to work. All the attempts to get rid of -mno-global-merge entirely have been met with skepticism, but I'm guessing that it's not a problem for just the UML "user" files, as they shouldn't(?) interact too much with modules.
Thank you for the patch! I think it is correct, as this flag only works for AArch64 and ARM, as it is only used in Clang::AddAArch64TargetArgs() and Clang::AddARMTargetArgs() in clang/lib/Driver/ToolChains/Clang.cpp, which are obviously never called with UML. I am not sure why we do not see warning during regular kernel builds, maybe something about how UML objects are compiled exposes this?
Yeah: this only seems to show up with UML source compiled as "user" files (i.e., talking to the host system). Most of the kernel source doesn't show the warning because -Qunused-arguments is set somewhere, probably in KBUILD_CPPFLAGS, which doesn't filter down into USER_CFLAGS: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Make...
Is this flag supposed to only work on ARM/AArch64, or is it just currently not implemented anywhere else. If it's actually ARM-specific, then moving it to the arch/arm{,64} makefile as in Kees' patch definitely makes more sense. If it's going to show up on other architectures at some point anyway, maybe something like this makes sense.
Regardless, I would definitely like to clean up this instance of the warning because I would like to make this warning a hard error so that we do not get cryptic cc-option failures:
https://github.com/ClangBuiltLinux/linux/issues/1587
Reviewed-by: Nathan Chancellor nathan@kernel.org
One small comment below.
arch/um/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/um/Makefile b/arch/um/Makefile index f2fe63bfd819..320b09cd513c 100644 --- a/arch/um/Makefile +++ b/arch/um/Makefile @@ -75,6 +75,10 @@ USER_CFLAGS = $(patsubst $(KERNEL_DEFINES),,$(patsubst -I%,,$(KBUILD_CFLAGS))) \ -D_FILE_OFFSET_BITS=64 -idirafter $(srctree)/include \ -idirafter $(objtree)/include -D__KERNEL__ -D__UM_HOST__
+ifdef CONFIG_CC_IS_CLANG
Is this ifdef needed?
No: it works fine without the ifdef: I just wanted to limit the damage I could do playing with things I didn't fully understand. :-)
+USER_CFLAGS := $(patsubst -mno-global-merge,,$(USER_CFLAGS)) +endif
#This will adjust *FLAGS accordingly to the platform. include $(srctree)/$(ARCH_DIR)/Makefile-os-$(OS)
Cheers, -- David