[PATCH 0/3] RFC: Platform Support for AMD Zen and AVX2/AVX

[PATCH 0/3] RFC: Platform Support for AMD Zen and AVX2/AVX

[Prem Mallappa]

From: Prem Mallappa <Premachandra.Mallappa@amd.com>

Hello Glibc Community,

This is in response to

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=24979
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=24080
[3] https://sourceware.org/bugzilla/show_bug.cgi?id=23249

It is clear that there is no panacea here. However,
here is an attempt to address them in parts.

From [1], enable customers who already have
"haswell" libs and has seen perf benifits by loading
them on AMD Zen.
(Load libraries by placing them in LD_LIBRARY_PATH/zen
or by a symbolic link zen->haswell)

From [2] and [3]
And, A futuristic generic-avx2/generic-avx libs,
enables OS vendors to supply an optimized set.
And haswell/zen are really a superset, hence
keeping it made sense.

By this we would like to open it up for discussion
The haswell/zen can be intel/amd
(or any other name, and supply ifunc based loading
internally)

Prem Mallappa (3):
  x86: Refactor platform support in cpu_features
  x86: Add AMD Zen and AVX2/AVX platform support
  x86: test to load from PLATFORM path

 sysdeps/x86/cpu-features.c | 113 ++++++++++++++++++++++---------------
 sysdeps/x86_64/Makefile    |   3 +-
 2 files changed, 69 insertions(+), 47 deletions(-)

-- 
2.20.1

[PATCH 1/3] x86: Refactor platform support in cpu_features

[Prem Mallappa]

From: Prem Mallappa <Premachandra.Mallappa@amd.com>

This is a preliminary support to have platform
for AMD processors.

Signed-off-by: Prem Mallappa <Premachandra.Mallappa@amd.com>
---
 sysdeps/x86/cpu-features.c | 99 ++++++++++++++++++++------------------
 1 file changed, 53 insertions(+), 46 deletions(-)

diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
index 81a170a819..a36f385976 100644
--- a/sysdeps/x86/cpu-features.c
+++ b/sysdeps/x86/cpu-features.c
@@ -302,6 +302,58 @@ _Static_assert (((index_arch_Fast_Unaligned_Load
 		     == index_arch_Fast_Copy_Backward)),
 		"Incorrect index_arch_Fast_Unaligned_Load");
 
