[flang-commits] [PATCH] D94087: [flang][openmp]At most one threads, simd and depend clause can appear on OpenMP ORDERED construct.
sameeran joshi via Phabricator via flang-commits
flang-commits at lists.llvm.org
Thu Jan 21 06:22:26 PST 2021
sameeranjoshi added inline comments.
================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:838-875
+ const parser::OmpDependClause::Source *foundSource{nullptr};
+ const parser::OmpDependClause::Source *foundSourceFromEnd{nullptr};
+ const parser::OmpDependClause::Sink *foundSink{nullptr};
+ for (const auto *vecElem : ompDependVector) {
+ foundSource = std::get_if<parser::OmpDependClause::Source>(&vecElem->u);
+ if (foundSource != nullptr) {
+ break;
----------------
kiranchandramohan wrote:
> sameeranjoshi wrote:
> > kiranchandramohan wrote:
> > > ```
> > > struct OmpDependenceType {
> > > ENUM_CLASS(Type, In, Out, Inout, Source, Sink)
> > > WRAPPER_CLASS_BOILERPLATE(OmpDependenceType, Type);
> > > };
> > >
> > > struct OmpDependClause {
> > > ....
> > > struct InOut {
> > > TUPLE_CLASS_BOILERPLATE(InOut);
> > > std::tuple<OmpDependenceType, std::list<Designator>> t;
> > > };
> > > std::variant<Source, Sink, InOut> u;
> > > };
> > > ```
> > > Can you use OmpDependenceType in OmpDependClause to simplify this logic? I think you can just count the source, sink and print out the error messages.
> > Could anyone please elaborate more on above?
> > I couldn't understand .
> > `parse-tree.h` mentions
> > ```
> > // 2.13.9 depend-type -> IN | OUT | INOUT | SOURCE | SINK
> > struct OmpDependenceType {
> > ENUM_CLASS(Type, In, Out, Inout, Source, Sink)
> > WRAPPER_CLASS_BOILERPLATE(OmpDependenceType, Type);
> > };
> > ```
> > Where as parsers omit's the `source` and sink in `OmpDependenceType` in `openmp-parsers.cpp`
> >
> > ```
> > TYPE_PARSER(
> > construct<OmpDependenceType>("IN"_id >> pure(OmpDependenceType::Type::In) ||
> > "INOUT" >> pure(OmpDependenceType::Type::Inout) ||
> > "OUT" >> pure(OmpDependenceType::Type::Out)))
> > ```
> >
> > I was confused if the parse tree has some issues.
> > Hence, if you somecan elaborate a bit on what exactly is expected that would be really helpful.
> > Or if there's a more better way of simplifying this code.
> I was assuming that the dependence_type is set always. If it is not then can it be set in the parser and used? Is there any reason why it is not set?
I have updated to simplify the logic. I am trying to not change parser in first run as there might be some reason why that might have been designed by original authors.
I have no strong opinions to change the parser if this logic still seems harder.
Thanks for `parser::Unwrap` inputs that helped to simplify things @kiranchandramohan.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94087/new/
https://reviews.llvm.org/D94087
More information about the flang-commits
mailing list