[PATCH 0/2] Kconfig: -O3 enablement

[PATCH 0/2] Kconfig: -O3 enablement

[Miko Larsson]

Hi,

This very small series allows -O3 to be used for all architectures. The
first patch marks -O3 as experimental, with the reasoning being that it
might expose unwanted regressions to users, and the second patch
actually allows -O3 by removing the "depend on ARC" string.

The reasoning behind this series is to open up -O3 so that bugs related
to it (both compiler-related and kernel-related) can be discovered by
eyeballs wanting to improve the "-O3 experience," as that might be
beneficial to both compilers and the kernel. This has been attempted
before [1], but unfortunately nothing ever came of it.

[1] https://lore.kernel.org/lkml/20191211104619.114557-1-oleksandr@redhat.com/

Cc: linux-kbuild@vger.kernel.org
Cc: x86@kernel.org
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Sean Christopherson <seanjc@google.com>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Chris Down <chris@chrisdown.name>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: Elliot Berman <quic_eberman@quicinc.com>
Cc: Oleksandr Natalenko <oleksandr@redhat.com>

Miko Larsson (2):
  Kconfig: Mark -O3 as experimental
  Kconfig: Allow -O3 for all architectures

 init/Kconfig | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.36.1

[PATCH 1/2] Kconfig: Mark -O3 as experimental

[Miko Larsson]

Mark -O3 as experimental, as it might cause unwanted regressions for
users.

Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com>
---
 init/Kconfig | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/init/Kconfig b/init/Kconfig
index c7900e897..9c292acb2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1402,12 +1402,15 @@ config CC_OPTIMIZE_FOR_PERFORMANCE
 	  helpful compile-time warnings.
 
 config CC_OPTIMIZE_FOR_PERFORMANCE_O3
-	bool "Optimize more for performance (-O3)"
+	bool "Optimize more for performance (-O3) (EXPERIMENTAL)"
 	depends on ARC
 	help
 	  Choosing this option will pass "-O3" to your compiler to optimize
 	  the kernel yet more for performance.
 
+	  This option is EXPERIMENTAL; you may encounter compiler bugs and/or
+	  kernel bugs with this option enabled.
+
 config CC_OPTIMIZE_FOR_SIZE
 	bool "Optimize for size (-Os)"
 	help
-- 
2.36.1

[PATCH 2/2] Kconfig: Allow -O3 for all architectures

[Miko Larsson]

This commit allows all architectures to use the experimental -O3
optimization option. Previously, only ARC was allowed to use this
option.

Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com>
---
 init/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/init/Kconfig b/init/Kconfig
index 9c292acb2..b88613cb5 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1403,7 +1403,6 @@ config CC_OPTIMIZE_FOR_PERFORMANCE
 
 config CC_OPTIMIZE_FOR_PERFORMANCE_O3
 	bool "Optimize more for performance (-O3) (EXPERIMENTAL)"
-	depends on ARC
 	help
 	  Choosing this option will pass "-O3" to your compiler to optimize
 	  the kernel yet more for performance.
-- 
2.36.1

Re: [PATCH 0/2] Kconfig: -O3 enablement

[Nick Desaulniers]

On Tue, Jun 21, 2022 at 6:35 AM Miko Larsson <mikoxyzzz@gmail.com> wrote:
>
> Hi,
>
> This very small series allows -O3 to be used for all architectures. The
> first patch marks -O3 as experimental, with the reasoning being that it
> might expose unwanted regressions to users, and the second patch
> actually allows -O3 by removing the "depend on ARC" string.

I think we should just remove -O3 support from KCONFIG.

If someone wants to mess around with "experimental features," there's
nothing stopping you from doing:

$ make KCFLAGS=-O3

