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

Add rawtodigi implementation using the Cupla backend #2

Merged
merged 3 commits into from
Apr 12, 2019

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Apr 10, 2019

No description provided.

@fwyzard
Copy link
Contributor Author

fwyzard commented Apr 10, 2019

Using CUDA as the cupla backend, I see the same performance as with native CUDA.

Using serial operations as the cupla backend is slower than the naive and CUDA implementations; however it was not optimised, and I am not sure what CUPLA_KERNEL_OPTI is actually doing (vs. the simpler CUPLA_KERNEL call).

@makortel
Copy link
Owner

Thanks, looks good. Before merging I'd like to ask for clarification

This PR fully includes #1, right?

For the the Cupla CPU backend, the input data "transfers" input -> input_h -> input_d are redundant, right?

I found it interesting to see that exactly the same code works fine also in the main.cc side, and the backend is fully controlled with the ALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLED vs. ALPAKA_ACC_GPU_CUDA_ENABLED. I suppose for CMSSW purposes we'd need to compile and link both versions.

@fwyzard
Copy link
Contributor Author

fwyzard commented Apr 11, 2019

This PR fully includes #1, right?

Yes.

For the the Cupla CPU backend, the input data "transfers" input -> input_h -> input_d are redundant, right?

Yes, and I think this is one main limitation of Cupla: to keep the code unchanged, it has to introduce these extra operations.
If we were to use Alpaka directly, it would be up to us to make sure the data are in the correct memory - more work at coding time, potentially less work at runtime,

I suppose for CMSSW purposes we'd need to compile and link both versions.

That is the other problem I am thinking about: both libraries export the same symbols - cuplaMemcpy, cuplaMemcpyAsync, etc., so we cannot just link them both to the same plugin.
I am not sure what happens if we try to link two different backend libraries to two different plugins, but I suspect we could quickly end up with linker and/or runtime issues.

@fwyzard
Copy link
Contributor Author

fwyzard commented Apr 11, 2019

One option could be to put all those symbols in different a namespace for each backend, and rely on the macro definition to pick the right one.

An other option could be to link the cupla backend statically into each plugin.
In fact, cupla itself does this, when one builds with CMake as advertised in the instructions.

@fwyzard
Copy link
Contributor Author

fwyzard commented Apr 11, 2019

Anyway, I am going to try the other backends and check their performance, as well.

@makortel
Copy link
Owner

Thanks, merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants