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.

Sign up for GitHub

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

Jump to bottom

#857: conditionally rely on PARALLEL WORKSHARE #859

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

#857: conditionally rely on PARALLEL WORKSHARE #859

wants to merge 1 commit into from

Conversation

hfp
Copy link
Member

@hfp hfp commented Oct 16, 2024 •
edited
Loading

  • Avoid nested parallelism (dbcsr_acc_set_active_device).
  • Rely on omp_get_level (instead of omp_in_parallel),
    omp_in_parallel only accounts for active regions.
  • Avoid IF-condition as part of the WORKSHARE-directive.
  • Removed WORKSHARE construct from dbcsr_ptr_util,
  • otherwise keep WORKSHARE if not in parallel region.
  • There is potentially invalid nesting (parallel+X),
    e.g., nested in master or sections constructs.

@alazzaro alazzaro self-assigned this Oct 16, 2024
@alazzaro
Copy link
Member

Thanks @hfp I will take a look next week...

@hfp hfp force-pushed the issue857 branch 2 times, most recently from e599ada to 9105048 Compare October 16, 2024 10:35
@hfp
Copy link
Member Author

hfp commented Oct 16, 2024 •
edited
Loading

Thanks @hfp I will take a look next week...

Might be easier now (less work, only three files). I separated some cleanup work (#860).

@hfp
Copy link
Member Author

hfp commented Oct 16, 2024 •
edited
Loading

@mkrack this single PR is apparently resolving all runtime issues with Intel Fortran Compiler (IFX) in CP2K/DBCSR. There is still the question why PARALLEL WORKSHARE DEFAULT(none) SHARED(...) IF(spawn) with spawn=.NOT. omp_in_parallel() does not work as opposed to the code shown here (but this might be well-covered by a bug report). Further, see #857 (comment) - it can be of interest for CP2K.

@hfp
Copy link
Member Author

hfp commented Oct 21, 2024

@alazzaro any feedback welcome - so far, no issues with this code on my side, I am actively using it since sending the PR.

@hfp
Copy link
Member Author

hfp commented Oct 23, 2024

Objective of this implementation is to exercise a way to preserve the (parallel) workshare. It does not judge (or confirms) whether the workshare is beneficial or not.

@hfp hfp mentioned this pull request Oct 23, 2024
Merged
@alazzaro
Copy link
Member

@hfp thanks, I will review everything next week and make a new RC that we can push to CP2K for more testings

@hfp hfp mentioned this pull request Oct 25, 2024
Merged
@hfp hfp force-pushed the issue857 branch 2 times, most recently from 77b1096 to f684cd2 Compare November 4, 2024 19:01
@hfp hfp force-pushed the issue857 branch 3 times, most recently from c513c8b to a4a3050 Compare November 11, 2024 20:15
@hfp hfp force-pushed the issue857 branch 2 times, most recently from b313c16 to e3fc8e4 Compare November 20, 2024 14:36
@hfp
cp2k#857: conditionally rely on PARALLEL WORKSHARE
39b8532 
- Avoid nested parallelism (dbcsr_acc_set_active_device).
- Rely on omp_get_level (instead of omp_in_parallel),
  omp_in_parallel only accounts for active regions.
- Avoid IF-condition as part of the WORKSHARE-directive.
- Removed WORKSHARE construct from dbcsr_ptr_util,
- otherwise keep WORKSHARE if not in parallel region.
- There is potentially invalid nesting (parallel+X),
  e.g., nested in master or sections constructs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alazzaro alazzaro

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hfp @alazzaro