>
> The reasoning behind this series is to open up -O3 so that bugs related
> to it (both compiler-related and kernel-related) can be discovered by
> eyeballs wanting to improve the "-O3 experience," as that might be
> beneficial to both compilers and the kernel. This has been attempted
> before [1], but unfortunately nothing ever came of it.
>
> [1] https://lore.kernel.org/lkml/20191211104619.114557-1-oleksandr@redhat.com/
>
> Cc: linux-kbuild@vger.kernel.org
> Cc: x86@kernel.org
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Chris Down <chris@chrisdown.name>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: John Ogness <john.ogness@linutronix.de>
> Cc: Elliot Berman <quic_eberman@quicinc.com>
> Cc: Oleksandr Natalenko <oleksandr@redhat.com>
>
> Miko Larsson (2):
>   Kconfig: Mark -O3 as experimental
>   Kconfig: Allow -O3 for all architectures
>
>  init/Kconfig | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> --
> 2.36.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 0/2] Kconfig: -O3 enablement

[Masahiro Yamada]

On Wed, Jun 22, 2022 at 1:17 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Tue, Jun 21, 2022 at 6:35 AM Miko Larsson <mikoxyzzz@gmail.com> wrote:
> >
> > Hi,
> >
> > This very small series allows -O3 to be used for all architectures. The
> > first patch marks -O3 as experimental, with the reasoning being that it
> > might expose unwanted regressions to users, and the second patch
> > actually allows -O3 by removing the "depend on ARC" string.
>
> I think we should just remove -O3 support from KCONFIG.
>
> If someone wants to mess around with "experimental features," there's
> nothing stopping you from doing:
>
> $ make KCFLAGS=-O3
>

ARC uses -O3 since day1.

"Generic build system uses -O2, we want -O3"
in commit cfdbc2e16e65c1ec1c23057640607cee98d1a1bd

If they want -O3, it is up to the ARC maintainer.



If you want to say "use this option carefully",
EXPERT might be another option.

    depends on ARC || EXPERT







> >
> > The reasoning behind this series is to open up -O3 so that bugs related
> > to it (both compiler-related and kernel-related) can be discovered by
> > eyeballs wanting to improve the "-O3 experience," as that might be
> > beneficial to both compilers and the kernel. This has been attempted
> > before [1], but unfortunately nothing ever came of it.
> >
> > [1] https://lore.kernel.org/lkml/20191211104619.114557-1-oleksandr@redhat.com/
> >
> > Cc: linux-kbuild@vger.kernel.org
> > Cc: x86@kernel.org
> > Cc: Nathan Chancellor <nathan@kernel.org>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > Cc: Sean Christopherson <seanjc@google.com>
> > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Chris Down <chris@chrisdown.name>
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: John Ogness <john.ogness@linutronix.de>
> > Cc: Elliot Berman <quic_eberman@quicinc.com>
> > Cc: Oleksandr Natalenko <oleksandr@redhat.com>
> >
> > Miko Larsson (2):
> >   Kconfig: Mark -O3 as experimental
> >   Kconfig: Allow -O3 for all architectures
> >
> >  init/Kconfig | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > --
> > 2.36.1
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 0/2] Kconfig: -O3 enablement

[Miko Larsson]

On Wednesday, 22 June 2022 03:57:34 CEST Masahiro Yamada wrote:
> If you want to say "use this option carefully",
> EXPERT might be another option.
> 
>     depends on ARC || EXPERT
>

Yeah, this would be a fair compromise, though I think it would be
better to use "visible if" instead of "depends on". I can get a v2 of
the series together if this is desired.

--
~miko

Re: [PATCH 0/2] Kconfig: -O3 enablement

[Masahiro Yamada]

On Fri, Jun 24, 2022 at 12:42 AM Miko Larsson <mikoxyzzz@gmail.com> wrote:
>
> On Wednesday, 22 June 2022 03:57:34 CEST Masahiro Yamada wrote:
> > If you want to say "use this option carefully",
> > EXPERT might be another option.
> >
> >     depends on ARC || EXPERT
> >
>
> Yeah, this would be a fair compromise, though I think it would be
> better to use "visible if" instead of "depends on". I can get a v2 of
> the series together if this is desired.


