| Message ID | 01fe73614988e2402a7526fb6b6e903bc3777bb5.1671179743.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] powerpc: Pass correct CPU reference to assembler | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | fail | 2 of 6 jobs failed. |
Hi! On Fri, Dec 16, 2022 at 09:35:50AM +0100, Christophe Leroy wrote: > The problem comes from the fact that CONFIG_PPC_E500MC is selected for > both the e500mc (32 bits) and the e5500 (64 bits), and therefore the > following makefile rule is wrong: > > cpu-as-$(CONFIG_PPC_E500MC) += $(call as-option,-Wa$(comma)-me500mc) Yes. > Today we have CONFIG_TARGET_CPU which provides the identification of the > expected CPU, it is used for GCC. Use it as well for the assembler. Why do you use -Wa, at all for this? The compiler should already pass proper options always! > +cpu-as-$(CONFIG_PPC_BOOK3S_64) += $(call as-option,-Wa$(comma)-many) What is this for? Using -many is a huge step back, it hides many problems :-( Segher
CC Joel for the -many subject. Le 16/12/2022 à 18:18, Segher Boessenkool a écrit : > Hi! > > On Fri, Dec 16, 2022 at 09:35:50AM +0100, Christophe Leroy wrote: >> The problem comes from the fact that CONFIG_PPC_E500MC is selected for >> both the e500mc (32 bits) and the e5500 (64 bits), and therefore the >> following makefile rule is wrong: >> >> cpu-as-$(CONFIG_PPC_E500MC) += $(call as-option,-Wa$(comma)-me500mc) > > Yes. > >> Today we have CONFIG_TARGET_CPU which provides the identification of the >> expected CPU, it is used for GCC. Use it as well for the assembler. > > Why do you use -Wa, at all for this? The compiler should already pass > proper options always! That's historical I guess. Comes from commit 14cf11af6cf6 ("powerpc: Merge enough to start building in arch/powerpc.") > >> +cpu-as-$(CONFIG_PPC_BOOK3S_64) += $(call as-option,-Wa$(comma)-many) > > What is this for? Using -many is a huge step back, it hides many > problems :-( The only thing I did is removed the -Wa,-mpower4 from the line, leaving the remaining part. Initialy it was: cpu-as-$(CONFIG_PPC_BOOK3S_64) += $(call as-option,-Wa$(comma)-mpower4) $(call as-option,-Wa$(comma)-many) It was added in 2018 by commit 960e30029863 ("powerpc/Makefile: Fix PPC_BOOK3S_64 ASFLAGS"). There is a long explanation it the commit. Should we remove it ? Christophe
On Fri, Dec 16, 2022 at 05:57:46PM +0000, Christophe Leroy wrote: > Le 16/12/2022 à 18:18, Segher Boessenkool a écrit : > > On Fri, Dec 16, 2022 at 09:35:50AM +0100, Christophe Leroy wrote: > >> Today we have CONFIG_TARGET_CPU which provides the identification of the > >> expected CPU, it is used for GCC. Use it as well for the assembler. > > > > Why do you use -Wa, at all for this? The compiler should already pass > > proper options always! > > That's historical I guess. Comes from commit 14cf11af6cf6 ("powerpc: > Merge enough to start building in arch/powerpc.") Ah. The patch moves stuff around, I thought more of it is new than it really is. Sorry. It would be good to get rid of all such things that do no good and can easily cause problems, of course, but that does not belong to this patch of course. > >> +cpu-as-$(CONFIG_PPC_BOOK3S_64) += $(call as-option,-Wa$(comma)-many) > > > > What is this for? Using -many is a huge step back, it hides many > > problems :-( > > The only thing I did is removed the -Wa,-mpower4 from the line, leaving > the remaining part. Initialy it was: > > cpu-as-$(CONFIG_PPC_BOOK3S_64) += $(call as-option,-Wa$(comma)-mpower4) > $(call as-option,-Wa$(comma)-many) > > It was added in 2018 by commit 960e30029863 ("powerpc/Makefile: Fix > PPC_BOOK3S_64 ASFLAGS"). There is a long explanation it the commit. > > Should we remove it ? The commit says it is a workaround for clang problems, so it needs testing there. It also needs testing everywhere else, because as I said it hides a lot of problems, so removing it will make a lot of sloppy code that has crept in since 2018 scream bloody murder :-( Segher
On Friday 16 December 2022 12:10:48 Segher Boessenkool wrote: > On Fri, Dec 16, 2022 at 05:57:46PM +0000, Christophe Leroy wrote: > > Le 16/12/2022 à 18:18, Segher Boessenkool a écrit : > > > On Fri, Dec 16, 2022 at 09:35:50AM +0100, Christophe Leroy wrote: > > >> Today we have CONFIG_TARGET_CPU which provides the identification of the > > >> expected CPU, it is used for GCC. Use it as well for the assembler. > > > > > > Why do you use -Wa, at all for this? The compiler should already pass > > > proper options always! > > > > That's historical I guess. Comes from commit 14cf11af6cf6 ("powerpc: > > Merge enough to start building in arch/powerpc.") > > Ah. The patch moves stuff around, I thought more of it is new than it > really is. Sorry. > > It would be good to get rid of all such things that do no good and can > easily cause problems, of course, but that does not belong to this patch > of course. Just a coincident but u-boot has similar problem... https://patchwork.ozlabs.org/project/uboot/patch/20221211141204.8153-1-pali@kernel.org/ So I agree that removal of -Wa,-mXXX is a good idea. I checked that gcc pass correct -Wa,-mXXX flag from -mcpu=YYY flag. > > >> +cpu-as-$(CONFIG_PPC_BOOK3S_64) += $(call as-option,-Wa$(comma)-many) > > > > > > What is this for? Using -many is a huge step back, it hides many > > > problems :-( > > > > The only thing I did is removed the -Wa,-mpower4 from the line, leaving > > the remaining part. Initialy it was: > > > > cpu-as-$(CONFIG_PPC_BOOK3S_64) += $(call as-option,-Wa$(comma)-mpower4) > > $(call as-option,-Wa$(comma)-many) > > > > It was added in 2018 by commit 960e30029863 ("powerpc/Makefile: Fix > > PPC_BOOK3S_64 ASFLAGS"). There is a long explanation it the commit. > > > > Should we remove it ? > > The commit says it is a workaround for clang problems, so it needs > testing there. It also needs testing everywhere else, because as I said > it hides a lot of problems, so removing it will make a lot of sloppy > code that has crept in since 2018 scream bloody murder :-( > > > Segher
On Friday 16 December 2022 09:35:50 Christophe Leroy wrote: > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile > index bf5f0a998273..528452ce80b4 100644 > --- a/arch/powerpc/Makefile > +++ b/arch/powerpc/Makefile > @@ -201,18 +201,20 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables > # often slow when they are implemented at all > KBUILD_CFLAGS += $(call cc-option,-mno-string) > > -cpu-as-$(CONFIG_40x) += -Wa,-m405 > -cpu-as-$(CONFIG_44x) += -Wa,-m440 > cpu-as-$(CONFIG_ALTIVEC) += $(call as-option,-Wa$(comma)-maltivec) > -cpu-as-$(CONFIG_PPC_E500) += -Wa,-me500 > + > +ifeq ($(CONFIG_TARGET_CPU),powerpc) > +cpu-as-$(CONFIG_TARGET_CPU_BOOL) += -Wa,-mppc > +else > +cpu-as-$(CONFIG_TARGET_CPU_BOOL) += -Wa,-m$(CONFIG_TARGET_CPU) > +endif This change will break compilation for e500 cores. Kconfig sets CONFIG_TARGET_CPU to string "8540" for e500 cores because gcc uses -mcpu=8540 for e500 but GNU AS uses -me500 for e500 cores.
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index bf5f0a998273..528452ce80b4 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -201,18 +201,20 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables # often slow when they are implemented at all KBUILD_CFLAGS += $(call cc-option,-mno-string) -cpu-as-$(CONFIG_40x) += -Wa,-m405 -cpu-as-$(CONFIG_44x) += -Wa,-m440 cpu-as-$(CONFIG_ALTIVEC) += $(call as-option,-Wa$(comma)-maltivec) -cpu-as-$(CONFIG_PPC_E500) += -Wa,-me500 + +ifeq ($(CONFIG_TARGET_CPU),powerpc) +cpu-as-$(CONFIG_TARGET_CPU_BOOL) += -Wa,-mppc +else +cpu-as-$(CONFIG_TARGET_CPU_BOOL) += -Wa,-m$(CONFIG_TARGET_CPU) +endif # When using '-many -mpower4' gas will first try and find a matching power4 # mnemonic and failing that it will allow any valid mnemonic that GAS knows # about. GCC will pass -many to GAS when assembling, clang does not. # LLVM IAS doesn't understand either flag: https://github.com/ClangBuiltLinux/linux/issues/675 # but LLVM IAS only supports ISA >= 2.06 for Book3S 64 anyway... -cpu-as-$(CONFIG_PPC_BOOK3S_64) += $(call as-option,-Wa$(comma)-mpower4) $(call as-option,-Wa$(comma)-many) -cpu-as-$(CONFIG_PPC_E500MC) += $(call as-option,-Wa$(comma)-me500mc) +cpu-as-$(CONFIG_PPC_BOOK3S_64) += $(call as-option,-Wa$(comma)-many) KBUILD_AFLAGS += $(cpu-as-y) KBUILD_CFLAGS += $(cpu-as-y)
Jan-Benedict reported issue with building ppc64e_defconfig with mainline GCC work: powerpc64-linux-gcc -Wp,-MMD,arch/powerpc/kernel/vdso/.gettimeofday-64.o.d -nostdinc -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -D__KERNEL__ -I ./arch/powerpc -DHAVE_AS_ATHIGH=1 -fmacro-prefix-map=./= -D__ASSEMBLY__ -fno-PIE -m64 -Wl,-a64 -mabi=elfv1 -Wa,-me500 -Wa,-me500mc -mabi=elfv1 -mbig-endian -Wl,-soname=linux-vdso64.so.1 -D__VDSO64__ -s -c -o arch/powerpc/kernel/vdso/gettimeofday-64.o arch/powerpc/kernel/vdso/gettimeofday.S arch/powerpc/kernel/vdso/gettimeofday.S: Assembler messages: arch/powerpc/kernel/vdso/gettimeofday.S:72: Error: unrecognized opcode: `stdu' arch/powerpc/kernel/vdso/gettimeofday.S:72: Error: unrecognized opcode: `stdu' arch/powerpc/kernel/vdso/gettimeofday.S:72: Error: unrecognized opcode: `std' arch/powerpc/kernel/vdso/gettimeofday.S:72: Error: unrecognized opcode: `std' arch/powerpc/kernel/vdso/gettimeofday.S:72: Error: unrecognized opcode: `ld' arch/powerpc/kernel/vdso/gettimeofday.S:72: Error: unrecognized opcode: `ld' ... make[1]: *** [arch/powerpc/kernel/vdso/Makefile:76: arch/powerpc/kernel/vdso/gettimeofday-64.o] Error 1 make: *** [arch/powerpc/Makefile:387: vdso_prepare] Error 2 This is due to assembler being called with -me500mc which is a 32 bits target. The problem comes from the fact that CONFIG_PPC_E500MC is selected for both the e500mc (32 bits) and the e5500 (64 bits), and therefore the following makefile rule is wrong: cpu-as-$(CONFIG_PPC_E500MC) += $(call as-option,-Wa$(comma)-me500mc) Today we have CONFIG_TARGET_CPU which provides the identification of the expected CPU, it is used for GCC. Use it as well for the assembler. With that change (And also commit 825eada7717c ("powerpc/64: Set default CPU in Kconfig")), it now is: powerpc64-linux-gcc -Wp,-MMD,arch/powerpc/kernel/vdso/.gettimeofday-64.o.d -nostdinc -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -D__KERNEL__ -I ./arch/powerpc -DHAVE_AS_ATHIGH=1 -fmacro-prefix-map=./= -D__ASSEMBLY__ -fno-PIE -m64 -Wl,-a64 -mabi=elfv1 -mcpu=e500mc64 -Wa,-me500mc64 -mabi=elfv1 -mbig-endian -Wl,-soname=linux-vdso64.so.1 -D__VDSO64__ -s -c -o arch/powerpc/kernel/vdso/gettimeofday-64.o arch/powerpc/kernel/vdso/gettimeofday.S Reported-by: Jan-Benedict Glaw <jbglaw@lug-owl.de> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- v2: When target cpu is powerpc, the option to be used is -mppc Commit 825eada7717c is in powerpc/next-test branch. Make sure the SHA doesn't change when it goes into powerpc/next --- arch/powerpc/Makefile | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)