[PATCH kernel v3] powerpc/makefile: Do not redefine $(CPP) for preprocessor
Alexey Kardashevskiy
aik at ozlabs.ru
Fri May 14 13:41:00 AEST 2021
On 14/05/2021 12:42, Masahiro Yamada wrote:
> On Fri, May 14, 2021 at 3:59 AM Nathan Chancellor <nathan at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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
>
--
Alexey
More information about the Linuxppc-dev
mailing list