This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Parsing + sema for "target enter data" and "target exit data" directives.
ClosedPublic

Authored by arpith-jacob on Jan 7 2016, 6:36 PM.

Details

Summary

This patch adds parsing and sema support for "#pragma omp target enter data" and "#pragma omp target exit data" directives.

Diff Detail

Event Timeline

arpith-jacob retitled this revision from to [OpenMP] Parsing + sema for "target enter data" and "target exit data" directives..
arpith-jacob updated this object.
arpith-jacob added a subscriber: cfe-commits.
arpith-jacob added inline comments.Jan 7 2016, 6:50 PM
lib/Parse/ParseOpenMP.cpp
42

I have written this code based on the comments in the 'target data' patch and also the approach to parsing 'cancellation'. This requires hard coded array index numbers below (see diff with cancellation and point tokens). If anyone can suggest better alternatives I will be happy to rework the code.

lib/Sema/SemaOpenMP.cpp
5942

This method is required to ensure that target enter/exit directives always have a map clause. It can be reused by other directives such as target data.

8528

I have to capture 'IsMapTypeImplicit' from the parser for more informative error messages. This helps me support the following case:

#pragma omp target enter data map(r) // expected-error {{map type must be specified for '#pragma omp target enter data'}}

and distinguish it from:

#pragma omp target enter data map(tofrom: r) // expected-error {{map type 'tofrom' is not allowed for '#pragma omp target enter data'}}

8531

This error occurs when the user specifies an invalid map type. After reporting the error I proceed to retain the clause so that it doesn't trigger the 'map clause not present' error.

ABataev edited edge metadata.Jan 11 2016, 9:40 PM

You need to add the code/tests for nesting of regions.

include/clang-c/Index.h
2283

Target exit and target enter constructs must be implemented in different patches

include/clang/AST/OpenMPClause.h
2745 ↗(On Diff #44302)

This also must be in a separate patch

lib/CodeGen/CGStmtOpenMP.cpp
2665

Just ignore this pragma for now, no need to crash a compiler

2670

Just ignore this pragma for now, no need to crash a compiler

arpith-jacob edited edge metadata.

Created a distinct patch for 'target enter data'.
Added nesting test cases where the unstructured statement is nested in various regions.

Created a distinct patch for 'target enter data'.
Added nesting test cases where the unstructured statement is nested in various regions.
Removed extraneous comments.

ABataev added inline comments.Jan 17 2016, 9:17 PM
include/clang/Sema/Sema.h
7767–7768

I don't think it must be exposed as a member of Sema. It is enough just to make it static in SemaOpenMP.cpp

lib/Parse/ParseOpenMP.cpp
175

'target exit data' is not supported in this patch

lib/Sema/SemaOpenMP.cpp
5933–5935

just if(*I != nullptr && (*I)->getClauseKind() == OMPC_map)

8529

Remove parens around 0

HasMapClause() is now static.
Removed comment about 'target exit data'.
Modified: just if(*I != nullptr && (*I)->getClauseKind() == OMPC_map)
Done: Remove parens around 0

arpith-jacob marked 4 inline comments as done.Jan 18 2016, 6:31 PM
ABataev accepted this revision.Jan 18 2016, 8:30 PM
ABataev edited edge metadata.
This revision is now accepted and ready to land.Jan 18 2016, 8:30 PM
sfantao closed this revision.Jan 19 2016, 11:20 AM
sfantao edited edge metadata.

Committed revision 258165.

Thanks,
Samuel