[PATCH] D121713: [OpenMP] Initial parsing/sema for the 'omp teams loop' construct

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

Also, would be good to add test(s) to `test/Analysis/cfg-openmp.cpp`



================
Comment at: clang/lib/Basic/OpenMPKinds.cpp:706
+    CaptureRegions.push_back(OMPD_teams);
+    CaptureRegions.push_back(OMPD_unknown);
+    break;
----------------
Why do you need `OMPD_unknown` region here?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:4279-4285
+    // TODO: 'loop' may require additional parameters depending on the binding.
+    // Treat similar to OMPD_simd/OMPD_for for now.
+    Sema::CapturedParamNameType Params[] = {
+        std::make_pair(StringRef(), QualType()) // __context with shared vars
+    };
+    ActOnCapturedRegionStart(DSAStack->getConstructLoc(), CurScope, CR_OpenMP,
+                             Params, /*OpenMPCaptureLevel=*/1);
----------------
Copy/paste? The binding here is known aready, no?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:10218-10238
+  // OpenMP 5.0 [2.9.5, loop construct]
+  // A list item may not appear in a lastprivate clause unless it is the
+  // loop iteration variable of a loop that is associated with the construct.
+  for (OMPClause *C : Clauses) {
+    if (auto *LPC = dyn_cast<OMPLastprivateClause>(C)) {
+      for (Expr *RefExpr : LPC->varlists()) {
+        SourceLocation ELoc;
----------------
Can we move this check to ActOnOpenMPLastprivate? To avoid copy/paste and double iteration on list items.


================
Comment at: clang/test/OpenMP/teams_generic_loop_ast_print.cpp:9-10
+
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 \
+// RUN:   -ast-dump  %s | FileCheck %s --check-prefix=DUMP
+
----------------
Why need a dump here?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121713/new/

https://reviews.llvm.org/D121713