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

Yashaswini Hegde via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 14 10:29:00 PST 2021


yhegde added inline comments.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:109
+            std::get<std::optional<parser::DoConstruct>>(x.t)}) {
+      if (doConstruct.value().IsDoWhile()) {
+        const auto &doStmt{std::get<parser::Statement<parser::NonLabelDoStmt>>(
----------------
kiranchandramohan wrote:
> If more than one loop is associated with the OpenMP do loop, should this check be performed on all the loops?
https://www.openmp.org/wp-content/uploads/openmp-4.5.pdf  , page num 74, line num 5-6  says  " If no collapse clause is present, the only loop that is associated with the loop construct is the one that immediately follows the directive." 
  
So our understanding was if no collapse is present, only the iterations of outermost loop are distributed among threads. 

gfrotran also throws error for only the outer loop. 


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:120
+
+void OmpStructureChecker::CheckLoopItrVariableIsInt(
+    const parser::OpenMPLoopConstruct &x) {
----------------
kiranchandramohan wrote:
> kiranchandramohan wrote:
> > CheckDoControl and CheckDoVariable in flang/lib/Semantics/check-do-forall.cpp does the same check but are probably OFF or set as warning by default. Can the code in check-do-forall be reused?
> Is the answer here NO?
I modified my code to suggested change. i.e I am doing similar kind of check like how it is done in check-do-forall.cpp

Will look at the rest of the review comments. Thank you. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92732



More information about the llvm-commits mailing list