[PATCH 1/2] powerpc/64: Move CPU -mtune options into Kconfig

[PATCH 1/2] powerpc/64: Move CPU -mtune options into Kconfig

[Michael Ellerman]

Currently the -mtune options are set in the Makefile, depending on what
is the compiler supports.

One downside of doing it that way is that the chosen -mtune option is
not recorded in the .config.

Another downside is that doing more complicated logic to calculate the
correct option gets messy in the Makefile.

So move the determination of which -mtune option to use into Kconfig
logic.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/Makefile                  | 4 +---
 arch/powerpc/platforms/Kconfig.cputype | 6 ++++++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 87d6ac27eebd..779956007f0c 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -156,9 +156,7 @@ endif
 CFLAGS-$(CONFIG_TARGET_CPU_BOOL) += -mcpu=$(CONFIG_TARGET_CPU)
 AFLAGS-$(CONFIG_TARGET_CPU_BOOL) += -mcpu=$(CONFIG_TARGET_CPU)
 
-CFLAGS-$(CONFIG_POWERPC64_CPU) += $(call cc-option,-mtune=power10,	\
-				  $(call cc-option,-mtune=power9,	\
-				  $(call cc-option,-mtune=power8)))
+CFLAGS-y += $(CONFIG_TUNE_CPU)
 
 asinstr := $(call as-instr,lis 9$(comma)foo@high,-DHAVE_AS_ATHIGH=1)
 
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 046b571496b1..7d7477b73951 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -273,6 +273,12 @@ config TARGET_CPU
 	default "e500mc" if E500MC_CPU
 	default "powerpc" if POWERPC_CPU
 
+config TUNE_CPU
+	string
+	default "-mtune=power10" if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power10)
+	default "-mtune=power9"  if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power9)
+	default "-mtune=power8"  if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power8)
+
 config PPC_BOOK3S
 	def_bool y
 	depends on PPC_BOOK3S_32 || PPC_BOOK3S_64
-- 
2.39.2

[PATCH 2/2] powerpc/64: Use -mtune=pwr10/9/8 for clang

[Michael Ellerman]

For the -mtune option clang doesn't accept power10/9/8, instead it
accepts pwr10/9/8. That will be fixed in future versions of clang, but
the kernel must support the clang versions in the wild.

So add support for the "pwr" spelling if clang is in use.

Reported-by: Nathan Chancellor <nathan@kernel.org>
BugLink: https://github.com/ClangBuiltLinux/linux/issues/1799
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/platforms/Kconfig.cputype | 4 ++++
 1 file changed, 4 insertions(+)

Need to confirm the clang <= 16 statement is correct.

diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 7d7477b73951..e4e0e81be7de 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -278,6 +278,10 @@ config TUNE_CPU
 	default "-mtune=power10" if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power10)
 	default "-mtune=power9"  if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power9)
 	default "-mtune=power8"  if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power8)
+	# clang <= 16 only supports the "pwr" names
+	default "-mtune=pwr10"   if POWERPC64_CPU && CC_IS_CLANG && $(cc-option,-mtune=pwr10)
+	default "-mtune=pwr9"    if POWERPC64_CPU && CC_IS_CLANG && $(cc-option,-mtune=pwr9)
+	default "-mtune=pwr8"    if POWERPC64_CPU && CC_IS_CLANG && $(cc-option,-mtune=pwr8)
 
 config PPC_BOOK3S
 	def_bool y
-- 
2.39.2

Re: [PATCH 1/2] powerpc/64: Move CPU -mtune options into Kconfig

[Nathan Chancellor]

