POWER micro-architecture dispatch by loveshack · Pull Request #345 · flame/blis

loveshack · 2019-09-24T10:05:18Z

This implements a Linux-specific, compiler-neutral selection, unlike the (possibly?) gcc-specific simple one you didn't like. It currently only affects the compilation of generic code.

Note that there's a strange problem I've had to kludge round by lowering the optimization levels noted in a log message, but it seems worth recording this now. I can only try with Fedora 28's gcc 8.3 (on power8) currently, but will try to get the system updated to a supported Fedora. I might be able to get access to power9, but haven't tried.

This was branched from the arm64 equivalent as there originally was a merge clash with bits I removed.

See the GCC doc concerning priorities.

This only affects compiling with cpu-specific flags, not anything in the micro-kernel.

power8 follows power9 in just using the reference micro-kernel. Also fix an unused variable warning.

This needs fixing properly somehow, but using -O3 (at least with gcc 8.3), we get this: Program received signal SIGILL, Illegal instruction. 0x000000001004c660 in bli_cntx_init_power9_ref (cntx=0x103e06b0) at ref_kernels/bli_cntx_ref.c:456 456 for ( i = 0; i < BLIS_NUM_LEVEL3_OPS; ++i ) vfuncs[ i ] = NULL; (gdb) bt #0 0x000000001004c660 in bli_cntx_init_power9_ref (cntx=0x103e06b0) at ref_kernels/bli_cntx_ref.c:456 flame#1 0x000000001004c0a8 in bli_cntx_init_power9 (cntx=<optimized out>) at config/power9/bli_cntx_init_power9.c:42 flame#2 0x000000001003c85c in bli_gks_register_cntx (id=BLIS_ARCH_POWER9, nat_fp=0x1004c090 <bli_cntx_init_power9>, ref_fp=0x1004c0d0 <bli_cntx_init_power9_ref>, ind_fp=<optimized out>) at frame/base/bli_gks.c:373 flame#3 0x000000001003c97c in bli_gks_init () at frame/base/bli_gks.c:155 flame#4 0x000000001003cfe8 in bli_init_apis () at frame/base/bli_init.c:78 flame#5 0x00007ffff7e045a8 in __pthread_once_slow () from /lib64/libpthread.so.0 flame#6 0x00000000100492e8 in bli_pthread_once (once=<optimized out>, init=<optimized out>) at frame/thread/bli_pthread.c:314 flame#7 0x000000001003d138 in bli_init_once () at frame/base/bli_init.c:104 flame#8 bli_init_auto () at frame/base/bli_init.c:54 flame#9 0x0000000010011300 in cdotc_ (n=<optimized out>, x=<optimized out>, incx=<optimized out>, y=<optimized out>, incy=<optimized out>) at frame/compat/bla_dot.c:89 flame#10 0x0000000010002a48 in check2_ (sfac=0x103d14dc <sfac>) at blastest/src/cblat1.c:529 flame#11 0x0000000010001ef4 in main () at blastest/src/cblat1.c:112

loveshack · 2019-09-24T10:06:29Z

I should have referenced #322.

Environment variable BLIS_CORETYPE is examined for a value matching an element of the relevant family. This overrides cpuid-style selection (similarly to the mechanism in OpenBLAS). This needs to be documented. It could be used for explicit runtime selection without a cpuid-type implementation.

Details: - Environment variable BLIS_CORETYPE is examined for a value matching an element of the relevant family (e.g. "skx"). This overrides cpuid-style micro-architecture selection (similarly to the mechanism in OpenBLAS). It needs to be documented, possibly for explicit runtime selection without a cpuid-type implementation, or for testing.

See the GCC doc about -march, -mtune, and -mpu and maybe https://community.arm.com/developer/tools-software/tools/b/tools-software-ides-blog/posts/compiler-flags-across-architectures-march-mtune-and-mcpu

Tighten the pattern to avoid generating garbage in the compilation command from auto configuration with the POWER code, and sort to avoid the environment-checking code duplicating the definitions.

fgvanzee · 2019-10-04T16:32:33Z

@loveshack Thanks for your efforts, Dave. This is an unusually busy time for us, so it will be a bit before I can take a look at these modifications.

In the meantime, if you aren't already doing so, I would suggest looking into the AppVeyor failures to see if those are related to the commits on this PR branch or if (perhaps less likely) they are being caused by something else (environment/hardware changes at AppVeyor, for example).

loveshack · 2019-10-06T18:12:22Z

