Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add missing dollar # OpenMP directive. #841

Merged
merged 1 commit into from
Feb 13, 2023
Merged

Conversation

ivan-pi
Copy link
Member

@ivan-pi ivan-pi commented Feb 13, 2023

Closes #840.

@ivan-pi ivan-pi assigned ivan-pi and unassigned ivan-pi Feb 13, 2023
@ivan-pi
Copy link
Member Author

ivan-pi commented Feb 13, 2023

I suppose a simple unit test could look as follows:

program test_get_os_type

!$ use omp_lib, only: omp_get_num_threads, omp_get_thread_num, &
!$    omp_get_max_threads
   use fpm_environment, only: get_os_type
   implicit none

   integer, allocatable :: os(:)
   integer :: n, i

   n = 1
!$ n = omp_get_max_threads()
   print *, "num_threads = ", n

   allocate(os(0:n-1))

   i = 0
!$omp parallel
!$ i = omp_get_thread_num()
   os(i) = get_os_type()
!$omp end parallel

   print *, all(os == os(0))

end program

The results I get:

$ fpm test test_get_os_type --flag -fopenmp
 num_threads =            4
 T

From a quick glance at CI.yml I don't think the parallel build features are being tested currently.

A few questions related to this:

  • Does the bootstrap fpm require parallel features?
  • Would it suffice to add another parallel test target? This would be similar to the one near line:
    - name: Test Fortran fpm
    , but with the addition of the -fopenmp flag.
  • Should the CI target run all tests, or only the ones that might use parallel execution?

Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Good catch, thanks for sharing.

@awvwgk
Copy link
Member

awvwgk commented Feb 13, 2023

In case we want to support testing with threading enabled we should still run the full testing IMO.

@awvwgk awvwgk merged commit f8c901c into fortran-lang:main Feb 13, 2023
@ivan-pi ivan-pi deleted the ompfix branch February 13, 2023 13:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing dollar # OpenMP threadprivate directive
2 participants