Why is "visible if" better than "depends on"?



> --
> ~miko
>
>


--
Best Regards
Masahiro Yamada

Re: [PATCH 0/2] Kconfig: -O3 enablement

[Miko Larsson]

On Thursday, 23 June 2022 17:44:57 CEST Masahiro Yamada wrote:
> On Fri, Jun 24, 2022 at 12:42 AM Miko Larsson <mikoxyzzz@gmail.com> wrote:
> > On Wednesday, 22 June 2022 03:57:34 CEST Masahiro Yamada wrote:
> > > If you want to say "use this option carefully",
> > > EXPERT might be another option.
> > > 
> > >     depends on ARC || EXPERT
> > 
> > Yeah, this would be a fair compromise, though I think it would be
> > better to use "visible if" instead of "depends on". I can get a v2 of
> > the series together if this is desired.
> 
> Why is "visible if" better than "depends on"?
> 

Technically it most likely doesn't matter, but logically it makes more
sense, since we'd make CC_OPTIMIZE_FOR_PERFORMANCE_O3 be visible if
we're on ARC or if we have EXPERT enabled, instead of depending on
them. But yeah, it probably doesn't matter.

--
~miko

Re: [PATCH 0/2] Kconfig: -O3 enablement

[Masahiro Yamada]

On Fri, Jun 24, 2022 at 2:00 AM Miko Larsson <mikoxyzzz@gmail.com> wrote:
>
> On Thursday, 23 June 2022 17:44:57 CEST Masahiro Yamada wrote:
> > On Fri, Jun 24, 2022 at 12:42 AM Miko Larsson <mikoxyzzz@gmail.com> wrote:
> > > On Wednesday, 22 June 2022 03:57:34 CEST Masahiro Yamada wrote:
> > > > If you want to say "use this option carefully",
> > > > EXPERT might be another option.
> > > >
> > > >     depends on ARC || EXPERT
> > >
> > > Yeah, this would be a fair compromise, though I think it would be
> > > better to use "visible if" instead of "depends on". I can get a v2 of
> > > the series together if this is desired.
> >
> > Why is "visible if" better than "depends on"?
> >
>
> Technically it most likely doesn't matter, but logically it makes more
> sense, since we'd make CC_OPTIMIZE_FOR_PERFORMANCE_O3 be visible if
> we're on ARC or if we have EXPERT enabled, instead of depending on
> them. But yeah, it probably doesn't matter.


Did you write and test the code?


"visible if" is only supported for "menu".
This is clearly documented at line 207
of Documentation/kbuild/kconfig-language.rst


Using "visible if" for config entry will just
result in the syntax error.





> --
> ~miko
>
>
>


--
Best Regards
Masahiro Yamada

Re: [PATCH 0/2] Kconfig: -O3 enablement

[Miko Larsson]

On Thursday, 23 June 2022 19:15:14 CEST Masahiro Yamada wrote:
> On Fri, Jun 24, 2022 at 2:00 AM Miko Larsson <mikoxyzzz@gmail.com> wrote:
> > On Thursday, 23 June 2022 17:44:57 CEST Masahiro Yamada wrote:
> > > On Fri, Jun 24, 2022 at 12:42 AM Miko Larsson <mikoxyzzz@gmail.com> 
wrote:
> > > > On Wednesday, 22 June 2022 03:57:34 CEST Masahiro Yamada wrote:
> > > > > If you want to say "use this option carefully",
> > > > > EXPERT might be another option.
> > > > > 
> > > > >     depends on ARC || EXPERT
> > > > 
> > > > Yeah, this would be a fair compromise, though I think it would be
> > > > better to use "visible if" instead of "depends on". I can get a v2 of
> > > > the series together if this is desired.
> > > 
> > > Why is "visible if" better than "depends on"?
> > 
> > Technically it most likely doesn't matter, but logically it makes more
> > sense, since we'd make CC_OPTIMIZE_FOR_PERFORMANCE_O3 be visible if
> > we're on ARC or if we have EXPERT enabled, instead of depending on
> > them. But yeah, it probably doesn't matter.
> 
> Did you write and test the code?
>

