[PATCH 1/2] x86, Makefile: Move the CPU-specific 64-bit tuning settings to arch/x86/Makefile.cpu

[tor...@mailbox.org]

In accordance with the FIXME comment in arch/x86/Makefile, move the
CPU-specific 64-bit tuning settings to arch/x86/Makefile.cpu.

Signed-off-by: Tor Vic <tor...@mailbox.org>
---
arch/x86/Makefile | 11 ++---------
arch/x86/Makefile.cpu | 12 ++++++++++++
2 files changed, 14 insertions(+), 9 deletions(-)
create mode 100644 arch/x86/Makefile.cpu

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 307fd0000a83..94105d7ad22c 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -119,15 +119,8 @@ else
# Use -mskip-rax-setup if supported.
KBUILD_CFLAGS += $(call cc-option,-mskip-rax-setup)

- # FIXME - should be integrated in Makefile.cpu (Makefile_32.cpu)
- cflags-$(CONFIG_MK8) += $(call cc-option,-march=k8)
- cflags-$(CONFIG_MPSC) += $(call cc-option,-march=nocona)
-
- cflags-$(CONFIG_MCORE2) += \
- $(call cc-option,-march=core2,$(call cc-option,-mtune=generic))
- cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom) \
- $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
- cflags-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=generic)
+ # CPU-specific tuning (64-bit).
+ include arch/x86/Makefile.cpu
KBUILD_CFLAGS += $(cflags-y)

KBUILD_CFLAGS += -mno-red-zone
diff --git a/arch/x86/Makefile.cpu b/arch/x86/Makefile.cpu
new file mode 100644
index 000000000000..fb407ae94d90
--- /dev/null
+++ b/arch/x86/Makefile.cpu
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+# CPU tuning section (64-bit) - shared with UML.
+# Must change only cflags-y (or [yn]), not CFLAGS! That makes a difference for UML.
+
+cflags-$(CONFIG_MK8) += $(call cc-option,-march=k8)
+cflags-$(CONFIG_MPSC) += $(call cc-option,-march=nocona)
+
+cflags-$(CONFIG_MCORE2) += \
+ $(call cc-option,-march=core2,$(call cc-option,-mtune=generic))
+cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom) \
+ $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
+cflags-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=generic)
--
2.32.0

[tor...@mailbox.org]

Add new generic x86-64 CPU tunings introduced with recent versions of
gcc and clang, as documented in x86-64-psABI [1].

This is taken straight from graysky's CPU optimization patch with minor
modifications [2].

[1] https://gitlab.com/x86-psABIs/x86-64-ABI/-/commit/77566eb03bc6a326811cb7e9a6b9396884b67c7c
[2] https://github.com/graysky2/kernel_compiler_patch

Signed-off-by: Tor Vic <tor...@mailbox.org>
---

arch/x86/Kconfig.cpu | 26 +++++++++++++++++++++++++-
arch/x86/Makefile.cpu | 6 ++++++
2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu
index 814fe0d349b0..a2c872aa5a0b 100644
--- a/arch/x86/Kconfig.cpu
+++ b/arch/x86/Kconfig.cpu
@@ -294,6 +294,30 @@ config GENERIC_CPU
Generic x86-64 CPU.
Run equally well on all x86-64 CPUs.

+config GENERIC_CPU_V2
+ bool "Generic-x86-64-v2"
+ depends on ( CC_IS_GCC && GCC_VERSION > 110000 ) || ( CC_IS_CLANG && CLANG_VERSION >= 120000 )
+ depends on X86_64
+ help
+ Generic x86-64 CPU.
+ Run equally well on all x86-64 CPUs with min support of x86-64-v2.
+
+config GENERIC_CPU_V3
+ bool "Generic-x86-64-v3"
+ depends on ( CC_IS_GCC && GCC_VERSION > 110000 ) || ( CC_IS_CLANG && CLANG_VERSION >= 120000 )
+ depends on X86_64
+ help
+ Generic x86-64-v3 CPU with v3 instructions.
+ Run equally well on all x86-64 CPUs with min support of x86-64-v3.
+
+config GENERIC_CPU_V4
+ bool "Generic-x86-64-v4"
+ depends on ( CC_IS_GCC && GCC_VERSION > 110000 ) || ( CC_IS_CLANG && CLANG_VERSION >= 120000 )
+ depends on X86_64
+ help
+ Generic x86-64 CPU with v4 instructions.
+ Run equally well on all x86-64 CPUs with min support of x86-64-v4.
+
endchoice

config X86_GENERIC
@@ -318,7 +342,7 @@ config X86_INTERNODE_CACHE_SHIFT
config X86_L1_CACHE_SHIFT
int
default "7" if MPENTIUM4 || MPSC
- default "6" if MK7 || MK8 || MPENTIUMM || MCORE2 || MATOM || MVIAC7 || X86_GENERIC || GENERIC_CPU
+ default "6" if MK7 || MK8 || MPENTIUMM || MCORE2 || MATOM || MVIAC7 || X86_GENERIC || GENERIC_CPU || GENERIC_CPU_V2 || GENERIC_CPU_V3 || GENERIC_CPU_V4
default "4" if MELAN || M486SX || M486 || MGEODEGX1
default "5" if MWINCHIP3D || MWINCHIPC6 || MCRUSOE || MEFFICEON || MCYRIXIII || MK6 || MPENTIUMIII || MPENTIUMII || M686 || M586MMX || M586TSC || M586 || MVIAC3_2 || MGEODE_LX

diff --git a/arch/x86/Makefile.cpu b/arch/x86/Makefile.cpu
index fb407ae94d90..f190feefe964 100644
--- a/arch/x86/Makefile.cpu
+++ b/arch/x86/Makefile.cpu
@@ -10,3 +10,9 @@ cflags-$(CONFIG_MCORE2) += \
cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom) \
$(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))

cflags-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=generic)
+

+# New generic x86-64 CPU options.
+# Don't call the compiler, we have Kconfig compiler checks for these.
+cflags-$(CONFIG_GENERIC_CPU_V2) += -march=x86-64-v2
+cflags-$(CONFIG_GENERIC_CPU_V3) += -march=x86-64-v3
+cflags-$(CONFIG_GENERIC_CPU_V4) += -march=x86-64-v4
--
2.32.0

[Nick Desaulniers]

Tor,
Thanks for the patches. One thing I always try to do is use
./scripts/get_maintainer.pl to get the appropriate maintainers and
reviewers cc'ed. I prefer to explicitly put the maintainers that I
expect to pick up a patch in the To: line.

$ ./scripts/get_maintainer.pl arch/x86/Makefile -norolestats
Thomas Gleixner <tg...@linutronix.de>
Ingo Molnar <mi...@redhat.com>
Borislav Petkov <b...@alien8.de>
x...@kernel.org
"H. Peter Anvin" <h...@zytor.com>
Nathan Chancellor <nat...@kernel.org>
Nick Desaulniers <ndesau...@google.com>
linux-...@vger.kernel.org
clang-bu...@googlegroups.com

added them to CC. Probably needs a refresh at this point, but
https://nickdesaulniers.github.io/blog/2017/05/16/submitting-your-first-patch-to-the-linux-kernel-and-responding-to-feedback/
is still mostly relevant.

So we have arch/x86/Makefile_32.cpu, would it be more symmetrical to
have a new arch/x86/Makefile_64.cpu (rather than Makefile.cpu)?

> KBUILD_CFLAGS += $(cflags-y)
>
> KBUILD_CFLAGS += -mno-red-zone
> diff --git a/arch/x86/Makefile.cpu b/arch/x86/Makefile.cpu
> new file mode 100644
> index 000000000000..fb407ae94d90
> --- /dev/null
> +++ b/arch/x86/Makefile.cpu
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# CPU tuning section (64-bit) - shared with UML.
> +# Must change only cflags-y (or [yn]), not CFLAGS! That makes a difference for UML.
> +
> +cflags-$(CONFIG_MK8) += $(call cc-option,-march=k8)
> +cflags-$(CONFIG_MPSC) += $(call cc-option,-march=nocona)
> +
> +cflags-$(CONFIG_MCORE2) += \
> + $(call cc-option,-march=core2,$(call cc-option,-mtune=generic))
> +cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom) \
> + $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
> +cflags-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=generic)

