[flang][OpenMP] Fix one bug in parsing depend(sink: vec)

[peixin]

Summary

This patch fixes one bug of no symbol found in parsing depend(sink: vec)
by replacing "Name" with "OmpObject". The "vec" in "depend(sink: vec)"
is one list of experssion of "OmpObject [Operator ScalarIntConstant]"
such as "i, j - 1".

[kiranchandramohan]

flang/include/flang/Parser/parse-tree.h

3439

In a dependence vector, the names correspond to the iteration variables of loops. While OmpObject can represent variables, it can also represent other things like array expressions, common-blocks etc. Without additional semantics checks this might not be correct, isn't it?

[peixin]

@kiranchandramohan Thanks for pointing out the problem. I think you are right.
I guess the problem is that the name resolution has not been finished. I tried the following change and the command ninja check-flang works fine.

diff --git a/flang/lib/Semantics/rewrite-parse-tree.cpp b/flang/lib/Semantics/rewrite-parse-tree.cpp
index 071a668de7fd..ff0a38117a73 100644
--- a/flang/lib/Semantics/rewrite-parse-tree.cpp
+++ b/flang/lib/Semantics/rewrite-parse-tree.cpp
@@ -46,6 +46,7 @@ public:
 
   // Name resolution yet implemented:
   // TODO: Can some/all of these now be enabled?
+  bool Pre(parser::Name &) { return false; }
   bool Pre(parser::EquivalenceStmt &) { return false; }
   bool Pre(parser::Keyword &) { return false; }
   bool Pre(parser::EntryStmt &) { return false; }

In addition, the following code can pass the frontend compilation and the name "j" is dumped in parser tree. I am not sure if this check should be implemented in the name resolution or semantic check.

!$omp do ordered(1)
do i = 2, N
  !$omp ordered depend(sink: j - 1)
  B(i) = bar(A(i), B(i-1))
end do

| | | | | | | Name = 'j'
| | | | | | | OmpDependSinkVecLength
| | | | | | | | DefinedOperator -> IntrinsicOperator = Subtract
| | | | | | | | Scalar -> Integer -> Constant -> Expr = '1_4'
| | | | | | | | | LiteralConstant -> IntLiteralConstant = '1'

Is it ok to use this fix and leave "TODO" here for temporary fix? I need this for lowering depend(sink: vec). Or can I post the question of the name resolution on slack and let someone who knows the implementation of the name resolution fix it?

[peixin]

I think resolving the name in flang/lib/Semantics/resolve-directives.cpp is better since some semantic checks can be done at the same time. I will use the method in D94087. This PR may can be closed.

[kiranchandramohan]

@peixin can you close this review?

flang/include/flang/Parser/parse-tree.h

 struct OmpDependSinkVecLength {
   TUPLE_CLASS_BOILERPLATE(OmpDependSinkVecLength);
   std::tuple<DefinedOperator, ScalarIntConstantExpr> t;
 };
 
 // 2.13.9 depend-vec -> iterator [+/- depend-vec-length],...,iterator[...]
 struct OmpDependSinkVec {
   TUPLE_CLASS_BOILERPLATE(OmpDependSinkVec);
-  std::tuple<Name, std::optional<OmpDependSinkVecLength>> t;
+  std::tuple<OmpObject, std::optional<OmpDependSinkVecLength>> t;
 };
 
 // 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);
 };
 

flang/lib/Parser/openmp-parsers.cpp

 TYPE_PARSER(construct<OmpAllocateClause>(
     maybe(construct<OmpAllocateClause::Allocator>(scalarIntExpr) / ":"),
     Parser<OmpObjectList>{}))
 
 // 2.13.9 DEPEND (SOURCE | SINK : vec | (IN | OUT | INOUT) : list
 TYPE_PARSER(construct<OmpDependSinkVecLength>(
     Parser<DefinedOperator>{}, scalarIntConstantExpr))
 
-TYPE_PARSER(
-    construct<OmpDependSinkVec>(name, maybe(Parser<OmpDependSinkVecLength>{})))
+TYPE_PARSER(construct<OmpDependSinkVec>(
+    Parser<OmpObject>{}, maybe(Parser<OmpDependSinkVecLength>{})))
 
 TYPE_PARSER(
     construct<OmpDependenceType>("IN"_id >> pure(OmpDependenceType::Type::In) ||
         "INOUT" >> pure(OmpDependenceType::Type::Inout) ||
         "OUT" >> pure(OmpDependenceType::Type::Out)))
 
 TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US,
     construct<OmpDependClause>(construct<OmpDependClause::Sink>(

flang/lib/Parser/unparse.cpp

void Unparse(const OmpAllocateClause &x) {
     Put(":");
     Walk(std::get<OmpObjectList>(x.t));
   }
   void Unparse(const OmpDependSinkVecLength &x) {
     Walk(std::get<DefinedOperator>(x.t));
     Walk(std::get<ScalarIntConstantExpr>(x.t));
   }
   void Unparse(const OmpDependSinkVec &x) {
-    Walk(std::get<Name>(x.t));
+    Walk(std::get<OmpObject>(x.t));
     Walk(std::get<std::optional<OmpDependSinkVecLength>>(x.t));
   }
   void Unparse(const OmpDependClause::InOut &x) {
     Put("(");
     Walk(std::get<OmpDependenceType>(x.t));
     Put(":");
     Walk(std::get<std::list<Designator>>(x.t), ",");
     Put(")");

flang/test/Semantics/omp-clause-validity01.f90

! Standalone Directives (basic)
   !$omp barrier
   !$omp taskwait
   !$omp taskwait depend(source)
   ! !$omp taskwait depend(sink:i-1)
   ! !$omp target enter data map(to:arrayA) map(alloc:arrayB)
   ! !$omp target update from(arrayA) to(arrayB)
   ! !$omp target exit data map(from:arrayA) map(delete:arrayB)
   !$omp ordered depend(source)
-  ! !$omp ordered depend(sink:i-1)
+  !$omp ordered depend(sink:i-1)
   !$omp flush (c)
   !$omp flush acq_rel
   !$omp flush release
   !$omp flush acquire
   !ERROR: If memory-order-clause is RELEASE, ACQUIRE, or ACQ_REL, list items must not be specified on the FLUSH directive
   !$omp flush release (c)
   !ERROR: SEQ_CST clause is not allowed on the FLUSH directive
   !$omp flush seq_cst