Page MenuHomePhabricator

[flang][OpenMP] Add semantic checks for strict nesting inside `teams` construct.
ClosedPublic

Authored by arnamoy10 on Apr 7 2021, 8:26 AM.

Details

Summary

This patch implements the following two checks:

  1. Only ‘distribute’ or ‘parallel’ regions are allowed to be strictly nested inside ‘teams’ region
  2. DISTRIBUTE region has to be strictly nested inside TEAMS region.

Modifies the existing test cases with the new error messages and adds a new test.

Diff Detail

Event Timeline

arnamoy10 created this revision.Apr 7 2021, 8:26 AM
arnamoy10 requested review of this revision.Apr 7 2021, 8:26 AM
Herald added a project: Restricted Project. · View Herald Transcript
flang/lib/Semantics/check-omp-structure.cpp
187

Nit: use braced initialization.

238

This way of iterating at the top-level blocks will miss cases where there is a construct (for e.g do loop) which has further blocks inside. See a test below,

!$omp teams
do i = 1, N
!$omp task
  do k = 1, N
   a = 3.14
enddo
!$omp end task
enddo
!$omp end teams
kiranchandramohan requested changes to this revision.Sun, Apr 11, 4:16 PM
This revision now requires changes to proceed.Sun, Apr 11, 4:16 PM
arnamoy10 updated this revision to Diff 337531.Wed, Apr 14, 1:00 PM

As per reviewers suggestion, following a bottom-up approach (instead of the previously implemented top down approach) to find the violations.

Modified the test case to add the suggested test case.

kiranchandramohan requested changes to this revision.Fri, Apr 23, 8:51 AM

Apologies for the delay. I have a question and a suggestion.

flang/lib/Semantics/check-omp-structure.cpp
238

This visit will cause an additional traversal of the subtree rooted at the teams node. Visits to these nodes will be happening anyway during the normal traversal with various Enter and Leave functions.

Would it be possible to do these checks in the following functions? You can add if they don't exist.

void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &c)
void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &c)

Let me know if there is an issue with this approach.

This revision now requires changes to proceed.Fri, Apr 23, 8:51 AM
arnamoy10 updated this revision to Diff 341235.Wed, Apr 28, 9:17 AM

Addressing reviewers' comments, avoiding additional traversals of the parse tree nodes.

flang/lib/Semantics/check-omp-structure.cpp
238

Makes sense. Thanks for the suggestion.

LGTM.

flang/test/Semantics/omp-nested-distribute.f90
105

Nit: empty line.

This revision is now accepted and ready to land.Wed, Apr 28, 4:17 PM
arnamoy10 closed this revision.Thu, Apr 29, 5:38 AM