I was going to say we probably don't want to limit -mtune=genric to
64b only, but it seems that arch/x86/Makefile_32.cpu has some
duplication here.

47 cflags-$(CONFIG_X86_GENERIC) += $(call tune,generic,$(call
tune,i686))

That probably could be hoisted back into arch/x86/Makefile before the
CONFIG_X86_32 check, but I'm not sure about the differences between
CONFIG_GENERIC_CPU vs CONFIG_X86_GENERIC?
--
Thanks,
~Nick Desaulniers

[Nick Desaulniers]

On Tue, Aug 17, 2021 at 7:24 AM torvic9 via Clang Built Linux
<clang-bu...@googlegroups.com> wrote:
>
> Add new generic x86-64 CPU tunings introduced with recent versions of
> gcc and clang, as documented in x86-64-psABI [1].
>
> This is taken straight from graysky's CPU optimization patch with minor
> modifications [2].
>
> [1] https://gitlab.com/x86-psABIs/x86-64-ABI/-/commit/77566eb03bc6a326811cb7e9a6b9396884b67c7c
> [2] https://github.com/graysky2/kernel_compiler_patch
>
> Signed-off-by: Tor Vic <tor...@mailbox.org>
> ---
> arch/x86/Kconfig.cpu | 26 +++++++++++++++++++++++++-
> arch/x86/Makefile.cpu | 6 ++++++
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu
> index 814fe0d349b0..a2c872aa5a0b 100644
> --- a/arch/x86/Kconfig.cpu
> +++ b/arch/x86/Kconfig.cpu
> @@ -294,6 +294,30 @@ config GENERIC_CPU
> Generic x86-64 CPU.
> Run equally well on all x86-64 CPUs.
>
> +config GENERIC_CPU_V2
> + bool "Generic-x86-64-v2"
> + depends on ( CC_IS_GCC && GCC_VERSION > 110000 ) || ( CC_IS_CLANG && CLANG_VERSION >= 120000 )

Can we stick with either > or >=, but at least be consistent? Also,
there's unnecessary whitespace inside the parens that could be
removed.

You'd be surprised how quickly using time-relative words like "New"
bitrot. My copy of "Modern C++" already doesn't feel very "modern" for
example. I'd just drop the comment block here; we don't need a
comment every time we skip cc-option checks. Thanks for the patches!

> +cflags-$(CONFIG_GENERIC_CPU_V2) += -march=x86-64-v2
> +cflags-$(CONFIG_GENERIC_CPU_V3) += -march=x86-64-v3
> +cflags-$(CONFIG_GENERIC_CPU_V4) += -march=x86-64-v4
> --

--
Thanks,
~Nick Desaulniers

[Nick Desaulniers]

Ah, cc-option can be used from Kconfig. Perhaps a cc-option call to
-march=x86-64-vX with a comment about which compiler versions first
supported these (so that we can remove the feature test when the
minimum supported toolchains are advanced) would be better?

--
Thanks,
~Nick Desaulniers

[Tor Vic]

On 17.08.21 17:58, Nick Desaulniers wrote:
> Tor,
> Thanks for the patches. One thing I always try to do is use
> ./scripts/get_maintainer.pl to get the appropriate maintainers and
> reviewers cc'ed. I prefer to explicitly put the maintainers that I
> expect to pick up a patch in the To: line.
>

I did use the script, but I wasn't sure whether to include groups,
individuals, or both.
Noted for next submission.

