[PATCH] D99757: [flang][OpenMP] Add semantic check for occurrence of constructs nested inside a SIMD region

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan accepted this revision.
kiranchandramohan added a comment.
This revision is now accepted and ready to land.

LGTM. I have a Nit comment about a counter. Leaving the decision to you on whether to consider it or not.



================
Comment at: flang/lib/Semantics/check-directive-structure.h:224
 
+  // Check if a gven construct set is present anywhere in the context hierarchy
+  bool FindDirInWholeContext(
----------------
Nit: gven -> given


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:192
+  if (dirContext_.size() >= 1) {
+    if (FindDirInWholeContext(llvm::omp::simdSet)) {
+      CheckSIMDNest(x);
----------------
Nit: Would it be better to keep a counter which counts the constructs in the simdSet? The counter can be incremented on Entering a simdSet construct and decremented on leaving. If counter is greater than 1 then FindDirInWholeContext is true. This way we avoid going through all the  entries in dirContext_ everytime we see a construct.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:313
+  // Check if the parent context has the SIMD clause
+  // Please not that we use GetContext() instead of GetContextParent()
+  // because PushContextAndClauseSets() has not been called on the
----------------
Nit: not -> note


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

https://reviews.llvm.org/D99757