I fixed the failure, after confusion why it seemed MS Windows-specific. I don't know how the export spec got lost.

Also remove unnecessary headers.

Mainly for BLIS_CORETYPE architecture selection.

Avoids test numerical errors (with gcc 9).

This makes a significant difference for dgemm built with -march=native on haswell. Assume that loop nests don't need restructuring, but do need unwinding the inner loop, and we also want to unroll non-nested loops.

Uses the generic kernel, but with configuration for z13 and z14+ which can be used dynamically to use vx (vector instructions) -- double for z13, double and single for z14+.

It can't be necessary, and broke on f31 390x due to a gcc bug. Also it might not work with other compilers that may support -fopenmp-simd.

Set BLIS_NUM_ARCHS automatically by inserting it as the last member in the enum. Also remove `=0` on first enum member since it is guaranteed by the standard. This make is easier and less error-prone to reorder enum entries. This change was also included in changes I made to #344 and #345 in remerging them, but I thought it important to make this change earlier rather than later.

It's GCC 5+.

devinamatthews

Lots of ARM stuff, please separate this out into a separate PR.

devinamatthews · 2020-10-19T20:04:21Z

frame/include/bli_type_defs.h

+// NOTE: This value must be updated to reflect the number of enum values
+// listed above for arch_t!
+#define BLIS_NUM_ARCHS 25
+


This isn't needed anymore, see last entry in arch_t.

Can you explain this comment, Devin?

In another PR, I made BLIS_NUM_ARCHS a member of the arch_t enum (the last one) so that it will always be up-to-date.

Gotcha. I hadn't seen that yet.

devinamatthews · 2020-10-19T20:05:26Z

config/cortexa15/make_defs.mk

 COPTFLAGS      := -O0
 else
-COPTFLAGS      := -O2
+COPTFLAGS      := -O2 -mcpu=cortrex-a15


This is a bit off-topic. Any ARM-related stuff should be moved to its own PR.

devinamatthews · 2020-10-19T20:08:57Z

config/power9/make_defs.mk.rej

+endif
+
+ # Store all of the variables here to new variables containing the
+ # configuration name.


I think this file is "junk", right?

Looks junky to me. What is a .rej file, anyway?

devinamatthews · 2020-10-19T20:09:58Z

config/s390x/bli_family_s390x.h

@@ -0,0 +1 @@
+#define BLIS_SIMD_SIZE 16 // 128-bit SIMD


@fgvanzee's gonna want the whole copyright header in here, maybe some additional boilerplate.

Yes, license header please kthnx.

devinamatthews · 2020-10-19T20:11:05Z

config/z13/bli_family_z13.h

@@ -0,0 +1 @@
+#define BLIS_SIMD_SIZE 16 // 128-bit SIMD


devinamatthews · 2020-10-19T20:28:59Z

frame/base/bli_gks.c

+		bli_gks_register_cntx( BLIS_ARCH_A64FX,   bli_cntx_init_a64fx,
+		                                              bli_cntx_init_a64fx_ref,
+		                                              bli_cntx_init_a64fx_ind );
+#endif


devinamatthews · 2020-10-19T20:29:10Z

frame/base/bli_cpuid.h