> $ ./scripts/get_maintainer.pl arch/x86/Makefile -norolestats
> Thomas Gleixner <tg...@linutronix.de>
> Ingo Molnar <mi...@redhat.com>
> Borislav Petkov <b...@alien8.de>
> x...@kernel.org
> "H. Peter Anvin" <h...@zytor.com>
> Nathan Chancellor <nat...@kernel.org>
> Nick Desaulniers <ndesau...@google.com>
> linux-...@vger.kernel.org
> clang-bu...@googlegroups.com
>
> added them to CC. Probably needs a refresh at this point, but
> https://nickdesaulniers.github.io/blog/2017/05/16/submitting-your-first-patch-to-the-linux-kernel-and-responding-to-feedback/
> is still mostly relevant.
>

Noted as well.
Thanks!

[Tor Vic]

That makes sense. Will do a v2.
Which of these possibilities is more "expensive"?
I remember a recent patch where some of these calls were removed.

[Borislav Petkov]

Every once in a while, patches like that pop up without any real numbers
advocating for the additional and perhaps unnecessary complexity.

If those -march switches don't bring any measureable improvements - and
I highly doubt they do - you shouldn't waste time with submitting them.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[Nick Desaulniers]

On Tue, Aug 17, 2021 at 11:31 AM Tor Vic <tor...@mailbox.org> wrote:
>
> On 17.08.21 18:22, Nick Desaulniers wrote:
> >
> > Ah, cc-option can be used from Kconfig. Perhaps a cc-option call to
> > -march=x86-64-vX with a comment about which compiler versions first
> > supported these (so that we can remove the feature test when the
> > minimum supported toolchains are advanced) would be better?
> >
>
> That makes sense. Will do a v2.
> Which of these possibilities is more "expensive"?
> I remember a recent patch where some of these calls were removed.

cc-option does "cost" more than these version checks, but it is more
concise about what we care about and they can be removed over time.
They also work better for different compilers, since it's easy to
forget to check one particular compiler, or get the compiler version
wrong, or break a pre-released compiler. We generally want to prefer
feature detection to version detection. I sent a series recently
removing stale checks.
--
Thanks,
~Nick Desaulniers

[Tor Vic]

In other words, I should back up these additions with benchmarks?
I have some slight doubts too, but I'll gladly do so and provide
some results with a v2.

>
> Thx.
>

[Christoph Hellwig]

On Tue, Aug 17, 2021 at 04:23:09PM +0200, tor...@mailbox.org wrote:
> +# SPDX-License-Identifier: GPL-2.0
> +# CPU tuning section (64-bit) - shared with UML.
> +# Must change only cflags-y (or [yn]), not CFLAGS! That makes a difference for UML.

Plase avoid the overly long line. Also having an empty line between the
SPDX tag and the actual top of the file comment really helps readability.

[Christoph Hellwig]

On Tue, Aug 17, 2021 at 04:24:48PM +0200, tor...@mailbox.org wrote:

> + Generic x86-64 CPU.
> + Run equally well on all x86-64 CPUs with min support of x86-64-v2.

> + help
> + Generic x86-64-v3 CPU with v3 instructions.
> + Run equally well on all x86-64 CPUs with min support of x86-64-v3.
> +

> + help
> + Generic x86-64 CPU with v4 instructions.
> + Run equally well on all x86-64 CPUs with min support of x86-64-v4.

How the f&%$% is a user supposed to know what these garbage descriptions
are supposed to mean?

[tor...@mailbox.org]

> Christoph Hellwig <h...@infradead.org> hat am 18.08.2021 09:11 geschrieben:

It's a copy-paste from the existing Makefile_32.cpu, so I guess it has to
be changed there as well?

[tor...@mailbox.org]

> Christoph Hellwig <h...@infradead.org> hat am 18.08.2021 09:15 geschrieben:

I agree, will come up with something better in v2.

[Masahiro Yamada]

See
https://patchwork.kernel.org/project/linux-kbuild/patch/20210724183556.7...@kernel.org/

> KBUILD_CFLAGS += $(cflags-y)
>
> KBUILD_CFLAGS += -mno-red-zone
> diff --git a/arch/x86/Makefile.cpu b/arch/x86/Makefile.cpu
> new file mode 100644
> index 000000000000..fb407ae94d90
> --- /dev/null
> +++ b/arch/x86/Makefile.cpu
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# CPU tuning section (64-bit) - shared with UML.
> +# Must change only cflags-y (or [yn]), not CFLAGS! That makes a difference for UML.
> +
> +cflags-$(CONFIG_MK8) += $(call cc-option,-march=k8)
> +cflags-$(CONFIG_MPSC) += $(call cc-option,-march=nocona)
> +
> +cflags-$(CONFIG_MCORE2) += \
> + $(call cc-option,-march=core2,$(call cc-option,-mtune=generic))
> +cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom) \
> + $(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
> +cflags-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=generic)
> --
> 2.32.0
>

> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-li...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/269701460.117528.1629210189833%40office.mailbox.org.



--
Best Regards
Masahiro Yamada

[tor...@mailbox.org]

> Borislav Petkov <b...@alien8.de> hat am 17.08.2021 20:36 geschrieben:

Borislav,
I ran some quick checks and benchmarks, and your doubts seem to be justified.
A 5.14-rc6 kernel compiled with the default 'generic' and one built with
'x86-64-v3' even have the exact same uncompressed file size.
Benchmarks were inconclusive as well.
So I will only resubmit the first patch of this series, with the changes
suggested by Masahiro and Christoph.

[John]

For what it's worth, I did some benchmarking a few years ago using the time it took to build with gcc as the endpoint and compared the generic x86_64 option to the core2 option as well as to some others -march= options introduced by the my patchset and found very small but statistically relevant differences in favor of the -march= options[1].

1. https://github.com/graysky2/kernel_compiler_patch

[Borislav Petkov]

On Wed, Aug 18, 2021 at 05:08:40PM +0200, tor...@mailbox.org wrote:
> I ran some quick checks and benchmarks, and your doubts seem to be
> justified. A 5.14-rc6 kernel compiled with the default 'generic' and
> one built with 'x86-64-v3' even have the exact same uncompressed file
> size. Benchmarks were inconclusive as well.

Lemme preface this with a IMHO:

Yeah, those -march machine-specific compiler switches don't really
show any perf improvements for kernels because, well, if you look at
the instruction stream a kernel executes, there's not really a whole
lot left to optimize after -O2.

Also, the percentage of time a machine spends in the kernel should be a
lot smaller (orders of magnitude) than in userspace - the operative word
being *should* here :-) - so there really isn't anything to optimize.

Not to say that there aren't a gazillion other places in the kernel that
could use more eyes and testing. ^Hint hint^

[Tor Vic]

That's something I'd like to help with, hence my (not very good)
submissions.
You're one of the kernel colonels, so I'm happy to get some *useful*
feedback which enables noobs like me to get a better understanding of
the kernel stuff.
Also, credits to graysky because this is actually based on his work.

>
> Thx.
>

[Borislav Petkov]

On Wed, Aug 18, 2021 at 06:28:47PM +0000, Tor Vic wrote:
> That's something I'd like to help with, hence my (not very good)
> submissions. You're one of the kernel colonels, so I'm happy to get
> some *useful* feedback which enables noobs like me to get a better
> understanding of the kernel stuff.

Sure, Greg so happens to have written a reply recently explaining what
to do:

https://lists.kernelnewbies.org/pipermail/kernelnewbies/2017-April/017765.html

Thx Greg!

In addition to the above, I'd suggest reading lkml, reading other
people's patches, review of those patches by people and trying to
understand what they're talking about. And then following through the
code to try to see it.

What also really helps is setting up a kvm guest, booting the kernel
with it and adding printks here and there and dumping interesting
information with them.

Once you start dealing with this, you might spend a couple of decades
figuring it all out in full depth. :-)

Oh, and this https://www.kernel.org/doc/html/latest

has all of the kernel's documentation formatted nicely.

I sincerely hope that helps a bit.

[David Laight]

From: Christoph Hellwig
> Sent: 18 August 2021 08:15

My thoughts.
I then looked up the link.
Most of the extra instructions are the AVX related ones.
They better not get enabled for a kernel build at all.

I wish I knew whether 'no-avx' actually implied 'no-avx2' and
all later avx options?
Or do kernel builds have to spot each new option and disable
in in turn?

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)