Skip to content
This repository was archived by the owner on Mar 25, 2025. It is now read-only.

Conversation

@olupton
Copy link
Contributor

@olupton olupton commented Dec 13, 2021

See BlueBrain/CoreNeuron#713 for context.

* Use `nrn_pragma_acc(...)` in place of `#pragma acc ...`.
* Add OpenMP target offload directives with `nrn_pragma_omp(...)`.
@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #28911 (:white_check_mark:) have been uploaded here!

Status and direct links:

@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #28927 (:white_check_mark:) have been uploaded here!

Status and direct links:

Changes towards XLC/LLVM compiler support for GPU offload.
* Do not include cuda.h and openacc.h.
* Use cnrn_target_ wrappers instead of acc_ API.
* OpenMP offload: simd can not be nested within target parallel loop

Co-authored-by: Pramod Kumbhar <[email protected]>
@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #29141 (:white_check_mark:) have been uploaded here!

Status and direct links:

Copy link
Contributor

@iomaganaris iomaganaris left a comment

Choose a reason for hiding this comment

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

LGTM

* Use nrn_pragma_{acc,omp} and CORENEURON_ENABLE_GPU.
* Add EIGEN_DEVICE_FUNC to header to fix a compilation warning.
* Fudge partialPivLu<N> for NVHPC + OpenMP without OpenACC.
@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #29749 (:white_check_mark:) have been uploaded here!

Status and direct links:

@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #29798 (:white_check_mark:) have been uploaded here!

Status and direct links:

@codecov-commenter
Copy link

Codecov Report

Merging #783 (8535e82) into master (6434099) will increase coverage by 0.01%.
The diff coverage is 11.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #783      +/-   ##
==========================================
+ Coverage   61.42%   61.44%   +0.01%     
==========================================
  Files         205      205              
  Lines       29849    29895      +46     
==========================================
+ Hits        18335    18368      +33     
- Misses      11514    11527      +13     
Impacted Files Coverage Δ
src/codegen/codegen_acc_visitor.cpp 0.54% <0.00%> (-0.05%) ⬇️
src/codegen/codegen_acc_visitor.hpp 0.00% <ø> (ø)
src/codegen/codegen_c_visitor.hpp 91.86% <ø> (ø)
src/solver/newton/newton.hpp 94.33% <ø> (ø)
test/unit/codegen/codegen_c_visitor.cpp 100.00% <ø> (ø)
src/codegen/codegen_c_visitor.cpp 68.50% <50.00%> (+0.03%) ⬆️
src/language/templates/visitors/ast_visitor.cpp 52.53% <0.00%> (-0.36%) ⬇️
test/unit/visitor/semantic_analysis.cpp 100.00% <0.00%> (ø)
src/visitors/semantic_analysis_visitor.cpp 100.00% <0.00%> (ø)
src/visitors/semantic_analysis_visitor.hpp 100.00% <0.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6434099...8535e82. Read the comment docs.

@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #30124 (:white_check_mark:) have been uploaded here!

Status and direct links:

* Update Eigen submodule commit with eigen#2.
* Cherry-pick setuptools fix #786.

Co-authored-by: Nicolas Cornu <[email protected]>
@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #30232 (:white_check_mark:) have been uploaded here!

Status and direct links:

@olupton olupton marked this pull request as ready for review December 22, 2021 08:42
Copy link
Contributor

@pramodk pramodk left a comment

Choose a reason for hiding this comment

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

LGTM 👌

printer->add_line("ThreadDatum* device_thread = cnrn_target_deviceptr(thread);");
printer->add_line(
" acc_memcpy_to_device(&(device_thread[{}]._pvoid), &device_ns, sizeof(void*));"_format(
"cnrn_target_memcpy_to_device(&(device_thread[{}]._pvoid), &device_ns);"_format(
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious - was auto deduced type was different here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think for this one it was important to have device_ns be void* rather than NewtonSpace*.

This is because now we have

template <typename T>
void cnrn_target_memcpy_to_device(T*, T*) { ... }

and if you pass void** and NewtonSpace** it will fail to deduce T. We have to pass two pointers of the same type (up to const qualification).

printer->start_block("if(nt->compute_gpu)");
printer->add_line("double* device_vec = cnrn_target_copyin(vec, vec_size / sizeof(double));");
printer->add_line("void* device_ns = cnrn_target_deviceptr(*ns);");
printer->add_line("ThreadDatum* device_thread = cnrn_target_deviceptr(thread);");
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious - was auto deduced type was different here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think auto or auto* would also be fine.
I believe I just left the type in so it was still spelled out once on the line.

@olupton olupton merged commit 46f8baf into master Dec 22, 2021
@olupton olupton deleted the hackathon_main branch December 22, 2021 16:59
JCGoran pushed a commit to neuronsimulator/nrn that referenced this pull request Mar 12, 2025
* Tweak NVHPC warning suppressions.
* Emit nrn_pragma_{acc,omp}(...) macros. (BlueBrain/nmodl#780)
* Use cnrn_target_ wrappers instead of acc_ API.
* GPU code generation improvements (BlueBrain/nmodl#782)
* Fix NVHPC + OpenMP ~ OpenACC compilation (BlueBrain/nmodl#784)
* Add EIGEN_DEVICE_FUNC to header to fix a compilation warning.
* Fudge partialPivLu<N> for NVHPC + OpenMP without OpenACC.
* Transfer ml only if cell is not artificial. (BlueBrain/nmodl#785)
* Update Eigen to include OpenMP fixes. (BlueBrain/nmodl#787, BlueBrain/nmodl#789)

Co-authored-by: Nicolas Cornu <[email protected]>
Co-authored-by: Pramod Kumbhar <[email protected]>

NMODL Repo SHA: BlueBrain/nmodl@46f8baf
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants