[flang][OpenMP] Parse bind clause for loop direcitve.

[ergawy]

Adds parsing for the bind clause. The clause was already part of the loop direcitve's definition but parsing was still missing.

[llvmbot]

@llvm/pr-subscribers-flang-semantics
@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-parser

Author: Kareem Ergawy (ergawy)

Changes

Adds parsing for the bind clause. The clause was already part of the loop direcitve's definition but parsing was still missing.

---

Full diff: https://github.com/llvm/llvm-project/pull/113662.diff

5 Files Affected:

• (modified) flang/include/flang/Parser/dump-parse-tree.h (+2)
• (modified) flang/include/flang/Parser/parse-tree.h (+7)
• (modified) flang/lib/Parser/openmp-parsers.cpp (+8)
• (modified) flang/test/Parser/OpenMP/target-loop-unparse.f90 (+14-2)
• (modified) llvm/include/llvm/Frontend/OpenMP/OMP.td (+1)

diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 76d2f164fc4bf0..358e9b58ad096c 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -547,6 +547,8 @@ class ParseTreeDumper {
   NODE_ENUM(OmpOrderClause, Type)
   NODE(parser, OmpOrderModifier)
   NODE_ENUM(OmpOrderModifier, Kind)
+  NODE(parser, OmpBindClause)
+  NODE_ENUM(OmpBindClause, Type)
   NODE(parser, OmpProcBindClause)
   NODE_ENUM(OmpProcBindClause, Type)
   NODE_ENUM(OmpReductionClause, ReductionModifier)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index c1884f6e88d1ec..cc174e2eae0ad6 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3688,6 +3688,13 @@ struct OmpScheduleClause {
       t;
 };
 
+// OMP 5.2 11.7.1 bind-clause ->
+//                  BIND( PARALLEL | TEAMS | THREAD )
+struct OmpBindClause {
+  ENUM_CLASS(Type, Parallel, Teams, Thread)
+  WRAPPER_CLASS_BOILERPLATE(OmpBindClause, Type);
+};
+
 // OpenMP Clauses
 struct OmpClause {
   UNION_CLASS_BOILERPLATE(OmpClause);
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 59a8757e58e8cc..702a28cd380df0 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -417,6 +417,12 @@ TYPE_PARSER(construct<OmpLastprivateClause>(
         pure(OmpLastprivateClause::LastprivateModifier::Conditional) / ":"),
     Parser<OmpObjectList>{}))
 
+// OMP 5.2 11.7.1 BIND ( PARALLEL | TEAMS | THREAD )
+TYPE_PARSER(construct<OmpBindClause>(
+    "PARALLEL" >> pure(OmpBindClause::Type::Parallel) ||
+    "TEAMS" >> pure(OmpBindClause::Type::Teams) ||
+    "THREAD" >> pure(OmpBindClause::Type::Thread)))
+
 TYPE_PARSER(
     "ACQUIRE" >> construct<OmpClause>(construct<OmpClause::Acquire>()) ||
     "ACQ_REL" >> construct<OmpClause>(construct<OmpClause::AcqRel>()) ||
@@ -431,6 +437,8 @@ TYPE_PARSER(
     "ATOMIC_DEFAULT_MEM_ORDER" >>
         construct<OmpClause>(construct<OmpClause::AtomicDefaultMemOrder>(
             parenthesized(Parser<OmpAtomicDefaultMemOrderClause>{}))) ||
+    "BIND" >> construct<OmpClause>(construct<OmpClause::Bind>(
+                       parenthesized(Parser<OmpBindClause>{}))) ||
     "COLLAPSE" >> construct<OmpClause>(construct<OmpClause::Collapse>(
                       parenthesized(scalarIntConstantExpr))) ||
     "COPYIN" >> construct<OmpClause>(construct<OmpClause::Copyin>(
diff --git a/flang/test/Parser/OpenMP/target-loop-unparse.f90 b/flang/test/Parser/OpenMP/target-loop-unparse.f90
index 3ee2fcef075a37..b2047070496527 100644
--- a/flang/test/Parser/OpenMP/target-loop-unparse.f90
+++ b/flang/test/Parser/OpenMP/target-loop-unparse.f90
@@ -1,6 +1,8 @@
+! RUN: %flang_fc1 -fdebug-unparse -fopenmp -fopenmp-version=50 %s | \
+! RUN:   FileCheck --ignore-case %s
 
-! RUN: %flang_fc1 -fdebug-unparse -fopenmp %s | FileCheck --ignore-case %s
-! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp %s | FileCheck --check-prefix="PARSE-TREE" %s
+! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=50 %s | \
+! RUN:   FileCheck --check-prefix="PARSE-TREE" %s
 
 ! Check for parsing of loop directive
 
@@ -14,6 +16,16 @@ subroutine test_loop
    j = j + 1
   end do
   !$omp end loop
+
+  !PARSE-TREE: OmpBeginLoopDirective
+  !PARSE-TREE-NEXT: OmpLoopDirective -> llvm::omp::Directive = loop
+  !PARSE-TREE-NEXT: OmpClauseList -> OmpClause -> Bind -> OmpBindClause -> Type = Thread
+  !CHECK: !$omp loop
+  !$omp loop bind(thread)
+  do i=1,10
+   j = j + 1
+  end do
+  !$omp end loop
 end subroutine
 
 subroutine test_target_loop
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index 1834ad4d037f3d..ccf19ce2584ef9 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -73,6 +73,7 @@ def OMPC_AtomicDefaultMemOrder : Clause<"atomic_default_mem_order"> {
 }
 def OMPC_Bind : Clause<"bind"> {
   let clangClass = "OMPBindClause";
+  let flangClass = "OmpBindClause";
 }
 def OMP_CANCELLATION_CONSTRUCT_Parallel : ClauseVal<"parallel", 1, 1> {}
 def OMP_CANCELLATION_CONSTRUCT_Loop : ClauseVal<"loop", 2, 1> {}

[ergawy]

Not very familiar with parsing, so hopefully what I did makes sense.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[kparzysz]

The changes LGTM. Are you planning to add the lowering updates in a separate patch (i.e. emitting a TODO error, or the actual support)?

[mjklemm]

Please leave a TODO message for code-gen. Also, we will need to add semantic checks to check for the right nesting of bind, where the compiler can check it.

[ergawy]

@kparzysz @mjklemm there is an error emitted by lowering already: https://github.com/llvm/llvm-project/blob/main/flang/lib/Lower/OpenMP/OpenMP.cpp#L2461.

Also, we will need to add semantic checks to check for the right nesting of bind, where the compiler can check it.

@mjklemm I need some help interpreting the spec here. Here are the restrictions on bind according to 5.2:

Restrictions to the bind clause are as follows:
• If teams is specified as binding then the corresponding loop region must be strictly nested inside a teams region.
• If teams is specified as binding and the corresponding loop region executes on a non-host device then the behavior of a 
reduction clause that appears on the corresponding loop construct is unspecified if the construct is not nested inside a teams 
construct.
• If parallel is specified as binding, the behavior is unspecified if the corresponding loop region is closely nested inside a simd 
region.

For the first restriction, the nesting is defined on regions, so I think we cannot check that at compile time. The second restriction also cannot be checked statically since the corresponding teams construct might not be strictly nested inside a target region. The third restriction is about unspecified behavior.

Also, in semantics description section, the nesting relation is defined between regions not constructs.

But I am sure I missed something here. Any clarifications of what semantic checks need to be performed would be great.

[kparzysz]

Accidentally deleted RUN line...

[ergawy]

The RUN line is still here. I just added -fopenmp-verion=50 to add the bind related tests. Hopefully, this is fine rather than creating a new file just for this.

[kparzysz]

For the first restriction, the nesting is defined on regions, so I think we cannot check that at compile time.

We should still do that for the cases that we can positively diagnose. There is quite a bit of that happening in check-omp-structure.cpp.

[ergawy]

Added semantic checks. Let me know if you have suggestions for other checks as well.

[ergawy]

Ping 🔔! Can you please have another look at this?

[mjklemm]

If teams is specified as binding then the corresponding loop region must be strictly nested inside a teams region.

Yes, that's a runtime requirement and not a compile-time requirement. Thus, it will be hard to check.

If teams is specified as binding and the corresponding loop region executes on a non-host device then the behavior of a
reduction clause that appears on the corresponding loop construct is unspecified if the construct is not nested inside a teams
construct.

This we should be able to check at compile time.

If parallel is specified as binding, the behavior is unspecified if the corresponding loop region is closely nested inside a simd
region.

Runtime condition.

The good thing is that OpenMP makes this unspecified, so we can have any failure mode that we desire, incl. just computing something useless. However, somehow producing a sensible error will increase the user experience with all this.

[mjklemm]

LGTM