This is an archive of the discontinued LLVM Phabricator instance.

[flang][OpenMP] Add semantic check for teams nesting

Authored by peixin on Jul 19 2021, 7:52 PM.



This patch implements the following check for TEAMS construct:

OpenMP Version 5.0 Teams construct restriction: A teams region can
only be strictly nested within the implicit parallel region or a target
region. If a teams construct is nested within a target construct, that
target construct must contain no statements, declarations or directives
outside of the teams construct.

Also add one test case for the check.

Diff Detail

Event Timeline

peixin created this revision.Jul 19 2021, 7:52 PM
peixin requested review of this revision.Jul 19 2021, 7:52 PM
peixin updated this revision to Diff 360004.Jul 19 2021, 9:00 PM

Fix errors info in related test cases.

peixin updated this revision to Diff 360006.Jul 19 2021, 9:01 PM
peixin edited the summary of this revision. (Show Details)Jul 20 2021, 12:18 AM
clementval requested changes to this revision.Jul 20 2021, 4:20 AM
clementval added inline comments.

This is only for OpenMP so it should be moved to check-omp-structure.h. Same for the two functions below. The check-directive-structure.h is meant to be shared bewteen OpenMP and OpenACC.


Which statement is outside?


parallel region instead of parallelregion


Missing a space here as well.


Missing space


Missing space


Missing space

This revision now requires changes to proceed.Jul 20 2021, 4:20 AM
peixin updated this revision to Diff 360138.Jul 20 2021, 8:04 AM

@clementval Thanks for the review.

Moved two sets of functions and counters to check-omp-structure.h and merged them. Also fix the errors info in the test cases.

peixin added inline comments.Jul 20 2021, 11:24 PM

This target directive has two teams construct nested inside and both of teams construct contain statements. Change the error info by replacing statements by statements or directives, which should be more reasonable.

Looks OK. Have two questions.


Is the parent teams check required?


Nit: Can we add a LastType at the end of the enum and use it as the size of the directiveNest_ array?

peixin updated this revision to Diff 362659.Jul 29 2021, 12:27 AM
peixin added a reviewer: Chuanfeng.
peixin set the repository for this revision to rG LLVM Github Monorepo.

@kiranchandramohan Thanks for the review. Fixed the patch according to your comments.


The restriction for two strictly nested teams has already been checked with the error info of distribute and parallel region only allowed strictly nested inside teams region. Rethink about it and reporting one more error here is more reasonable. Deleted the check of parent teams and added the error info in test case omp-nested-teams.f90.


Good idea. Added it.

peixin updated this revision to Diff 362666.Jul 29 2021, 1:24 AM

Change directiveNest_[2] to directiveNest_[3] since one more element LastType was added.


@clementval do you have further comments?


Nit: Sorry, I meant can you move this below the enum and use LastType (with SIMDNest =1 ) or LastType +1 as the size of the directiveNest_array?

enum directiveNestType { SIMDNest, TargetBlockOnlyTeams, LastType };
int directiveNest_[LastType+1] = {0};
peixin updated this revision to Diff 362715.Jul 29 2021, 4:22 AM
peixin set the repository for this revision to rG LLVM Github Monorepo.

@kiranchandramohan Thanks for the suggestions, which prevents from using magic number. Replaced directiveNest_[3] with directiveNest_[LastType+1].

clementval accepted this revision.Aug 12 2021, 7:12 PM

LGTM as well. Sorry for the delay in reviewing the change to the patch. You can go ahead and land it.

This revision is now accepted and ready to land.Aug 12 2021, 7:12 PM
This revision was automatically updated to reflect the committed changes.