Re: [PATCH v2] powerpc: Pass correct CPU reference to assembler
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.
Re: [PATCH v2] powerpc: Pass correct CPU reference to assembler
On Friday 16 December 2022 12:10:48 Segher Boessenkool wrote:
> On Fri, Dec 16, 2022 at 05:57:46PM +, 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-p...@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
Re: [PATCH v2] powerpc: Pass correct CPU reference to assembler
On Fri, Dec 16, 2022 at 05:57:46PM +, 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
Re: [PATCH v2] powerpc: Pass correct CPU reference to assembler
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
Re: [PATCH v2] powerpc: Pass correct CPU reference to assembler
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