+static void
+set_platform (struct cpu_features *cpu_features)
+{
+#ifdef __x86_64__
+  const char *platform = NULL;
+
+  GLRO(dl_hwcap) = HWCAP_X86_64;
+
+  if (cpu_features->basic.kind == arch_kind_intel)
+    {
+      if (CPU_FEATURES_ARCH_P (cpu_features, AVX512F_Usable)
+	  && CPU_FEATURES_CPU_P (cpu_features, AVX512CD))
+	{
+	  if (CPU_FEATURES_CPU_P (cpu_features, AVX512ER))
+	    {
+	      if (CPU_FEATURES_CPU_P (cpu_features, AVX512PF))
+		platform = "xeon_phi";
+	    }
+	  else
+	    {
+	      if (CPU_FEATURES_CPU_P (cpu_features, AVX512BW)
+		  && CPU_FEATURES_CPU_P (cpu_features, AVX512DQ)
+		  && CPU_FEATURES_CPU_P (cpu_features, AVX512VL))
+		GLRO(dl_hwcap) |= HWCAP_X86_AVX512_1;
+	    }
+	}
+
+      if (platform == NULL
+	  && CPU_FEATURES_ARCH_P (cpu_features, AVX2_Usable)
+	  && CPU_FEATURES_ARCH_P (cpu_features, FMA_Usable)
+	  && CPU_FEATURES_CPU_P (cpu_features, BMI1)
+	  && CPU_FEATURES_CPU_P (cpu_features, BMI2)
+	  && CPU_FEATURES_CPU_P (cpu_features, LZCNT)
+	  && CPU_FEATURES_CPU_P (cpu_features, MOVBE)
+	  && CPU_FEATURES_CPU_P (cpu_features, POPCNT))
+	platform = "haswell";
+    }
+
+  if (platform != NULL)
+    GLRO(dl_platform) = platform;
+#else
+  GLRO(dl_hwcap) = 0;
+  if (CPU_FEATURES_CPU_P (cpu_features, SSE2))
+    GLRO(dl_hwcap) |= HWCAP_X86_SSE2;
+
+  if (CPU_FEATURES_ARCH_P (cpu_features, I686))
+    GLRO(dl_platform) = "i686";
+  else if (CPU_FEATURES_ARCH_P (cpu_features, I586))
+    GLRO(dl_platform) = "i586";
+#endif
+}
+
 static inline void
 init_cpu_features (struct cpu_features *cpu_features)
 {
@@ -506,52 +558,7 @@ no_cpuid:
   GLRO(dl_hwcap_mask) = HWCAP_IMPORTANT;
 #endif
 
-#ifdef __x86_64__
-  GLRO(dl_hwcap) = HWCAP_X86_64;
-  if (cpu_features->basic.kind == arch_kind_intel)
-    {
-      const char *platform = NULL;
-
-      if (CPU_FEATURES_ARCH_P (cpu_features, AVX512F_Usable)
-	  && CPU_FEATURES_CPU_P (cpu_features, AVX512CD))
-	{
-	  if (CPU_FEATURES_CPU_P (cpu_features, AVX512ER))
-	    {
-	      if (CPU_FEATURES_CPU_P (cpu_features, AVX512PF))
-		platform = "xeon_phi";
-	    }
-	  else
-	    {
-	      if (CPU_FEATURES_CPU_P (cpu_features, AVX512BW)
-		  && CPU_FEATURES_CPU_P (cpu_features, AVX512DQ)
-		  && CPU_FEATURES_CPU_P (cpu_features, AVX512VL))
-		GLRO(dl_hwcap) |= HWCAP_X86_AVX512_1;
-	    }
-	}
-
-      if (platform == NULL
-	  && CPU_FEATURES_ARCH_P (cpu_features, AVX2_Usable)
-	  && CPU_FEATURES_ARCH_P (cpu_features, FMA_Usable)
-	  && CPU_FEATURES_CPU_P (cpu_features, BMI1)
-	  && CPU_FEATURES_CPU_P (cpu_features, BMI2)
-	  && CPU_FEATURES_CPU_P (cpu_features, LZCNT)
-	  && CPU_FEATURES_CPU_P (cpu_features, MOVBE)
-	  && CPU_FEATURES_CPU_P (cpu_features, POPCNT))
-	platform = "haswell";
-
-      if (platform != NULL)
-	GLRO(dl_platform) = platform;
-    }
-#else
-  GLRO(dl_hwcap) = 0;
-  if (CPU_FEATURES_CPU_P (cpu_features, SSE2))
-    GLRO(dl_hwcap) |= HWCAP_X86_SSE2;
-
-  if (CPU_FEATURES_ARCH_P (cpu_features, I686))
-    GLRO(dl_platform) = "i686";
-  else if (CPU_FEATURES_ARCH_P (cpu_features, I586))
-    GLRO(dl_platform) = "i586";
-#endif
+  set_platform(cpu_features);
 
 #if CET_ENABLED
 # if HAVE_TUNABLES
-- 
2.20.1

[PATCH 2/3] x86: Add AMD Zen and AVX2/AVX platform support

[Prem Mallappa]

From: Prem Mallappa <Premachandra.Mallappa@amd.com>

Priority as below
- AVX512
- Haswell/Zen as they are superset of AVX2/AVX
- AVX2
- AVX

Signed-off-by: Prem Mallappa <Premachandra.Mallappa@amd.com>
---
 sysdeps/x86/cpu-features.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
index a36f385976..2097107968 100644
--- a/sysdeps/x86/cpu-features.c
+++ b/sysdeps/x86/cpu-features.c
@@ -328,20 +328,34 @@ set_platform (struct cpu_features *cpu_features)
 		GLRO(dl_hwcap) |= HWCAP_X86_AVX512_1;
 	    }
 	}
+    }
 
-      if (platform == NULL
-	  && CPU_FEATURES_ARCH_P (cpu_features, AVX2_Usable)
-	  && CPU_FEATURES_ARCH_P (cpu_features, FMA_Usable)
-	  && CPU_FEATURES_CPU_P (cpu_features, BMI1)
-	  && CPU_FEATURES_CPU_P (cpu_features, BMI2)
-	  && CPU_FEATURES_CPU_P (cpu_features, LZCNT)
-	  && CPU_FEATURES_CPU_P (cpu_features, MOVBE)
-	  && CPU_FEATURES_CPU_P (cpu_features, POPCNT))
+  if (platform == NULL
+      && CPU_FEATURES_ARCH_P (cpu_features, AVX2_Usable)
+      && CPU_FEATURES_ARCH_P (cpu_features, FMA_Usable)
+      && CPU_FEATURES_CPU_P (cpu_features, BMI1)
+      && CPU_FEATURES_CPU_P (cpu_features, BMI2)
+      && CPU_FEATURES_CPU_P (cpu_features, LZCNT)
+      && CPU_FEATURES_CPU_P (cpu_features, MOVBE)
+      && CPU_FEATURES_CPU_P (cpu_features, POPCNT))
+    {
+      if (cpu_features->basic.kind == arch_kind_intel)
 	platform = "haswell";
+      else if (cpu_features->basic.kind == arch_kind_amd)
+	platform = "zen";
     }
 
