OpenMP-Validation-and-Verification / OpenMP_VV

OpenMP Offloading Validation & Verification Suite; Official repository. We have migrated from bitbucket!! For documentation, results, publication and presentations, please check out our website ->
https://crpl.cis.udel.edu/ompvvsollve/
Other
54 stars 19 forks source link

5.1/teams/test_target_set_num_teams.c: error: OpenMP runtime API call ‘omp_get_max_teams’ strictly nested in a ‘teams’ region #792

Closed tob2 closed 8 months ago

tob2 commented 8 months ago

Pull Request #728 added 5.1/teams/test_target_set_num_teams.c – which fails to compile with GCC mainline/14 with:

tests/5.1/teams/test_target_set_num_teams.c: In function ‘main’:
tests/5.1/teams/test_target_set_num_teams.c:28:37: error: OpenMP runtime API call ‘omp_get_max_teams’ strictly nested in a ‘teams’ region
   28 |                         num_teams = omp_get_max_teams();
      |                                     ^~~~~~~~~~~~~~~~~~~

https://github.com/SOLLVE/sollve_vv/blob/802c0a8b3e3ae08d0691acd55e40c940aea57768/tests/5.1/teams/test_target_set_num_teams.c#L25-L30

As the compiler notes, the code violates the following as it does not list omp_get_max_teams as being permitted:

Restrictions to the teams construct are as follows: ...

  • distribute, distribute simd, distribute parallel worksharing-loop, distribute parallel worksharing-loop SIMD, parallel regions, including any *parallel regions arising from combined constructs, omp_get_num_teams() regions, and omp_get_team_num() regions are the only OpenMP regions that may be strictly nested inside the teams region.

→ OpenMP 5.1, "2.7 teams Construct", [103:30–33], https://www.openmp.org/spec-html/5.1/openmpse15.html#x62-620002.7

@jrreap @andrewkallai @spophale @fel-cab

fel-cab commented 8 months ago

@tob2 I'm confused with the wording of the specification then, since it refers to the routine omp_get_num_teams() as a region. And from your comment, I should understand it as the only routines that can be called from inside the teams construct. Anyhow, if I understand correctly your comment, we could solve this problem by changing omp_get_max_teams() for omp_get_num_teams(), right?

tob2 commented 8 months ago

Regarding "refers to the routine omp_get_num_teams() as a region.", the spec (TR12 at least) has:

"region — All code encountered during a specific instance of the execution of a given construct, structured block sequence or OpenMP library routine."

And, yes, using omp_get_num_teams() instead of omp_get_max_teams() would work. But you could also do, e.g.

 #pragma omp teams  
 { 
    #omp parallel if(0)
    {
    if (omp_get_team_num() == 0 ) { 
        num_teams = omp_get_max_teams(); 
    } 
    }
}

As this seems to fulfill:

strictly nested region — A region nested inside another region with no other explicit region nested between them.

However, the question is really what you want to test for ... — I think doing so for the first teams region and keeping the second teams region as it would be an option.

BTW: I think the num_teams <= OMPVV_NUM_TEAMS_HOST should be num_teams > OMPVV_NUM_TEAMS_HOST instead (in both OMPVV_ERROR_IF and OMPVV_TEST_AND_SET).

andrewkallai commented 8 months ago

The omp_get_max_teams() can be used by num_teams = omp_get_max_teams(); itself or by using the nested parallel construct as suggested by @tob2 Felipe suggests to add the teams region instead as it originally was to query omp_get_num_teams() and to error if num_teams > value of the ICV. In addition, the error checking for the num_teams clause should be changed according to the suggestions by @tob2