GeezerVison

I am concerned all this will become a maintenance burden moving forward. Is there a way to use this as a library or a subrepo?

What does this comment about library or subrepo mean? This sounds very hostile to NumPy supporting other architectures.

I am concerned all this will become a maintenance burden moving forward. Is there a way to use this as a library or a subrepo?

@mattip this is quite clearly addressed in the discussion that spurred this PR: gh-13393. And we also discussed it with the Steering Council before saying yes to posting the bounty mentioned in gh-13393. So yes it's a concern, but if things are done the right way there's no reason that this should substantially increase the maintenance burden. Please review that issue and comment on code or ideas that are potentially problematic, no need to start a new abstract discussion.

This sounds very hostile to NumPy supporting other architectures.

@edelsohn that's certainly not our intent as a project. We'd really like to have complete SIMD support for PowerPC as well as other architectures, with the compiler doing most of the heavy lifting.

That said, this is quite a bit of code. I believe a lot of it is well-tested though, taken over from OpenCV (correct me if I'm wrong @seiko2plus). Some more context in the issue description would be useful:

• there's a TODO for an example
• this contains both build-time and run-time features. does that change the situation we have today? IIRC we wanted to move more to run-time, is that achieved?
• what parts are and aren't covered by CI (we have PPC on TravisCI, were thinking about ARM via drone.io I believe, and conda-forge has ARM too)?

What does this comment about library or subrepo mean?

It is simply another way to organize the code. NumPy already has two submodules that provide tools to build the documentation, putting the code in a submodule allows the NumPy to use the code while letting development proceed at its own pace. Likewise, NumPy builds several libraries for the sorting and math code, probably not strictly necessary these days, but routines in the libraries are linked in when needed, and strictly speaking could be used by downstream projects. You have a number of new files dedicated to this work, so it might be nice to put them together in subdirecties, maybe organized by architecture. Note that the libraries have their own setup and include files and configure the system. So the question here is how should we organize the code, not whether we should have it.

It seems to me, after a cursory inspection, that how you want to do the distutils integration might be dispositive. It would be nice to package this up into a separate library with its own distutils integration tooling so that I could write other packages that use this code without needing to use numpy.distutils. numpy.distutils works well for numpy and scipy, but we try not to make it necessary for packages that just want to build against numpy's functionality.

On the other hand, I recognize that the distutils integration required is fairly significant, and welding together two significant distutils augmentations is often fraught, so I can see an argument for avoiding it and just making this one more feature that numpy.distutils alone provides, like f2py. If that's the case, then numpy/numpy is the right place. There isn't much else that would be profitable to break out into a separate package.

One thing to note is that we ought to be able to use this infrastructure to build another package that uses CPU features that the numpy binary itself was not compiled to use. I don't see why that would not be the case in this PR, but I just wanted it stated explicitly. Organizing this as a separate library may help enforce that when future modifications are made.

@mattip,

I am concerned all this will become a maintenance burden moving forward.

A maintenance burden already moving forward, I think what I'm doing is just trying to ease the burden.

Is there a way to use this as a library or a subrepo?

I'm afraid not or at least for now , because the universal intrinsics (mapped intrinsics) are going to be heavily expanded depending on the needs and I think it should be part of NumPy's core, later we should forbid or at least reduce the direct use of raw SIMD intrinsics and only count on universal intrinsics for CPU optimizations.

@edelsohn,

This sounds very hostile to NumPy supporting other architectures.

Actually, most of the work here is mainly for the x86 due to the need to dealing with many SIMD extensions, compilers, and platforms which make it kinda complicated and I don't see VSX or even NEON create any obstacles here, maybe supporting BE mode for VSX will be a little bit challengable but I'm planning to implement it in a separate pull-request after I get done from re-implement the current optimized kernels using the new SIMD interface.

@rommers,

That said, this is quite a bit of code.

Yes, well I had to add some fundamental universal intrinsics just for testing propose but sure I will add a lot more depending on the needs.

I believe a lot of it is well-tested though

I believe we shouldn't put our trust in any intrinsics even if it directly mapping to native instruction-set since
compilers aren't developed by angels, that may be why we should try to cover all the used intrinsics within a unit testing.

taken over from OpenCV (correct me if I'm wrong @seiko2plus)

Like I mentioned before in #13393 the whole road map is inspired by OpenCV but not exactly the same design or code but I can tell the similarity is very high especially to infrastructure.

there's a TODO for an example

Of course, I will not be able to cover all areas in this pr but I think it's necessary to provide a decent example for illustrative and testing purposes and please feel free to add any extra tasks.

what parts are and aren't covered by CI (we have PPC on TravisCI, were thinking about ARM via drone.io I believe, and conda-forge has ARM too)?

if it's necessary having a custom CI using "buildbot" could be an alternative solution too.

@charris,

You have a number of new files dedicated to this work, so it might be nice to put them together in subdirecties, maybe organized by architecture. Note that the libraries have their own setup and include files and configure the system.

Do you think the current work isn't organized enough?

So the question here is how should we organize the code, not whether we should have it

Although this work still not completed yet, I would truly appreciate it if you could provide a proposal or any kind of notes.

@rkern,

it seems to me, after a cursory inspection, that how you want to do the distutils integration might be dispositive. It would be nice to package this up into a separate library with its own distutils integration tooling so that I could write other packages that use this code without needing to use numpy.distutils. numpy.distutils works well for numpy and scipy, but we try not to make it necessary for packages that just want to build against numpy's functionality.

On the other hand, I recognize that the distutils integration required is fairly significant, and welding together two significant distutils augmentations is often fraught, so I can see an argument for avoiding it and just making this one more feature that numpy.distutils alone provides, like f2py. If that's the case, then numpy/numpy is the right place. There isn't much else that would be profitable to break out into a separate package.

One thing to note is that we ought to be able to use this infrastructure to build another package that uses CPU features that the numpy binary itself was not compiled to use. I don't see why that would not be the case in this PR, but I just wanted it stated explicitly. Organizing this as a separate library may help enforce that when future modifications are made.

Sure it is a great idea and it seems like the community prefer this way (a separate package) but wait as you know every project have his own needs, that means you have to find compatibilities, flexible design which means more time, more issues and I think we should focus on the main purpose instead at least for now and have some fun by optimize some kernels :).

if it's necessary having a custom CI using "buildbot" could be an alternative solution too.

no not necessary, and in fact very much undesired. we used to have those in the pre-TravisCI age and they're a maintenance nightmare. we'll just cover what we can with the hosted CI platforms we have, and the rest we test manually at PR develop/merge time and rely on community input to keep things working. that's the way it is now too for AIX, Sparc, etc. we'll just slowly expand coverage as we go

I think we should merge this as-is. With a PR this large, there will always be more to clean up around documentation and comments. PR gh-15551 adds a document to describe the mechanism here, and should be merged after this PR.

Thanks @seiko2plus. Step 1 is done!

I started to love the purple color, thank you!

@seiko2plus This PR raises compiler warnings with fedora gcc 10 when AVX is missing.

/home/charris/Workspace/numpy.git/numpy/distutils/checks/cpu_avx512f.c:5:13: error: AVX512F vector return without AVX512F enabled changes the ABI [-Werror=psabi]
    5 |     __m512i a = _mm512_abs_epi32(_mm512_setzero_si512());
      |             ^
In file included from /usr/lib/gcc/x86_64-redhat-linux/10/include/immintrin.h:55,
                 from /home/charris/Workspace/numpy.git/numpy/distutils/checks/cpu_avx512f.c:1:
/usr/lib/gcc/x86_64-redhat-linux/10/include/avx512fintrin.h:15594:1: error: inlining failed in call to ‘always_inline’ ‘_mm512_castsi512_si128’: target specific option mismatch
15594 | _mm512_castsi512_si128 (__m512i __A)
      | ^~~~~~~~~~~~~~~~~~~~~~

EDIT: Also for other avx checks. Seems the checks should not be compiled without AVX.

@charris, class CCompilerOpt is doing two tests for each required feature to determine the compiler support:

• check the flags without any involved intrinsics, to determine the availability of flags.
• check the test file with or without the flags depends on the availability of flags(friendly with msvc).

If both tests failed, then the feature will be skipped, but since we're dealing with GCC-10 both tests should be passed.
And according to the provided error, CCompilerOpt was testing the check file cpu_avx512f.c without providing the avx512f flag -mavx512f which means, the flags test was failed.

However, I tested (GCC version 10.1.0) and it looks fine to me.
please could you provide the build log or provide at least the failure part that related to testing flag -mavx512f?

@seiko2plus I don't see that warning anymore, may have changed with a recent kernel update. Let me try with an older kernel.

No, they are still there, but tests run fine. I think the errors are configuration output that runtests.py should not be showing, so this is probably some sort of build organization problem. Are those the sort of errors you would expect during configuration? @mattip IIRC, you were the one who suppressed the configuration output, thoughts?

Could you put the complete build log up somewhere like https://paste.ubuntu.com/ ?

@mattip Here: https://paste.ubuntu.com/p/VH7x8qJkVS/ . Search for error:.

@charris, umm well it seems '-march=native' is provided through environment variable CFLAGS or it is part of distutils config vars. see the final build log:

CPU baseline  : 
  Requested   : 'native'
  Enabled     : SSE SSE2 SSE3 SSSE3 SSE41 POPCNT SSE42 AVX F16C FMA3 AVX2
  Flags       : -msse -msse2 -msse3 -mssse3 -msse4.1 -mpopcnt -msse4.2 -mavx -mf16c -mfma -mavx2

CPU dispatch  : 
  Requested   : 'max -xop -fma4'
  Enabled     : AVX512F AVX512CD AVX512_KNL AVX512_KNM AVX512_SKX AVX512_CLX AVX512_CNL AVX512_ICL
  Generated   : none
CCompilerOpt._cache_write[786] : write cache to path -> /home/charris/Workspace/numpy.git/build/temp.linux-x86_64-3.9/ccompiler_opt_cache_ext.py

########### CLIB COMPILER OPTIMIZATION ###########
CPU baseline  : 
  Requested   : 'native'
  Enabled     : SSE SSE2 SSE3 SSSE3 SSE41 POPCNT SSE42 AVX F16C FMA3 AVX2
  Flags       : -msse -msse2 -msse3 -mssse3 -msse4.1 -mpopcnt -msse4.2 -mavx -mf16c -mfma -mavx2

CPU dispatch  : 
  Requested   : 'max -xop -fma4'
  Enabled     : AVX512F AVX512CD AVX512_KNL AVX512_KNM AVX512_SKX AVX512_CLX AVX512_CNL AVX512_ICL
  Generated   : none
CCompilerOpt._cache_write[786] : write cache to path -> /home/charris/Workspace/numpy.git/build/temp.linux-x86_64-3.9/ccompiler_opt_cache_clib.py

The failure messages are normal in this case since CCompilerOpt was testing all features with the native flag to determine
the supported features by the running machine during the build time.

This is part of "build_clib" (line 627). The configuration work is done during "build_src" (line 135). Maybe we should do the same tricks during CCompilerOpt to ignore warnings that we do during build_src.

@mattip, why we should suppress the error messages?

@charris,

I think the errors are configuration output that runtests.py should not be showing, so this is probably some sort of build organization problem.

Yes, It should be showing if --show-build-log is enabled.

Are those the sort of errors you would expect during configuration?

Yes, when the compiler doesn't support certain feature.

I am not sure whether we should suppress the warnings when someone specifies -mnative or not.