On Fri, Mar 03, 2023 at 12:16:55AM +1100, Michael Ellerman wrote:
> Currently the -mtune options are set in the Makefile, depending on what
> is the compiler supports.
> 
> One downside of doing it that way is that the chosen -mtune option is
> not recorded in the .config.
> 
> Another downside is that doing more complicated logic to calculate the
> correct option gets messy in the Makefile.
> 
> So move the determination of which -mtune option to use into Kconfig
> logic.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  arch/powerpc/Makefile                  | 4 +---
>  arch/powerpc/platforms/Kconfig.cputype | 6 ++++++
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 87d6ac27eebd..779956007f0c 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -156,9 +156,7 @@ endif
>  CFLAGS-$(CONFIG_TARGET_CPU_BOOL) += -mcpu=$(CONFIG_TARGET_CPU)
>  AFLAGS-$(CONFIG_TARGET_CPU_BOOL) += -mcpu=$(CONFIG_TARGET_CPU)
>  
> -CFLAGS-$(CONFIG_POWERPC64_CPU) += $(call cc-option,-mtune=power10,	\
> -				  $(call cc-option,-mtune=power9,	\
> -				  $(call cc-option,-mtune=power8)))
> +CFLAGS-y += $(CONFIG_TUNE_CPU)
>  
>  asinstr := $(call as-instr,lis 9$(comma)foo@high,-DHAVE_AS_ATHIGH=1)
>  
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index 046b571496b1..7d7477b73951 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -273,6 +273,12 @@ config TARGET_CPU
>  	default "e500mc" if E500MC_CPU
>  	default "powerpc" if POWERPC_CPU
>  
> +config TUNE_CPU
> +	string
> +	default "-mtune=power10" if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power10)
> +	default "-mtune=power9"  if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power9)
> +	default "-mtune=power8"  if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power8)

Would it be cleaner to hoist the POWERPC64_CPU dependency?

config TUNE_CPU
	string
	default "-mtune=power10" if CC_IS_GCC   && $(cc-option,-mtune=power10)
	default "-mtune=power9"  if CC_IS_GCC   && $(cc-option,-mtune=power9)
	default "-mtune=power8"  if CC_IS_GCC   && $(cc-option,-mtune=power8)
	depends on POWERPC64_CPU

> +
>  config PPC_BOOK3S
>  	def_bool y
>  	depends on PPC_BOOK3S_32 || PPC_BOOK3S_64
> -- 
> 2.39.2
> 

Re: [PATCH 2/2] powerpc/64: Use -mtune=pwr10/9/8 for clang

[Nathan Chancellor]

Hi Michael,

Thanks for the workaround and sorry this has come to bite us :/

On Fri, Mar 03, 2023 at 12:16:56AM +1100, Michael Ellerman wrote:
> For the -mtune option clang doesn't accept power10/9/8, instead it
> accepts pwr10/9/8. That will be fixed in future versions of clang, but
> the kernel must support the clang versions in the wild.
> 
> So add support for the "pwr" spelling if clang is in use.
> 
> Reported-by: Nathan Chancellor <nathan@kernel.org>

I think that should actually be

Reported-by: Nick Desaulniers <ndesaulniers@google.com>

> BugLink: https://github.com/ClangBuiltLinux/linux/issues/1799
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  arch/powerpc/platforms/Kconfig.cputype | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> Need to confirm the clang <= 16 statement is correct.

Currently, this is indeed the case. It is possible that Nemanja's patch
will get applied to release/16.x before 16.0.0 final but it might not.
We can always update it later. I think we do want to push to get that
patch applied because I forgot that it is only in 16.0.0 that '-mtune'
starts to do something on PowerPC:

https://github.com/llvm/llvm-project/commit/1dc26b80b872a94c581549a21943756a8c3448a3

Prior to that change, '-mtune' was accepted but did nothing. It is only
once it was hooked up to the backend that we got the spew of warnings. I
think that warrants us trying to get Nemanja's patch into 16.0.0, which
may allow us to drop this workaround altogether...

> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index 7d7477b73951..e4e0e81be7de 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -278,6 +278,10 @@ config TUNE_CPU
>  	default "-mtune=power10" if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power10)
>  	default "-mtune=power9"  if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power9)
>  	default "-mtune=power8"  if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power8)
> +	# clang <= 16 only supports the "pwr" names
> +	default "-mtune=pwr10"   if POWERPC64_CPU && CC_IS_CLANG && $(cc-option,-mtune=pwr10)
> +	default "-mtune=pwr9"    if POWERPC64_CPU && CC_IS_CLANG && $(cc-option,-mtune=pwr9)
> +	default "-mtune=pwr8"    if POWERPC64_CPU && CC_IS_CLANG && $(cc-option,-mtune=pwr8)
>  
>  config PPC_BOOK3S
>  	def_bool y
> -- 
> 2.39.2
> 

Re: [PATCH 2/2] powerpc/64: Use -mtune=pwr10/9/8 for clang

[Michael Ellerman]

Nathan Chancellor <nathan@kernel.org> writes:
> Hi Michael,
>
> Thanks for the workaround and sorry this has come to bite us :/
>
> On Fri, Mar 03, 2023 at 12:16:56AM +1100, Michael Ellerman wrote:
>> For the -mtune option clang doesn't accept power10/9/8, instead it
>> accepts pwr10/9/8. That will be fixed in future versions of clang, but
>> the kernel must support the clang versions in the wild.
>> 
>> So add support for the "pwr" spelling if clang is in use.
>> 
>> Reported-by: Nathan Chancellor <nathan@kernel.org>
>
> I think that should actually be
>
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>

I guess yeah.

>> BugLink: https://github.com/ClangBuiltLinux/linux/issues/1799
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>

Thanks.

>> ---
>>  arch/powerpc/platforms/Kconfig.cputype | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> Need to confirm the clang <= 16 statement is correct.
>
> Currently, this is indeed the case. It is possible that Nemanja's patch
> will get applied to release/16.x before 16.0.0 final but it might not.
>
> We can always update it later. I think we do want to push to get that
> patch applied because I forgot that it is only in 16.0.0 that '-mtune'
> starts to do something on PowerPC:
>
> https://github.com/llvm/llvm-project/commit/1dc26b80b872a94c581549a21943756a8c3448a3
>
> Prior to that change, '-mtune' was accepted but did nothing. It is only
> once it was hooked up to the backend that we got the spew of warnings. I
> think that warrants us trying to get Nemanja's patch into 16.0.0, which
> may allow us to drop this workaround altogether...

Aha OK, I missed that the warning was new in 16.

I'll sit on this for now then until we know if that change will make it
into clang 16.

cheers

Re: [PATCH 1/2] powerpc/64: Move CPU -mtune options into Kconfig

[Michael Ellerman]

Nathan Chancellor <nathan@kernel.org> writes:
> On Fri, Mar 03, 2023 at 12:16:55AM +1100, Michael Ellerman wrote:
>> Currently the -mtune options are set in the Makefile, depending on what
>> is the compiler supports.
>> 
>> One downside of doing it that way is that the chosen -mtune option is
>> not recorded in the .config.
>> 
>> Another downside is that doing more complicated logic to calculate the
>> correct option gets messy in the Makefile.
>> 
>> So move the determination of which -mtune option to use into Kconfig
>> logic.
>> 
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>
>> ---
>>  arch/powerpc/Makefile                  | 4 +---
>>  arch/powerpc/platforms/Kconfig.cputype | 6 ++++++
>>  2 files changed, 7 insertions(+), 3 deletions(-)
>> 
>> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> index 87d6ac27eebd..779956007f0c 100644
>> --- a/arch/powerpc/Makefile
>> +++ b/arch/powerpc/Makefile
>> @@ -156,9 +156,7 @@ endif
>>  CFLAGS-$(CONFIG_TARGET_CPU_BOOL) += -mcpu=$(CONFIG_TARGET_CPU)
>>  AFLAGS-$(CONFIG_TARGET_CPU_BOOL) += -mcpu=$(CONFIG_TARGET_CPU)
>>  
>> -CFLAGS-$(CONFIG_POWERPC64_CPU) += $(call cc-option,-mtune=power10,	\
>> -				  $(call cc-option,-mtune=power9,	\
>> -				  $(call cc-option,-mtune=power8)))
>> +CFLAGS-y += $(CONFIG_TUNE_CPU)
>>  
>>  asinstr := $(call as-instr,lis 9$(comma)foo@high,-DHAVE_AS_ATHIGH=1)
>>  
>> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
>> index 046b571496b1..7d7477b73951 100644
>> --- a/arch/powerpc/platforms/Kconfig.cputype
>> +++ b/arch/powerpc/platforms/Kconfig.cputype
>> @@ -273,6 +273,12 @@ config TARGET_CPU
>>  	default "e500mc" if E500MC_CPU
>>  	default "powerpc" if POWERPC_CPU
>>  
>> +config TUNE_CPU
>> +	string
>> +	default "-mtune=power10" if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power10)
>> +	default "-mtune=power9"  if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power9)
>> +	default "-mtune=power8"  if POWERPC64_CPU && CC_IS_GCC   && $(cc-option,-mtune=power8)
>
> Would it be cleaner to hoist the POWERPC64_CPU dependency?

