[PATCH] D92732: [Flang][OpenMP 4.5] Add semantic check for OpenMP Do Loop Constructs

[Yashaswini Hegde via Phabricator via llvm-commits]

yhegde marked an inline comment as done.
yhegde added inline comments.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:147-148
+  std::int64_t collapseLevel{GetCollapseLevel(x)};
+  if (!collapseLevel)
+    collapseLevel++;
+
----------------
kiranchandramohan wrote:
> Why is collapse level used here but not in CheckLoopItrVariableIsInt?
I think with or without collapse clause, do iteration variables in all loops should be checked for integer type.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:213
+        for (auto iit{blk.begin()}; iit != blk.end(); ++iit) {
+          if (const auto *ifstmt{parser::Unwrap<parser::IfStmt>(*iit)}) {
+            const auto &actionstmt{std::get<1>(ifstmt->t)};
----------------
kiranchandramohan wrote:
> Is there an assumption here that if statement will be present when a cycle statement exists? Does the standard say that?
Cycles with out  ifstmts are also handled. 


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:337
+
+  threadPrivateSymbols.clear();
+  const auto &list{std::get<parser::OmpObjectList>(x.t)};
----------------
kiranchandramohan wrote:
> We do symbol related checks in resolve-directives right? Is there any particular reason this code should be here?
All do iteration variable related checks have been implemented in check-omp-structure.cpp


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:205
+
+void OmpStructureChecker::CheckCycleConstraints(
+    const parser::OpenMPLoopConstruct &x) {
----------------
kiranchandramohan wrote:
> kiranchandramohan wrote:
> > Are you handling cycle with the name of a loop?
> I probably did not make myself clear here. What i was suggesting was that it is not enough to look at where the cycle exists but also have to consider where the cycle branches to. Consider the testcase below.
> 
> ```
> program test
>   integer i, j, k
>   !$omp do  collapse(2)
>   foo: do i = 0, 10
>          do j = 0, 10
>            do k  = 0, 10
>              if (k .lt. 1) cycle foo
>              print *, i, j, k
>            end do
>          end do
>   end do foo
>   !$omp end do
> end program
> ```
Done. Thanks. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92732