Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH, TST: Bring the NumPy C SIMD vectorization interface "NPYV" to Python #16782

Merged
merged 11 commits into from
Oct 29, 2020

Conversation

seiko2plus
Copy link
Member

@seiko2plus seiko2plus commented Jul 8, 2020

Implement new CPython extension(_simd module)

A module to bring the NumPy C SIMD vectorization interface "NPYV"

The module is designed to be extremely flexible so that it can accommodate any kind
intrinsics, also to generate a python interface almost similar to the C interface.

The main purpose of this module is to test NPYV intrinsics in python,
but still can be used as an effective solution in designing SIMD kernels.

Also, add a new command-line argument --simd-test to control of targeted
CPU features for the _simd module.

Add testing unit that covers the current implemented intrinsics

based on _simd module.

TODO:

Disabled the following tasks, since I'm not sure which pull-request is going to merge first
[ ] add test unit for math intrinsics(sqrt, square, abs, recip) #16247

@mattip
Copy link
Member

mattip commented Jul 10, 2020

@seiko2plus Can you rebase this off master now that #16397 is merged?

@charris charris added 01 - Enhancement component: numpy._core component: SIMD Issues in SIMD (fast instruction sets) code or machinery labels Jul 12, 2020
@seiko2plus
Copy link
Member Author

@mattip, The binary size of this module depending on the default value of --simd-test is about 1400kb(after striping the debugging info).
The module is mainly used for testing the NPYV through CI, so we can disable it on the building wheel via build --simd-test="".

Copy link
Member

@mattip mattip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, thanks.

numpy/core/src/_simd/_simd.h Outdated Show resolved Hide resolved
numpy/core/src/_simd/_simd.h Outdated Show resolved Hide resolved
numpy/core/src/_simd/_simd.h Outdated Show resolved Hide resolved
numpy/core/src/_simd/_simd.h Outdated Show resolved Hide resolved
numpy/core/src/_simd/_simd_inc.h.src Outdated Show resolved Hide resolved
numpy/core/src/_simd/_simd_inc.h.src Outdated Show resolved Hide resolved
numpy/core/src/_simd/_simd_inc_easyintrin.h Outdated Show resolved Hide resolved
numpy/core/tests/test_simd.py Outdated Show resolved Hide resolved
numpy/core/tests/test_simd.py Outdated Show resolved Hide resolved
numpy/distutils/command/build.py Outdated Show resolved Hide resolved
@mattip
Copy link
Member

mattip commented Jul 21, 2020

circleCI breakage is fixed on master, so this needs a rebase/merge to clear it.

@seiko2plus
Copy link
Member Author

@mattip,

circleCI breakage is fixed on master, so this needs a rebase/merge to clear it.

done, the circleCI should pass it.

@mattip
Copy link
Member

mattip commented Aug 23, 2020

Can you think of a way to redo numpy/core/src/_simd/_simd_inc_easyintrin.h in C rather than as large preprocessor macros?

@seiko2plus
Copy link
Member Author

@mattip, yes but I will need c++(template) or python through pyas_template.py that's how the pyas template looks loops_cmp.dispatch.pyas.c. currently working on simplifying it. I will move it later into a separate pr along with doc and testing unit.

@seiko2plus seiko2plus changed the title ENH, TST: Bring the NumPy C SIMD vectorization interface "NPYV" to Python WIP::ENH, TST: Bring the NumPy C SIMD vectorization interface "NPYV" to Python Aug 31, 2020
@seiko2plus seiko2plus marked this pull request as draft August 31, 2020 10:55
@seiko2plus
Copy link
Member Author

it seems I have to remove the draft status to make Travis and Shippable fetching it

@mattip
Copy link
Member

mattip commented Oct 21, 2020

I think we should merge this to unlock the other PRs. It still needs docstrings, but let's leave that for a future PR. @eric-wieser what do you think?

@mattip
Copy link
Member

mattip commented Oct 26, 2020

I am happy to leave the documentation for another PR, which may involve some UX changes. For instance, in order to use this I must do

from `np.core._simd import targets  # what else does _simd provide?
v1 = targets['baseline'].load_f32(range(4))  # mixing dictionary access to target and modules
v2, v3 = _simd.targets['baseline'].zip_f32(v1, v1)

The size of the module is not too bad for the boost in functionality: it allows trying out all the SIMD intrinsics via python.

@eric-wieser
Copy link
Member

eric-wieser commented Oct 26, 2020

The code for the new modules looks good in this patch, as do the tests that those modules behave as expected.

I haven't reviewed the SIMD tests in this PR that use the new modules to actually do the testing in any detail, but someone else can sign off on those. Alternatively, we could split them to a new PR.

Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TestUtility class seems a bit odd to me. It might make more sense as a standalone class that uses instance state - the suggestions below are incomplete but should give an overall view of what I'm suggesting.

Its tempting to make SuffixInfo class I suggest be part of the _simd module, rather than just in this one test.

numpy/core/tests/test_simd.py Show resolved Hide resolved
numpy/core/tests/test_simd.py Show resolved Hide resolved
numpy/core/tests/test_simd.py Show resolved Hide resolved
seiko2plus and others added 10 commits October 27, 2020 11:46
  '_simd' is a new module to bring the NumPy C SIMD vectorization interface "NPYV"

  The module is designed to be extremely flexible so that it can accommodate any kind
  intrinsics, also to generate a python interface almost similar to the C interface.

  The main purpose of this module is to test NPYV intrinsics in python,
  but still can be used as an effective solution in designing SIMD kernels.

  Also add a new command-line argument `--simd-test` to control of targeted CPU features
  for the `_simd` module.

Co-authored-by: Matti Picus <matti.picus@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
…rectly

Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
 - use plain variables
 - clean up aligned allocate
 - use `PyArg_ParseTuple` for empty args
 - use `Py_ssize_t` instead of `unsigned` and `size_t`
 - improve coding style
 - no need for a custom raises assertions
 - use parametrize instead of inner loops
 - leave a comment about nature of mode testing unit
 - shift to get max/min of int72
 - add more info to repr of vector object
 - get ride of exec() and use type() instead
 - use `.inc` as extension for sub-headers instead of `.h`
 - add `FMA4` and drop `SSE41` from _SIMD targets

Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@mattip
Copy link
Member

mattip commented Oct 29, 2020

@seiko2plus there are a couple of suggestion still open. Could you either accept them or comment? This is really close now, lets finish it up.

@seiko2plus
Copy link
Member Author

@mattip, both comments were part of parameterize() suggestion, I left comments for them.

@mattip mattip merged commit 8829b80 into numpy:master Oct 29, 2020
@mattip
Copy link
Member

mattip commented Oct 29, 2020

Thanks @seiko2plus and @eric-wieser. Please open new issues/PR for parts of this that still need iterating on, it is a large addition so there is bound to be more to finish.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
01 - Enhancement component: numpy._core component: SIMD Issues in SIMD (fast instruction sets) code or machinery
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants