[PATCH] D15125: [OPENMP] 'omp distribute' directive basic support.
Alexey Bataev via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 2 06:02:47 PST 2015
ABataev added inline comments.
================
Comment at: include/clang/AST/OpenMPClause.h:667
@@ -666,3 +666,3 @@
OpenMPScheduleClauseKind Kind;
- /// \brief Start location of the schedule ind in source code.
+ /// \brief Start location of the schedule kind in source code.
SourceLocation KindLoc;
----------------
This must be in a separate patch
================
Comment at: include/clang/AST/OpenMPClause.h:708-709
@@ -707,4 +707,4 @@
public:
- /// \brief Build 'schedule' clause with schedule kind \a Kind and chunk size
- /// expression \a ChunkSize.
+ /// \brief Build 'dist_schedule' clause with schedule kind \a Kind and chunk
+ /// size expression \a ChunkSize.
///
----------------
I think this is wrong, this is not for dist_schedule, but for schedule clause
================
Comment at: include/clang/AST/OpenMPClause.h:779
@@ -778,1 +778,3 @@
+/// \brief This represents 'dist_schedule' clause in the '#pragma omp ...'
+/// directive.
----------------
This must be in a separate patch
================
Comment at: include/clang/AST/RecursiveASTVisitor.h:2732-2738
@@ -2727,1 +2731,9 @@
template <typename Derived>
+bool RecursiveASTVisitor<Derived>::VisitOMPDistScheduleClause(
+ OMPDistScheduleClause *C) {
+ TRY_TO(TraverseStmt(C->getChunkSize()));
+ TRY_TO(TraverseStmt(C->getHelperChunkSize()));
+ return true;
+}
+
+template <typename Derived>
----------------
separate patch
================
Comment at: include/clang/AST/StmtOpenMP.h:2279
@@ +2278,3 @@
+ /// \brief true if current directive has inner cancel directive.
+ bool HasCancel;
+
----------------
Distribute directive cannot have inner cancel
================
Comment at: include/clang/Basic/OpenMPKinds.h:89-95
@@ -88,2 +88,9 @@
+/// \brief OpenMP attributes for 'dist_schedule' clause.
+enum OpenMPDistScheduleClauseKind {
+#define OPENMP_DIST_SCHEDULE_KIND(Name) OMPC_DIST_SCHEDULE_##Name,
+#include "clang/Basic/OpenMPKinds.def"
+ OMPC_DIST_SCHEDULE_unknown
+};
+
OpenMPDirectiveKind getOpenMPDirectiveKind(llvm::StringRef Str);
----------------
Separate patch
================
Comment at: lib/AST/StmtPrinter.cpp:882
@@ +881,3 @@
+void OMPClausePrinter::VisitOMPDistScheduleClause(OMPDistScheduleClause *Node) {
+ OS << "schedule(" << getOpenMPSimpleClauseTypeName(
+ OMPC_dist_schedule, Node->getDistScheduleKind());
----------------
dist_schedule, not schedule
================
Comment at: lib/Basic/OpenMPKinds.cpp:475
@@ -452,1 +474,3 @@
+ DKind == OMPD_parallel_for_simd || DKind == OMPD_parallel_sections ||
+ DKind == OMPD_distribute; // TODO add next directives.
}
----------------
distribute is not a worksharing directive
================
Comment at: lib/Parse/ParseOpenMP.cpp:162
@@ -160,3 +161,3 @@
/// 'for simd' | 'parallel for simd' | 'target' | 'target data' |
-/// 'taskgroup' | 'teams' {clause}
+/// 'taskgroup' | 'teams' {clause} | 'distribute'
/// annot_pragma_openmp_end
----------------
'distribute' misses clauses, put your changes right after 'teams'
================
Comment at: lib/Parse/ParseOpenMP.cpp:239
@@ -238,1 +238,3 @@
+ case OMPD_distribute:
+ case OMPD_target_data: {
ConsumeToken();
----------------
Restore the order of target_data and taskloop directives, put distribute right after taskloop
================
Comment at: lib/Sema/SemaOpenMP.cpp:1530
@@ -1521,2 +1529,3 @@
// | Parent directive | Child directive | Closely (!), No-Closely(+), Both(*)|
+ // | | | Strictly (!!) |
// +------------------+-----------------+------------------------------------+
----------------
It is not required, closely without no-closely means strictly
================
Comment at: lib/Sema/SemaOpenMP.cpp:1989
@@ -1979,2 +1988,3 @@
// | teams | taskloop | + |
+ // | teams | distribute | !! |
// +------------------+-----------------+------------------------------------+
----------------
Add rules for distribute to all other directives
================
Comment at: lib/Sema/SemaOpenMP.cpp:2026-2027
@@ -2015,3 +2025,4 @@
ShouldBeInOrderedRegion,
- ShouldBeInTargetRegion
+ ShouldBeInTargetRegion,
+ ShouldBeInTeamsRegion
} Recommend = NoRecommend;
----------------
I don't think this change is required
================
Comment at: lib/Sema/SemaOpenMP.cpp:2062-2072
@@ -2050,2 +2061,13 @@
return false;
+ if (isOpenMPDistributeDirective(CurrentRegion)) {
+ // OpenMP 4.5 [2.17 Nesting of Regions]
+ // The region associated with the distribute construct must be strictly
+ // nested inside a teams region
+ if (ParentRegion != OMPD_teams) {
+ SemaRef.Diag(StartLoc,
+ diag::err_omp_distribute_strictly_nested_in_teams);
+ return true;
+ }
+ return false;
+ }
if (CurrentRegion == OMPD_cancellation_point ||
----------------
This must be handled in lines 2173-2181
================
Comment at: tools/libclang/CIndex.cpp:4489
@@ -4477,1 +4488,3 @@
+ case CXCursor_OMPDistributeDirective:
+ return cxstring::createRef("OMPForDirective");
case CXCursor_OverloadCandidate:
----------------
kkwli0 wrote:
> "OMPDistributeDirective"
"OMPDistributeDirective"
Repository:
rL LLVM
http://reviews.llvm.org/D15125
More information about the cfe-commits
mailing list