I was experimenting with some follow-on patches that add more cases for
other CPUs, but that got messy and it'll need a bit more work.

So for now yes I should just hoist that dependency.

cheers

> config TUNE_CPU
> 	string
> 	default "-mtune=power10" if CC_IS_GCC   && $(cc-option,-mtune=power10)
> 	default "-mtune=power9"  if CC_IS_GCC   && $(cc-option,-mtune=power9)
> 	default "-mtune=power8"  if CC_IS_GCC   && $(cc-option,-mtune=power8)
> 	depends on POWERPC64_CPU

Re: [PATCH 2/2] powerpc/64: Use -mtune=pwr10/9/8 for clang

[Nathan Chancellor]

On Fri, Mar 03, 2023 at 10:53:02AM +1100, Michael Ellerman wrote:
> Nathan Chancellor <nathan@kernel.org> writes:
> > Hi Michael,
> >
> > Thanks for the workaround and sorry this has come to bite us :/
> >
> > On Fri, Mar 03, 2023 at 12:16:56AM +1100, Michael Ellerman wrote:
> >> For the -mtune option clang doesn't accept power10/9/8, instead it
> >> accepts pwr10/9/8. That will be fixed in future versions of clang, but
> >> the kernel must support the clang versions in the wild.
> >> 
> >> So add support for the "pwr" spelling if clang is in use.
> >> 
> >> Reported-by: Nathan Chancellor <nathan@kernel.org>
> >
> > I think that should actually be
> >
> > Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> 
> I guess yeah.
> 
> >> BugLink: https://github.com/ClangBuiltLinux/linux/issues/1799
> >> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> >
> > Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> 
> Thanks.
> 
> >> ---
> >>  arch/powerpc/platforms/Kconfig.cputype | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >> 
> >> Need to confirm the clang <= 16 statement is correct.
> >
> > Currently, this is indeed the case. It is possible that Nemanja's patch
> > will get applied to release/16.x before 16.0.0 final but it might not.
> >
> > We can always update it later. I think we do want to push to get that
> > patch applied because I forgot that it is only in 16.0.0 that '-mtune'
> > starts to do something on PowerPC:
> >
> > https://github.com/llvm/llvm-project/commit/1dc26b80b872a94c581549a21943756a8c3448a3
> >
> > Prior to that change, '-mtune' was accepted but did nothing. It is only
> > once it was hooked up to the backend that we got the spew of warnings. I
> > think that warrants us trying to get Nemanja's patch into 16.0.0, which
> > may allow us to drop this workaround altogether...
> 
> Aha OK, I missed that the warning was new in 16.
> 
> I'll sit on this for now then until we know if that change will make it
> into clang 16.

It was merged into release/16.x a few hours ago:

https://github.com/llvm/llvm-project/issues/61128

https://github.com/llvm/llvm-project/commit/9b2e09e9fb1aa3bbe3668d7cc585188a3014d1b9

So I think this particular workaround is no longer needed :)

Cheers,
Nathan