+  if (platform == NULL
+      && CPU_FEATURES_ARCH_P (cpu_features, AVX2_Usable))
+    platform = "generic-avx2";
+
+  if (platform == NULL
+      && CPU_FEATURES_ARCH_P (cpu_features, AVX_Usable))
+    platform = "generic-avx";
+
   if (platform != NULL)
     GLRO(dl_platform) = platform;
+
 #else
   GLRO(dl_hwcap) = 0;
   if (CPU_FEATURES_CPU_P (cpu_features, SSE2))
-- 
2.20.1

[PATCH 3/3] x86: test to load from PLATFORM path

[Prem Mallappa]

From: Prem Mallappa <Premachandra.Mallappa@amd.com>

Test will load modifier library from new
PLATFORM path as well.

Signed-off-by: Prem Mallappa <Premachandra.Mallappa@amd.com>
---
 sysdeps/x86_64/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sysdeps/x86_64/Makefile b/sysdeps/x86_64/Makefile
index d51cf03ac9..b09881b2d7 100644
--- a/sysdeps/x86_64/Makefile
+++ b/sysdeps/x86_64/Makefile
@@ -71,6 +71,7 @@ CFLAGS-tst-platformmod-2.c = -mno-avx
 LDFLAGS-tst-platformmod-2.so = -Wl,-soname,tst-platformmod-2.so
 $(objpfx)tst-platform-1: $(objpfx)tst-platformmod-1.so
 $(objpfx)tst-platform-1.out: $(objpfx)x86_64/tst-platformmod-2.so
+$(objpfx)tst-platform-1.out: $(objpfx)zen/tst-platformmod-2.so
 # Turn off AVX512F_Usable and AVX2_Usable so that GLRO(dl_platform) is
 # always set to x86_64.
 tst-platform-1-ENV = LD_PRELOAD=$(objpfx)\$$PLATFORM/tst-platformmod-2.so \
@@ -161,7 +162,7 @@ do-tests-clean common-mostlyclean: tst-x86_64-1-clean
 tst-x86_64-1-clean:
 	-rm -rf $(objpfx)x86_64
 
-$(objpfx)x86_64/tst-platformmod-2.os: $(objpfx)tst-platformmod-2.os
+$(objpfx)%/tst-platformmod-2.os: $(objpfx)tst-platformmod-2.os
 	$(make-target-directory)
 	rm -f $@
 	ln $< $@
-- 
2.20.1

Re: [PATCH 0/3] RFC: Platform Support for AMD Zen and AVX2/AVX

[Adhemerval Zanella]

On 16/03/2020 03:44, Prem Mallappa via Libc-help wrote:
> From: Prem Mallappa <Premachandra.Mallappa@amd.com>
> 
> Hello Glibc Community,
> 
> This is in response to
> 
> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=24979
> [2] https://sourceware.org/bugzilla/show_bug.cgi?id=24080
> [3] https://sourceware.org/bugzilla/show_bug.cgi?id=23249
> 
> It is clear that there is no panacea here. However,
> here is an attempt to address them in parts.

Thanks for working on this. I have wrote a open letter after I
read some mishandled criticism that glibc was worsening AMD
performance on purpose [1]. It is good to see some response from
AMD itself.

About the patch, I would ask you to send it to libc-alpha
instead [2], this maillist is usually for glibc questions including 
build problems and library usage. Patches discussions, even
RFC ones, are done on libc-alpha.

Regarding the patch, it looks good in general but I would ask
you to remove the 'Signed-off-by:' (we don't use DCO, but rather
copyright assignments).

[1] https://sourceware.org/pipermail/libc-alpha/2019-September/106474.html
[2] https://sourceware.org/mailman/listinfo/libc-alpha


> 
> From [1], enable customers who already have
> "haswell" libs and has seen perf benifits by loading
> them on AMD Zen.
> (Load libraries by placing them in LD_LIBRARY_PATH/zen
> or by a symbolic link zen->haswell)
> 
> From [2] and [3]
> And, A futuristic generic-avx2/generic-avx libs,
> enables OS vendors to supply an optimized set.
> And haswell/zen are really a superset, hence
> keeping it made sense.
> 
> By this we would like to open it up for discussion
> The haswell/zen can be intel/amd
> (or any other name, and supply ifunc based loading
> internally)
> 
> Prem Mallappa (3):
>   x86: Refactor platform support in cpu_features
>   x86: Add AMD Zen and AVX2/AVX platform support
>   x86: test to load from PLATFORM path
> 
>  sysdeps/x86/cpu-features.c | 113 ++++++++++++++++++++++---------------
>  sysdeps/x86_64/Makefile    |   3 +-
>  2 files changed, 69 insertions(+), 47 deletions(-)
>