[PATCH] D110502: [Flang][openmp] Add semantic checks for OpenMP critical construct name resolution

[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 two NIT comments. Please apply these if possible.

Once you submit, can you cherry-pick the changes to the fir-dev branch?



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:1055
+  if (!dirName && !ompClause.source.empty() &&
+      ompClause.source.NULTerminatedToString() != "hint(omp_sync_hint_none)") {
+    context_.Say(dir.source,
----------------
Nit: It would be better if you can get the value of the expression and check for it to be ZERO. You can also add a comment above indication what this check is.


```
| | | | OmpClauseList -> OmpClause -> Hint -> Constant -> Expr = '0_4'
| | | | | Designator -> DataRef -> Name = 'omp_sync_hint_none'
```


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:1058
+        parser::MessageFormattedText{
+            "Hint clause cannot exist on an unnamed CRITICAL directive"_err_en_US});
+  }
----------------
Nit: Consider changing to something like the following.

Hint clause with a value other than omp_sync_hint_none cannot be specified for an unnamed CRITICAL directive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110502