From: Alexey Kardashevskiy <aik@ozlabs.ru>
To: Nathan Chancellor <nathan@kernel.org>, linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
clang-built-linux@googlegroups.com,
Nick Desaulniers <ndesaulniers@google.com>,
Michal Marek <michal.lkml@markovi.net>,
Michael Ellerman <mpe@ellerman.id.au>,
Masahiro Yamada <masahiroy@kernel.org>,
Segher Boessenkool <segher@kernel.crashing.org>
Subject: Re: [PATCH kernel v3] powerpc/makefile: Do not redefine $(CPP) for preprocessor
Date: Fri, 14 May 2021 11:56:01 +1000 [thread overview]
Message-ID: <af1e3d74-a373-09ae-ba61-8db2a906d71a@ozlabs.ru> (raw)
In-Reply-To: <dedc7262-2956-37b2-ebfd-ae8eb9b56716@kernel.org>
On 14/05/2021 04:59, Nathan Chancellor wrote:
> On 5/13/2021 4:59 AM, Alexey Kardashevskiy wrote:
>> The $(CPP) (do only preprocessing) macro is already defined in Makefile.
>> However POWERPC redefines it and adds $(KBUILD_CFLAGS) which results
>> in flags duplication. Which is not a big deal by itself except for
>> the flags which depend on other flags and the compiler checks them
>> as it parses the command line.
>>
>> Specifically, scripts/Makefile.build:304 generates ksyms for .S files.
>> If clang+llvm+sanitizer are enabled, this results in
>>
>> -emit-llvm-bc -fno-lto -flto -fvisibility=hidden \
>> -fsanitize=cfi-mfcall -fno-lto ...
>>
>> in the clang command line and triggers error:
>>
>> clang-13: error: invalid argument '-fsanitize=cfi-mfcall' only allowed
>> with '-flto'
>>
>> This removes unnecessary CPP redefinition. Which works fine as in most
>> place KBUILD_CFLAGS is passed to $CPP except
>> arch/powerpc/kernel/vdso64/vdso(32|64).lds. To fix vdso, this does:
>> 1. add -m(big|little)-endian to $CPP
>> 2. add target to $KBUILD_CPPFLAGS as otherwise clang ignores
>> -m(big|little)-endian if
>> the building platform does not support big endian (such as x86).
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v3:
>> * moved vdso cleanup in a separate patch
>> * only add target to KBUILD_CPPFLAGS for CLANG
>>
>> v2:
>> * fix KBUILD_CPPFLAGS
>> * add CLANG_FLAGS to CPPFLAGS
>> ---
>> Makefile | 1 +
>> arch/powerpc/Makefile | 3 ++-
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 15b6476d0f89..5b545bef7653 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -576,6 +576,7 @@ CC_VERSION_TEXT = $(subst $(pound),,$(shell $(CC)
>> --version 2>/dev/null | head -
>> ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)
>> ifneq ($(CROSS_COMPILE),)
>> CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
>> +KBUILD_CPPFLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
>
> You can avoid the duplication here by just doing:
>
> KBUILD_CPPFLAGS += $(CLANG_FLAGS)
This has potential of duplicating even more flags which is exactly what
I am trying to avoid here.
> I am still not super happy about the flag duplication but I am not sure
> I can think of a better solution. If KBUILD_CPPFLAGS are always included
> when building .o files,
My understanding is that KBUILD_CPPFLAGS should not be added for .o. Who
does know or decide for sure about what CPPFLAGS are for? :)
> maybe we should just add $(CLANG_FLAGS) to
> KBUILD_CPPFLAGS instead of KBUILD_CFLAGS?
>
>> endif
>> ifeq ($(LLVM_IAS),1)
>> CLANG_FLAGS += -integrated-as
>> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> index 3212d076ac6a..306bfd2797ad 100644
>> --- a/arch/powerpc/Makefile
>> +++ b/arch/powerpc/Makefile
>> @@ -76,6 +76,7 @@ endif
>> ifdef CONFIG_CPU_LITTLE_ENDIAN
>> KBUILD_CFLAGS += -mlittle-endian
>> +KBUILD_CPPFLAGS += -mlittle-endian
>> KBUILD_LDFLAGS += -EL
>> LDEMULATION := lppc
>> GNUTARGET := powerpcle
>> @@ -83,6 +84,7 @@ MULTIPLEWORD := -mno-multiple
>> KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect)
>> else
>> KBUILD_CFLAGS += $(call cc-option,-mbig-endian)
>> +KBUILD_CPPFLAGS += $(call cc-option,-mbig-endian)
>> KBUILD_LDFLAGS += -EB
>> LDEMULATION := ppc
>> GNUTARGET := powerpc
>> @@ -208,7 +210,6 @@ KBUILD_CPPFLAGS += -I $(srctree)/arch/$(ARCH)
>> $(asinstr)
>> KBUILD_AFLAGS += $(AFLAGS-y)
>> KBUILD_CFLAGS += $(call cc-option,-msoft-float)
>> KBUILD_CFLAGS += -pipe $(CFLAGS-y)
>> -CPP = $(CC) -E $(KBUILD_CFLAGS)
>> CHECKFLAGS += -m$(BITS) -D__powerpc__ -D__powerpc$(BITS)__
>> ifdef CONFIG_CPU_BIG_ENDIAN
>>
>
--
Alexey
next prev parent reply other threads:[~2021-05-14 1:56 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-13 11:59 [PATCH kernel v3] powerpc/makefile: Do not redefine $(CPP) for preprocessor Alexey Kardashevskiy
2021-05-13 18:59 ` Nathan Chancellor
2021-05-14 1:56 ` Alexey Kardashevskiy [this message]
2021-05-14 2:42 ` Masahiro Yamada
2021-05-14 3:41 ` Alexey Kardashevskiy
2021-05-14 8:46 ` Segher Boessenkool
2021-05-17 3:23 ` Alexey Kardashevskiy
2021-05-17 19:10 ` Segher Boessenkool
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=af1e3d74-a373-09ae-ba61-8db2a906d71a@ozlabs.ru \
--to=aik@ozlabs.ru \
--cc=clang-built-linux@googlegroups.com \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=masahiroy@kernel.org \
--cc=michal.lkml@markovi.net \
--cc=mpe@ellerman.id.au \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=segher@kernel.crashing.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).