Admittedly, I didn't, since I had falsely assumed that "visible if" was
just an "alternative" to "depends on".

> "visible if" is only supported for "menu".
> This is clearly documented at line 207
> of Documentation/kbuild/kconfig-language.rst
> 
> 
> Using "visible if" for config entry will just
> result in the syntax error.
>

Oops, yeah, I wasn't aware of this. Sorry.

--
~miko

Re: [PATCH 0/2] Kconfig: -O3 enablement

[Arnd Bergmann]

On Wed, Jun 22, 2022 at 3:57 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> On Wed, Jun 22, 2022 at 1:17 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > On Tue, Jun 21, 2022 at 6:35 AM Miko Larsson <mikoxyzzz@gmail.com> wrote:
> > >
> > > Hi,
> > >
> > > This very small series allows -O3 to be used for all architectures. The
> > > first patch marks -O3 as experimental, with the reasoning being that it
> > > might expose unwanted regressions to users, and the second patch
> > > actually allows -O3 by removing the "depend on ARC" string.
> >
> > I think we should just remove -O3 support from KCONFIG.

I agree that would be best

> > If someone wants to mess around with "experimental features," there's
> > nothing stopping you from doing:
> >
> > $ make KCFLAGS=-O3
> >
>
> ARC uses -O3 since day1.
>
> "Generic build system uses -O2, we want -O3"
> in commit cfdbc2e16e65c1ec1c23057640607cee98d1a1bd
>
> If they want -O3, it is up to the ARC maintainer.

I suppose whatever the reason for using -O3 at the time has
likely changed by now.

> If you want to say "use this option carefully",
> EXPERT might be another option.
>
>     depends on ARC || EXPERT

This probably also needs a dependency on !COMPILE_TEST so we don't
report compile-time problems that are specific to -O3. Maybe a good first
step would be to turn this into

      depends on ARCH && EXPERT && !COMPILE_TEST

which should help both with compile-testing on ARC, and it would
prevent it from being visible on other architectures.

        Arnd

Re: [PATCH 0/2] Kconfig: -O3 enablement

[Linus Torvalds]

On Thu, Jun 23 at 7:27:29 PM  Arnd Bergmann <arnd@arndb.de> wrote:
>
> This probably also needs a dependency on !COMPILE_TEST so we don't
> report compile-time problems that are specific to -O3.

Honestly, let's just remove -O3 entirely.

Enabling it, and then not even build-testing the result, is just about
the *worst* possible case. That's just horrible.

The argument that "but ARC uses it" is not an argument. It was always
a bad argument, and ARC needs to just fix whatever it is that made it
an issue (likely already fixed with a compiler upgrade).

And there is no way I would ever accept this as a "let people try it" when

 - as mentioned, just use KCFLAGS=-O3 if you want to

 - -O3 has a *loong* history of generating worse code than -O2

so I will *not* be taking these kinds of patches without some very
serious explanations of why -O3 has suddenly become acceptable again.

Those explanations had better be more than "let people try". They
should have in-depth actual performance numbers for a real load, not
some made-up "bigger is better" logic.

                 Linus

Re: [PATCH 0/2] Kconfig: -O3 enablement

[Linus Torvalds]

> On Fri, Jun 24, 2022 at 11:29 AM Linus Torvalds <linus@linux-foundation.org> wrote:

Bah. That was really me, just with a badly set up "reply through lore"
setup, so with the wrong email address.

That's what happens when you are

 (a) incompetent

 (b) stopped getting the mailing lists as regular email because you
think lore works so well

 (c) normally rely on being cc'd

My apologies for the incompetence.

                Linus