ENH, SIMD: Optimize/vectorize comparison and logical operations for VSX by rafaelcfsousa · Pull Request #21258 · numpy/numpy

rafaelcfsousa · 2022-03-28T16:21:51Z

This PR optimizes the following operations for Power/VSX:

equal, not_equal, less, less_equal, greater and greater_equal
logical_and, logical_not and logical_or
absolute

See below the average speedup with N=1000000:

Comparisson functions:
- arry OP arr: bool (32.77x) float(3.36x) double(1.43x)
- arry OP scalar: bool (33.47x) float(6.34x) double(2.25x)
Logical functions:
- logical_and: bool (118.12x)
- logical_not: bool ( 21.41x)
- logical_or: bool ( 24.40x)
Miscellaneous:
- absolute: bool (23.12x)

Note: This PR enables the optimizations above for Power8, Power9 and Power10.

rafaelcfsousa · 2022-04-13T14:06:20Z

Hi, I would like to request a review of my code (cc: @mattip , @seiko2plus )

The errors in the CI seem to be something related to timeout. The PRs
of the first page were closed and opened (in sequence) last week.

Thanks!

mattip · 2022-04-13T21:06:25Z

It seems this is a bit orthogonal to the SIMD dispatching framework. It is defining new loops with specific vsx labels, rather than trying to use the existing macro names. Is there a reason you chose this path?

Since you are redefining the dispatching, we need to be very sure you are not changing the dispatching for the other architectures. Can you run benchmarks on an AVX512 machine to make sure nothing has changed?

rafaelcfsousa · 2022-04-14T19:00:26Z

It seems this is a bit orthogonal to the SIMD dispatching framework. It is defining new loops with specific vsx labels, rather than trying to use the existing macro names. Is there a reason you chose this path?

Since you are redefining the dispatching, we need to be very sure you are not changing the dispatching for the other architectures. Can you run benchmarks on an AVX512 machine to make sure nothing has changed?

Hi @mattip, thank you for reviewing my code!

The source file loops.c.src does not support the Universal SIMD Intrinsics (please, correct me if I am wrong). In my understanding, this source file (loops.c.src) includes the file simd.inc.src, which provides optimized implementation using specific SIMD intrinsics for some of the operations defined in loops.c.src. What I did was add optimized implementations for VSX/Power in the file simd.inc.src for the operations defined in loops.c.src that were not being optimized by the compiler automatically.

The source file loops.c.src also does not make use of the SIMD dispatching framework. Instead, the appropriated kernels are selected (statically) during the setup/building according to the support of the compiler being used.

The changes of this PR do not affect x86 (AVX2, AVX512, etc) since they are enabled only for VSX/Power. I also executed benchmarks on an AVX512 machine to make sure that it would not generate issues in terms of performance.

P.S.: I just pushed a new commit to avoid unnecessary code that was being generated for x86. Now, with/without this PR the object generated from loops.c.src for x86 has the same size (1080032B).

mattip · 2022-04-27T07:37:01Z

          docstrings.get('numpy.core.umath.logical_and'),
          'PyUFunc_SimpleBinaryComparisonTypeResolver',
-          TD(nodatetime_or_obj, out='?', simd=[('avx2', ints)]),
+          TD(bints+inexact, out='?', simd=[('avx2', ints), ('vsx', '?fd')]),


bints+inexact is equivalent to nodatetime_or_obj, let's leave this as it was

Suggested change

TD(bints+inexact, out='?', simd=[('avx2', ints), ('vsx', '?fd')]),

TD(nodatetime_or_obj, out='?', simd=[('avx2', ints), ('vsx', '?fd')]),

We have a lint error with this change, should I fix it?

mattip · 2022-04-27T07:44:06Z

                "xmmintrin.h",  # SSE
                "emmintrin.h",  # SSE2
                "immintrin.h",  # AVX
+                "altivec.h",  # VSX


Did this used to be available on macOS too?

Hi @mattip, this line generates the macro #define HAVE_ALTIVEC_H 1 in the source file numpy/core/include/numpy/config.h, which means that, if I remove it the optimizations of this PR wouldn't be generated for VSX/Power.

mattip · 2022-04-27T07:46:38Z

A few minor nits, this looks good to me. @seiko2plus any thoughts?

This commit optimizes the following operations for Power/VSX: - equal, not_equal, less, less_equal, greater and greater_equal - logical_and, logical_not and logical_or - absolute See below the average speedup with N=1000000: - Comparisson functions: - arry OP arr: bool (32.77x) float(3.36x) double(1.43x) - arry OP scalar: bool (33.47x) float(6.34x) double(2.25x) - Logical functions: - logical_and: bool (118.12x) - logical_not: bool ( 21.41x) - logical_or: bool ( 24.40x) - Miscellaneous: - absolute: bool (23.12x)

seiko2plus

sorry for delay response, but the current need to modified to accomplish the following:

using dispatch-able sources rather then compiler target attribute
using universal intrinsics instead of raw SIMD.

seiko2plus · 2022-04-28T00:53:58Z

          'PyUFunc_AbsoluteTypeResolver',
-          TD(bints+flts+timedeltaonly, dispatch=[('loops_unary_fp', 'fd')]),
+          TD(ints+flts+timedeltaonly, dispatch=[('loops_unary_fp', 'fd')]),
+          TD('?', out='?', simd=[('vsx', '?')]),


Suggested change

TD('?', out='?', simd=[('vsx', '?')]),

TD('?', out='?', dispatch=[('loops_comparison', '?')]),

please don't follow our legacy code, dispatch-able sources should be used instead, easy to trace, can be manged through cpu options(--cpu-dispatch, --cpu-baseline) wthin the buid.

seiko2plus · 2022-04-28T01:04:30Z

+ * 2) 8x 64-bit vectors, for DOUBLE.
+ */
+
+static void NPY_GCC_OPT_3 NPY_INLINE NPY_GCC_TARGET_VSX


please use universal intrinsics instead, it will require adding new intrinsics for packing boolean values, the rest of intrinsics already implemented.

npyv_b8 npyv_pack_b8_b16(npyv_b16 a, npyv_b16 b); npyv_b8 npyv_pack_b8_b32(npyv_b32 a, npyv_b32 b, npyv_b32 c, npyv_b32 d); npyv_b8 npyv_pack_b8_b64(npyv_b64 a, npyv_b64 b, npyv_b64 c, npyv_b64 d, npyv_b64 e, npyv_b64 f, npyv_b64 g, npyv_b64 h);

seiko2plus · 2022-04-28T01:10:11Z

@rafaelcfsousa,

The source file loops.c.src does not support the Universal SIMD Intrinsics (please, correct me if I am wrong). In my understanding, this source file (loops.c.src) includes the file simd.inc.src, which provides optimized implementation using specific SIMD intrinsics for some of the operations defined in loops.c.src. What I did was add optimized implementations for VSX/Power in the file simd.inc.src for the operations defined in loops.c.src that were not being optimized by the compiler automatically.

just create new dispatch-able source e.g. loops_comparison.c.src, and move the implementation of inner loops inside it. for examples check #20397, #20363.

EDIT: add another example.

mattip · 2022-05-05T06:13:45Z

@rafaelcfsousa the cutoff for the next version of NumPy is getting close.

rafaelcfsousa · 2022-05-06T13:23:49Z

Hi @mattip, I already have the code implemented, and will spend 1-2 days executing testing / benchmarking.

Instead of submitting the changes here, I will open a new PR as the code is completely different from the one of this PR. If you prefer I submit here, let me know.

What I did:
I moved all comparison functions (all integer types, boolean, float32 and double) to a new dispatchable source file. I used the universal simd intrinsics and I followed what was suggested by @seiko2plus above.

Well, as I do not have access to an ARM processor for testing, can I rely on the CI for this task? (not sure how to do that)

mattip · 2022-05-06T13:27:01Z

Cool. A new PR is fine, whatever is easiest for you

can I rely on the CI for this task

Yup. If you have doubts, please point them out. I have access to aarch64 (linux) and arm64 (macOS) machines.

rafaelcfsousa · 2022-05-09T21:42:16Z

I will close this PR as I opened another PR to implement the changes requested above (#21483).

github-actions Bot added the 01 - Enhancement label Mar 28, 2022

rafaelcfsousa force-pushed the p10_enh_comparison branch from 59cf6b1 to 957f601 Compare March 28, 2022 17:13

charris closed this Apr 6, 2022

charris reopened this Apr 6, 2022

rafaelcfsousa force-pushed the p10_enh_comparison branch from 957f601 to 680bda0 Compare April 14, 2022 18:50

mattip reviewed Apr 27, 2022

View reviewed changes

Comment thread numpy/core/code_generators/generate_umath.py Outdated

mattip reviewed Apr 27, 2022

View reviewed changes

rafaelcfsousa force-pushed the p10_enh_comparison branch from 680bda0 to 7e55cbe Compare April 28, 2022 00:33

seiko2plus requested changes Apr 28, 2022

View reviewed changes

rafaelcfsousa closed this May 9, 2022

	TD(bints+inexact, out='?', simd=[('avx2', ints), ('vsx', '?fd')]),
	TD(nodatetime_or_obj, out='?', simd=[('avx2', ints), ('vsx', '?fd')]),

	TD('?', out='?', simd=[('vsx', '?')]),
	TD('?', out='?', dispatch=[('loops_comparison', '?')]),

Uh oh!

Uh oh!

Conversation

rafaelcfsousa commented Mar 28, 2022

Uh oh!

rafaelcfsousa commented Apr 13, 2022

Uh oh!

mattip commented Apr 13, 2022

Uh oh!

rafaelcfsousa commented Apr 14, 2022

Uh oh!

mattip Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

rafaelcfsousa Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

rafaelcfsousa Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mattip Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

rafaelcfsousa Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

mattip commented Apr 27, 2022

Uh oh!

seiko2plus left a comment

Choose a reason for hiding this comment

Uh oh!

seiko2plus Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

seiko2plus Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

seiko2plus commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattip commented May 5, 2022

Uh oh!

rafaelcfsousa commented May 6, 2022

Uh oh!

mattip commented May 6, 2022

Uh oh!

rafaelcfsousa commented May 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

seiko2plus commented Apr 28, 2022 •

edited

Loading