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, SIMD: Add support for dispatching C++ sources #18837

Merged
merged 1 commit into from
Apr 23, 2021

Conversation

seiko2plus
Copy link
Member

Same usage as the C dispatch-able sources except files extensions
should be `.dispatcher.cpp` or `.dispatch.cxx` rather than `.dispatch.c`

@seiko2plus seiko2plus added 01 - Enhancement component: SIMD Issues in SIMD (fast instruction sets) code or machinery labels Apr 22, 2021
    Same usage as the C dispatch-able sources except files extensions
    should be `.dispatcher.cpp` or `.dispatch.cxx` rather than `.dispatch.c`
@charris
Copy link
Member

charris commented Apr 23, 2021

Hmm, this brings up the question of how we want to support c++. Do we want to modify the ccompiler or make a new cppcompiler. Or use meson :) @rgommers @mattip @seberg Thoughts?

@seberg
Copy link
Member

seberg commented Apr 23, 2021

Wouldn't meson be more of a mid- to longerterm solution/replacement, while we would like to try function templates fairly soon? And I guess most of distutils does all of this already and this is just generalizes the same things to the simd dispatching?

So, I know neither this code well nor the C++ deployment in general, but it seems fine to just go with it.

@rgommers
Copy link
Member

And I guess most of distutils does all of this already and this is just generalizes the same things to the simd dispatching?

Exactly, SciPy has lots of C++ code and happily uses numpy.distutils. There's nothing missing aside from this PR AFAIK.

@rgommers
Copy link
Member

@seiko2plus this PR looks good, but since there's no C++ code yet it's hard to test. I'm not suggesting you add in C++ code here (that would make it large and hard to merge). But did you test it locally? Is there a branch or WIP PR you could share to test this? Or would you prefer to just merge this and iterate on it if it later turns out to be incomplete?

@seiko2plus
Copy link
Member Author

@rgommers,

But did you test it locally?

Sure, I did.

Is there a branch or WIP PR you could share to test this?

The current work is kinda a miss a little bit and I'm working on splitting them into 4 PRs:

  • Add support for dispatching C++ sources(this one)
  • Add support for Doxygen through Breathe
  • Several fixes to the C headers that brings compatibility with c++
  • C++ wrappers for C universal intrinsics(NPYV) [documented]

Or would you prefer to just merge this and iterate on it

Yes, that what I prefer but I can add a testing unit for C++ dispatching if it's nesseary. What do you think?

@charris charris merged commit a9409e1 into numpy:main Apr 23, 2021
@charris
Copy link
Member

charris commented Apr 23, 2021

but I can add a testing unit for C++ dispatching

Let's wait until the rest gets in and is working. I'm not sure what we should be testing at this point.

Thanks Sayed.

@rgommers
Copy link
Member

Add support for Doxygen through Breathe

That seems fine, it looks like Breathe is pretty actively maintained right now.

The related Exhale (https://github.com/svenevs/exhale) is best avoided, it's unmaintained.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
01 - Enhancement 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