[PATCH] D93051: [Flang][openmp] Add semantic checks for OpenMP critical construct.
Kiran Chandramohan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 19 15:52:23 PST 2021
kiranchandramohan requested changes to this revision.
kiranchandramohan added inline comments.
This revision now requires changes to proceed.
================
Comment at: flang/lib/Semantics/check-directive-structure.h:278-282
+ const auto &directiveName{std::get<parser::Verbatim>(beginDir.t)};
+ const auto &upperCaseDirName{
+ parser::ToUpperCaseLetters(directiveName.source.ToString())};
+ const auto &beginDirName{std::get<std::optional<parser::Name>>(beginDir.t)};
+ const auto &endDirName{std::get<std::optional<parser::Name>>(endDir.t)};
----------------
Can you use parser::Unwrap<parser::Name> to simplify the code?
================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:243
+// TODO: Change it to a more better version without raw string comparision
+// once the module files are present.
+static bool IsHintExprAs(
----------------
The module file has omp_sync_hint_none.
https://github.com/llvm/llvm-project/blob/3729ee893948be7e3ba138b2a04c4cdfa6257cdf/flang/module/omp_lib.h#L46
================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:260
+
+void OmpStructureChecker::Leave(const parser::OpenMPCriticalConstruct &x) {
+ // [OMP-5.0][2.17.1] Unless the effect is as if hint(omp_sync_hint_none)
----------------
Can this be part of Enter? Or is there a convention that checking clauses will only happen during leave?
================
Comment at: flang/lib/Semantics/resolve-directives.cpp:334
Symbol::Flag::OmpFirstPrivate, Symbol::Flag::OmpLastPrivate,
- Symbol::Flag::OmpReduction};
+ Symbol::Flag::OmpReduction, Symbol::Flag::OmpCriticalLock};
----------------
Does lock need a new symbol? What is the issue you saw?
================
Comment at: flang/lib/Semantics/resolve-directives.cpp:1003
+ std::get<std::optional<parser::Name>>(beginCriticalDir.t)}) {
+ ResolveOmpName(*criticalName, Symbol::Flag::OmpCriticalLock);
+ }
----------------
Can the name here be a variable declared elsewhere? If not do we have to resolve the name?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93051/new/
https://reviews.llvm.org/D93051
More information about the llvm-commits
mailing list