| Message ID | 20230302131656.50626-2-mpe@ellerman.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] powerpc/64: Move CPU -mtune options into Kconfig | expand |
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 >
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
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
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
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.