[PATCH] D106165: [flang][OpenMP] Add semantic check for target nesting

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added inline comments.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:516
+  if (eligibleTARGET) {
+    context_.Say(parser::FindSourceLocation(c),
+        "%s directive cannot be nested inside TARGET region"_err_en_US,
----------------
clementval wrote:
> The standard says it is unspecified but not really restricted. 
Same concern as @clementval. Should this be a warning or an error?


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:479
 
+void OmpStructureChecker::CheckTARGETNest(const parser::OpenMPConstruct &c) {
+  // 2.12.5 Target Construct Restriction
----------------
Is the restriction for closely nested or does it apply to any kind of nesting? For e.g is the following allowed and will the test catch this?

-> target
-> -> parallel
-> -> -> target data


================
Comment at: flang/lib/Semantics/check-omp-structure.h:228
   void CheckSIMDNest(const parser::OpenMPConstruct &x);
+  void CheckTARGETNest(const parser::OpenMPConstruct &x);
   std::int64_t GetOrdCollapseLevel(const parser::OpenMPLoopConstruct &x);
----------------
Nit: I think it will be better to rename this as CheckTargetNest.


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

https://reviews.llvm.org/D106165