+};
+enum
+{
+	MODEL_Z900 = 0,


Is MODEL_Z900 handled?

devinamatthews · 2020-10-19T20:29:42Z

frame/include/bli_arch_config.h


+#ifdef BLIS_CONFIG_A64FX
+CNTX_INIT_PROTS( a64fx )
+#endif


devinamatthews · 2020-10-19T20:29:55Z

frame/include/bli_arch_config.h

+#ifdef BLIS_FAMILY_ARM32
+#include "bli_family_arm32.h"
+#endif
+


devinamatthews · 2020-10-19T20:31:21Z

frame/include/bli_type_defs.h

 	BLIS_ARCH_CORTEXA53,
 	BLIS_ARCH_CORTEXA15,
 	BLIS_ARCH_CORTEXA9,
+	BLIS_ARCH_A64FX,


fgvanzee · 2020-10-19T21:07:19Z

Somehow, notification of Devin's reviews made it into my inbox. Anyhow, while I'm here...

loveshack and others added 9 commits September 23, 2019 11:08

Don't override -mcpu with -march

4799dd5

See the GCC doc concerning priorities.

Add arm64 micro-arch selection logic (flame#299)

2958b14

This only affects compiling with cpu-specific flags, not anything in the micro-kernel.

Document ThunderX2 hardware support

eaed681

Add power8 config and power family (flame#322)

6f23078

power8 follows power9 in just using the reference micro-kernel. Also fix an unused variable warning.

Add comments about arm32 selection

509c4d7

Avoid a warning

d596dd3

Use working URL for ConfigurationHowTo

250f681

Include power family header

67f9458

loveshack added 10 commits September 26, 2019 15:49

ppc64: Match more cpp macros and conditionalize Linux-specifics

d22f787

Add example procinfo for s390x

674edaa

arm64: Conditionalize Linux-specific code

8ffdb0c

Remove old comment and fix indentation

92fa38b

Note .dir-locals.el in docs

804b504

Use -mcpu for ARM

e525e0a

See the GCC doc about -march, -mtune, and -mpu and maybe https://community.arm.com/developer/tools-software/tools/b/tools-software-ides-blog/posts/compiler-flags-across-architectures-march-mtune-and-mcpu

Declare bli_arch_string correctly for last change

bc1412f

Fix configure grepping for BLIS_CONFIG_ in bli_cpuid.c

ee33976

Tighten the pattern to avoid generating garbage in the compilation command from auto configuration with the POWER code, and sort to avoid the environment-checking code duplicating the definitions.

loveshack mentioned this pull request Oct 4, 2019

allow environment to select family members #350

Closed

Replace inadvertently-removed export of bli_arch_string

df27ef3

loveshack added 6 commits October 6, 2019 19:36

Include ARM family headers

85e1e5a

Don't take architecture from environment when configuring

9ef6efd

Also remove unnecessary headers.

Update configuration howto

9f4ab6d

Mainly for BLIS_CORETYPE architecture selection.

Fix parsing in vpu_count on workstation SKX

f811608

Merge branch 'power'

611ff30

Merge branch 'arm64-cpuid'

1a166b4

loveshack added 19 commits August 30, 2020 18:27

Detect s390x, and _LP64 as 64-bit

f044ac9

Fix power build errors

aaff129

Merge branch 'power'

5467b6a

Use -fno-loop-unroll-and-jam -funroll-loops for power8/generic

2af8c8a

Avoids test numerical errors (with gcc 9).

Use -fno-loop-unroll-and-jam -funroll-loops for generic

cdae063

This makes a significant difference for dgemm built with -march=native on haswell. Assume that loop nests don't need restructuring, but do need unwinding the inner loop, and we also want to unroll non-nested loops.

Comment

19705bb

Add s390x family

3bada5d

Uses the generic kernel, but with configuration for z13 and z14+ which can be used dynamically to use vx (vector instructions) -- double for z13, double and single for z14+.

Use only inner loop unrolling for generic kernels

b61c249

Fix conditionals for bli_env_check on power

133b388

Merge branch 'power'

611f7c8

Merge branch 's390x'

0b15084

Merge branch 'arm64-cpuid'

b2220e8

Act on BLIS_CORETYPE generic or invalid (treated as generic)

dc69793

Fix bool_t instances

04d6f47

Don't use -march=native when testing for openmp-simd

3d05722

It can't be necessary, and broke on f31 390x due to a gcc bug. Also it might not work with other compilers that may support -fopenmp-simd.

COmment

022eb6c

Fix s390x config header

5431493

Merge branch 'arm64-cpuid'

9e7bd6c

Merge branch 's390x'

cd45fbc

devinamatthews mentioned this pull request Sep 7, 2020

Update bli_type_defs.h #446

Closed

loveshack added 4 commits September 11, 2020 14:57

Avoid dangling else warning

4b07a51

Fix BLIS_NUM_ARCHS

bce2c4f

Conditionalize use of -fno-loop-unroll-and-jam

eb1e4f2

It's GCC 5+.

Comment

c90000b

devinamatthews requested changes Oct 19, 2020

View reviewed changes

devinamatthews mentioned this pull request May 23, 2021

auto-configure fails on POWER9 #501

Closed

devinamatthews mentioned this pull request Dec 1, 2022

Provide a ppc64le target #691

Closed

devinamatthews mentioned this pull request Feb 1, 2023

add runtime selection of power architecture #718

Merged

Conversation

loveshack commented Sep 24, 2019

Uh oh!

loveshack commented Sep 24, 2019

Uh oh!

fgvanzee commented Oct 4, 2019

Uh oh!

loveshack commented Oct 6, 2019 via email

Uh oh!

devinamatthews left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fgvanzee commented Oct 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants