Message ID | 20210513115904.519912-1-aik@ozlabs.ru (mailing list archive) |
---|---|
State | Rejected, archived |
Headers | show |
Series | [kernel,v3] powerpc/makefile: Do not redefine $(CPP) for preprocessor | expand |
Related | show |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (1fab3666d738e4af3a7450c44441310e4d7a7e53) |
snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
snowpatch_ozlabs/build-pmac32 | fail | Build failed! |
snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 1 warnings, 0 checks, 28 lines checked |
snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
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) 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, 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 >
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 >> >
On Fri, May 14, 2021 at 3:59 AM Nathan Chancellor <nathan@kernel.org> 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: I do not know how to reproduce this for powerpc. Currently, only x86 and arm64 select ARCH_SUPPORTS_LTO_CLANG. Is this a fix for a potential issue? > > 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) > > 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, maybe we should just add $(CLANG_FLAGS) to > KBUILD_CPPFLAGS instead of KBUILD_CFLAGS? Hmm, I think including --target=* in CPP flags is sensible, but not all CLANG_FLAGS are CPP flags. At least, -(no)-integrated-as is not a CPP flag. We could introduce a separate CLANG_CPP_FLAGS, but it would require more code changes... So, I do not have a strong opinion either way. BTW, another approach might be to modify the linker script. In my best guess, the reason why powerpc adding the endian flag to CPP is this line in arch/powerpc/kernel/vdso64/vdso64.lds.S #ifdef __LITTLE_ENDIAN__ OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle") #else OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc") #endif You can use the CONFIG option to check the endian-ness. #ifdef CONFIG_CPU_BIG_ENDIAN OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc") #else OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle") #endif All the big endian arches define CONFIG_CPU_BIG_ENDIAN. (but not all little endian arches define CONFIG_CPU_LITTLE_ENDIAN) So, #ifdef CONFIG_CPU_BIG_ENDIAN < big endian code > #else < little endian code > #endif works for all architectures. Only the exception is you cannot replace the one in uapi headers. arch/powerpc/include/uapi/asm/byteorder.h: #ifdef __LITTLE_ENDIAN__ since it is exported to userspace, where CONFIG options are not available. BTW, various flags are historically used. - CONFIG_CPU_BIG_ENDIAN / CONFIG_CPU_LITTLE_ENDIAN - __BIG_ENDIAN / __LITTLE_ENDIAN - __LITTLE_ENDIAN__ (powerpc only) __LITTLE_ENDIAN__ is defined by powerpc gcc and clang. My experiments... [1] powerpc-linux-gnu-gcc -> __BIG_ENDIAN__ is defined masahiro@grover:~$ echo | powerpc-linux-gnu-gcc -E -dM -x c - | grep ENDIAN #define __ORDER_LITTLE_ENDIAN__ 1234 #define __BIG_ENDIAN__ 1 #define __FLOAT_WORD_ORDER__ __ORDER_BIG_ENDIAN__ #define __ORDER_PDP_ENDIAN__ 3412 #define _BIG_ENDIAN 1 #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__ #define __VEC_ELEMENT_REG_ORDER__ __ORDER_BIG_ENDIAN__ #define __ORDER_BIG_ENDIAN__ 4321 [2] powerpc-linux-gnu-gcc + -mlittle-endian -> __LITTLE_ENDIAN__ is defined masahiro@grover:~$ echo | powerpc-linux-gnu-gcc -E -dM -x c - -mlittle-endian | grep ENDIAN #define __ORDER_LITTLE_ENDIAN__ 1234 #define _LITTLE_ENDIAN 1 #define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__ #define __ORDER_PDP_ENDIAN__ 3412 #define __LITTLE_ENDIAN__ 1 #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ #define __VEC_ELEMENT_REG_ORDER__ __ORDER_LITTLE_ENDIAN__ #define __ORDER_BIG_ENDIAN__ 4321 [3] other arch gcc -> neither of them is defined masahiro@grover:~$ echo | gcc -E -dM -x c - | grep ENDIAN #define __ORDER_LITTLE_ENDIAN__ 1234 #define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__ #define __ORDER_PDP_ENDIAN__ 3412 #define __ORDER_BIG_ENDIAN__ 4321 #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ masahiro@grover:~$ echo | arm-linux-gnueabihf-gcc -E -dM -x c - -mlittle-endian | grep ENDIAN #define __ORDER_LITTLE_ENDIAN__ 1234 #define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__ #define __ORDER_PDP_ENDIAN__ 3412 #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ #define __ORDER_BIG_ENDIAN__ 4321 masahiro@grover:~$ echo | arm-linux-gnueabihf-gcc -E -dM -x c - -mbig-endian | grep ENDIAN #define __ORDER_LITTLE_ENDIAN__ 1234 #define __FLOAT_WORD_ORDER__ __ORDER_BIG_ENDIAN__ #define __ORDER_PDP_ENDIAN__ 3412 #define __ARM_BIG_ENDIAN 1 #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__ #define __ORDER_BIG_ENDIAN__ 4321 [4] Clang --target=powerpc-linux-gnu -> __BIG_ENDIAN__ is defined masahiro@grover:~$ echo | ~/tools/clang-latest/bin/clang -E --target=powerpc-linux-gnu -dM -x c - | grep ENDIAN #define _BIG_ENDIAN 1 #define __BIG_ENDIAN__ 1 #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__ #define __ORDER_BIG_ENDIAN__ 4321 #define __ORDER_LITTLE_ENDIAN__ 1234 #define __ORDER_PDP_ENDIAN__ 3412 [5] very recent Clang understands --target=powerpcle-linux-gnu --> __LITTLE_ENDIAN__ is defined masahiro@grover:~$ echo | ~/tools/clang-latest/bin/clang -E --target=powerpcle-linux-gnu -dM -x c - | grep ENDIAN #define _LITTLE_ENDIAN 1 #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ #define __LITTLE_ENDIAN__ 1 #define __ORDER_BIG_ENDIAN__ 4321 #define __ORDER_LITTLE_ENDIAN__ 1234 #define __ORDER_PDP_ENDIAN__ 3412 [6] very recent Clang, --target=powerpc-linux-gnu + -mlittle-endian --> __LITTLE_ENDIAN__ is defined masahiro@grover:~$ echo | ~/tools/clang-latest/bin/clang -E --target=powerpc-linux-gnu -dM -x c - -mlittle-endian | grep ENDIAN #define _LITTLE_ENDIAN 1 #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ #define __LITTLE_ENDIAN__ 1 #define __ORDER_BIG_ENDIAN__ 4321 #define __ORDER_LITTLE_ENDIAN__ 1234 #define __ORDER_PDP_ENDIAN__ 3412 [7] Clang, target with little endian only , -mbig-endian is ignored masahiro@grover:~$ echo | clang -E -dM -x c - | grep ENDIAN #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ #define __LITTLE_ENDIAN__ 1 #define __ORDER_BIG_ENDIAN__ 4321 #define __ORDER_LITTLE_ENDIAN__ 1234 #define __ORDER_PDP_ENDIAN__ 3412 masahiro@grover:~$ echo | clang -E -dM -x c - -mbig-endian | grep ENDIAN #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ #define __LITTLE_ENDIAN__ 1 #define __ORDER_BIG_ENDIAN__ 4321 #define __ORDER_LITTLE_ENDIAN__ 1234 #define __ORDER_PDP_ENDIAN__ 3412 -- Best Regards Masahiro Yamada
On 14/05/2021 12:42, Masahiro Yamada wrote: > On Fri, May 14, 2021 at 3:59 AM Nathan Chancellor <nathan@kernel.org> 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: > > I do not know how to reproduce this for powerpc. > Currently, only x86 and arm64 select > ARCH_SUPPORTS_LTO_CLANG. > > Is this a fix for a potential issue? Yeah, it is work in progress to enable LTO_CLANG for PPC64: https://github.com/aik/linux/commits/lto > > >>> 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) >> >> 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, maybe we should just add $(CLANG_FLAGS) to >> KBUILD_CPPFLAGS instead of KBUILD_CFLAGS? > > Hmm, I think including --target=* in CPP flags is sensible, > but not all CLANG_FLAGS are CPP flags. > At least, -(no)-integrated-as is not a CPP flag. > > We could introduce a separate CLANG_CPP_FLAGS, but > it would require more code changes... > > So, I do not have a strong opinion either way. > > > > BTW, another approach might be to modify the linker script. > > > In my best guess, the reason why powerpc adding the endian flag to CPP > is this line in arch/powerpc/kernel/vdso64/vdso64.lds.S > > #ifdef __LITTLE_ENDIAN__ > OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle") > #else > OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc") > #endif > > > You can use the CONFIG option to check the endian-ness. > > #ifdef CONFIG_CPU_BIG_ENDIAN > OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc") > #else > OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle") > #endif > > > All the big endian arches define CONFIG_CPU_BIG_ENDIAN. > (but not all little endian arches define CONFIG_CPU_LITTLE_ENDIAN) This should work with .lds. But missing --target=* might still hit us somewhere else later, these include 3 header files each and there might be endianness dependent stuff. > > > So, > #ifdef CONFIG_CPU_BIG_ENDIAN > < big endian code > > #else > < little endian code > > #endif > > works for all architectures. > > > Only the exception is you cannot replace the one in uapi headers. > arch/powerpc/include/uapi/asm/byteorder.h: #ifdef __LITTLE_ENDIAN__ > since it is exported to userspace, where CONFIG options are not available. > > > > BTW, various flags are historically used. > > - CONFIG_CPU_BIG_ENDIAN / CONFIG_CPU_LITTLE_ENDIAN > - __BIG_ENDIAN / __LITTLE_ENDIAN > - __LITTLE_ENDIAN__ (powerpc only) > > > > __LITTLE_ENDIAN__ is defined by powerpc gcc and clang. > > My experiments... > > > [1] powerpc-linux-gnu-gcc -> __BIG_ENDIAN__ is defined > > masahiro@grover:~$ echo | powerpc-linux-gnu-gcc -E -dM -x c - | grep ENDIAN > #define __ORDER_LITTLE_ENDIAN__ 1234 > #define __BIG_ENDIAN__ 1 > #define __FLOAT_WORD_ORDER__ __ORDER_BIG_ENDIAN__ > #define __ORDER_PDP_ENDIAN__ 3412 > #define _BIG_ENDIAN 1 > #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__ > #define __VEC_ELEMENT_REG_ORDER__ __ORDER_BIG_ENDIAN__ > #define __ORDER_BIG_ENDIAN__ 4321 > > > [2] powerpc-linux-gnu-gcc + -mlittle-endian -> __LITTLE_ENDIAN__ is defined > > masahiro@grover:~$ echo | powerpc-linux-gnu-gcc -E -dM -x c - > -mlittle-endian | grep ENDIAN > #define __ORDER_LITTLE_ENDIAN__ 1234 > #define _LITTLE_ENDIAN 1 > #define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__ > #define __ORDER_PDP_ENDIAN__ 3412 > #define __LITTLE_ENDIAN__ 1 > #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ > #define __VEC_ELEMENT_REG_ORDER__ __ORDER_LITTLE_ENDIAN__ > #define __ORDER_BIG_ENDIAN__ 4321 > > > [3] other arch gcc -> neither of them is defined > > masahiro@grover:~$ echo | gcc -E -dM -x c - | grep ENDIAN > #define __ORDER_LITTLE_ENDIAN__ 1234 > #define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__ > #define __ORDER_PDP_ENDIAN__ 3412 > #define __ORDER_BIG_ENDIAN__ 4321 > #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ > > masahiro@grover:~$ echo | arm-linux-gnueabihf-gcc -E -dM -x c - > -mlittle-endian | grep ENDIAN > #define __ORDER_LITTLE_ENDIAN__ 1234 > #define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__ > #define __ORDER_PDP_ENDIAN__ 3412 > #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ > #define __ORDER_BIG_ENDIAN__ 4321 > > masahiro@grover:~$ echo | arm-linux-gnueabihf-gcc -E -dM -x c - > -mbig-endian | grep ENDIAN > #define __ORDER_LITTLE_ENDIAN__ 1234 > #define __FLOAT_WORD_ORDER__ __ORDER_BIG_ENDIAN__ > #define __ORDER_PDP_ENDIAN__ 3412 > #define __ARM_BIG_ENDIAN 1 > #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__ > #define __ORDER_BIG_ENDIAN__ 4321 > > > [4] Clang --target=powerpc-linux-gnu -> __BIG_ENDIAN__ is defined > > masahiro@grover:~$ echo | ~/tools/clang-latest/bin/clang -E > --target=powerpc-linux-gnu -dM -x c - | grep ENDIAN > #define _BIG_ENDIAN 1 > #define __BIG_ENDIAN__ 1 > #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__ > #define __ORDER_BIG_ENDIAN__ 4321 > #define __ORDER_LITTLE_ENDIAN__ 1234 > #define __ORDER_PDP_ENDIAN__ 3412 > > > > [5] very recent Clang understands --target=powerpcle-linux-gnu --> > __LITTLE_ENDIAN__ is defined > > masahiro@grover:~$ echo | ~/tools/clang-latest/bin/clang -E > --target=powerpcle-linux-gnu -dM -x c - | grep ENDIAN > #define _LITTLE_ENDIAN 1 > #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ > #define __LITTLE_ENDIAN__ 1 > #define __ORDER_BIG_ENDIAN__ 4321 > #define __ORDER_LITTLE_ENDIAN__ 1234 > #define __ORDER_PDP_ENDIAN__ 3412 > > > [6] very recent Clang, --target=powerpc-linux-gnu + -mlittle-endian > --> __LITTLE_ENDIAN__ is defined > > masahiro@grover:~$ echo | ~/tools/clang-latest/bin/clang -E > --target=powerpc-linux-gnu -dM -x c - -mlittle-endian | grep ENDIAN > #define _LITTLE_ENDIAN 1 > #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ > #define __LITTLE_ENDIAN__ 1 > #define __ORDER_BIG_ENDIAN__ 4321 > #define __ORDER_LITTLE_ENDIAN__ 1234 > #define __ORDER_PDP_ENDIAN__ 3412 > > > > > [7] Clang, target with little endian only , -mbig-endian is ignored > masahiro@grover:~$ echo | clang -E -dM -x c - | grep ENDIAN > #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ > #define __LITTLE_ENDIAN__ 1 > #define __ORDER_BIG_ENDIAN__ 4321 > #define __ORDER_LITTLE_ENDIAN__ 1234 > #define __ORDER_PDP_ENDIAN__ 3412 > masahiro@grover:~$ echo | clang -E -dM -x c - -mbig-endian | grep ENDIAN > #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ > #define __LITTLE_ENDIAN__ 1 > #define __ORDER_BIG_ENDIAN__ 4321 > #define __ORDER_LITTLE_ENDIAN__ 1234 > #define __ORDER_PDP_ENDIAN__ 3412 Nice research :) Thanks, > > > -- > Best Regards > Masahiro Yamada >
Hi! On Fri, May 14, 2021 at 11:42:32AM +0900, Masahiro Yamada wrote: > In my best guess, the reason why powerpc adding the endian flag to CPP > is this line in arch/powerpc/kernel/vdso64/vdso64.lds.S > > #ifdef __LITTLE_ENDIAN__ > OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle") > #else > OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc") > #endif Which is equivalent to #ifdef __LITTLE_ENDIAN__ OUTPUT_FORMAT("elf64-powerpcle") #else OUTPUT_FORMAT("elf64-powerpc") #endif so please change that at the same time if you touch this :-) > __LITTLE_ENDIAN__ is defined by powerpc gcc and clang. This predefined macro is required by the newer ABIs, but all older compilers have it as well. _LITTLE_ENDIAN is not supported on all platforms (but it is if your compiler targets Linux, which you cannot necessarily rely on). These macros are PowerPC-specific. For GCC, for all targets, you can say #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ You do not need any of the other *ORDER__ macros in most cases. See "info cpp" for the sordid details. > [2] powerpc-linux-gnu-gcc + -mlittle-endian -> __LITTLE_ENDIAN__ is defined You can just write -mbig and -mlittle btw. Those aren't available on all targets, but neither are the long-winded -m{big,little}-endian option names. Pet peeve, I know :-) HtH, Segher
On 5/14/21 18:46, Segher Boessenkool wrote: > Hi! > > On Fri, May 14, 2021 at 11:42:32AM +0900, Masahiro Yamada wrote: >> In my best guess, the reason why powerpc adding the endian flag to CPP >> is this line in arch/powerpc/kernel/vdso64/vdso64.lds.S >> >> #ifdef __LITTLE_ENDIAN__ >> OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle") >> #else >> OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc") >> #endif > > Which is equivalent to > > #ifdef __LITTLE_ENDIAN__ > OUTPUT_FORMAT("elf64-powerpcle") > #else > OUTPUT_FORMAT("elf64-powerpc") > #endif > > so please change that at the same time if you touch this :-) "If you touch this" approach did not work well with this patch so sorry but no ;) and for a separate patch, I'll have to dig since when it is equal, do you know? > >> __LITTLE_ENDIAN__ is defined by powerpc gcc and clang. > > This predefined macro is required by the newer ABIs, but all older That's good so I'll stick to it. > compilers have it as well. _LITTLE_ENDIAN is not supported on all > platforms (but it is if your compiler targets Linux, which you cannot > necessarily rely on). These macros are PowerPC-specific. > > For GCC, for all targets, you can say > #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ > You do not need any of the other *ORDER__ macros in most cases. > See "info cpp" for the sordid details. > >> [2] powerpc-linux-gnu-gcc + -mlittle-endian -> __LITTLE_ENDIAN__ is defined > > You can just write -mbig and -mlittle btw. Those aren't available on > all targets, but neither are the long-winded -m{big,little}-endian > option names. Pet peeve, I know :-) I am looking the same guarantees across modern enough gcc and clang and I am not sure all of the above is valid for clang 10.0.something (or whatever we say we support) ;)
Hi! On Mon, May 17, 2021 at 01:23:11PM +1000, Alexey Kardashevskiy wrote: > On 5/14/21 18:46, Segher Boessenkool wrote: > >On Fri, May 14, 2021 at 11:42:32AM +0900, Masahiro Yamada wrote: > >>In my best guess, the reason why powerpc adding the endian flag to CPP > >>is this line in arch/powerpc/kernel/vdso64/vdso64.lds.S > >> > >>#ifdef __LITTLE_ENDIAN__ > >>OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle") > >>#else > >>OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc") > >>#endif > > > >Which is equivalent to > > > >#ifdef __LITTLE_ENDIAN__ > >OUTPUT_FORMAT("elf64-powerpcle") > >#else > >OUTPUT_FORMAT("elf64-powerpc") > >#endif > > > >so please change that at the same time if you touch this :-) > > "If you touch this" approach did not work well with this patch so sorry > but no ;) > > and for a separate patch, I'll have to dig since when it is equal, do > you know? Since 1994, when the three-arg version was introduced (the one-arg version is from 1992). > >>__LITTLE_ENDIAN__ is defined by powerpc gcc and clang. > > > >This predefined macro is required by the newer ABIs, but all older > > That's good so I'll stick to it. Great. > >You can just write -mbig and -mlittle btw. Those aren't available on > >all targets, but neither are the long-winded -m{big,little}-endian > >option names. Pet peeve, I know :-) > > I am looking the same guarantees across modern enough gcc and clang and > I am not sure all of the above is valid for clang 10.0.something (or > whatever we say we support) ;) -mbig/-mlittle is supported in GCC since times immemorial. Whether LLVM supports it as well just depends on how good their emulation is, I have no idea. Segher
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:%-=%)) 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
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(-)