[PATCH] D53448: [OpenMP][NVPTX] Use single loops when generating code for distribute parallel for

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 24 07:29:30 PDT 2018


ABataev added inline comments.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:904
+  ///
+  virtual bool isStaticChunked(OpenMPDistScheduleClauseKind ScheduleKind,
+                               bool Chunked) const;
----------------
I'd rename this into `isDistStaticChunked`


================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:4249
     CodeGenFunction &CGF, const OMPLoopDirective &S,
-    OpenMPScheduleClauseKind &ScheduleKind,
+    OpenMPScheduleTy &ScheduleKind,
     llvm::Value *&Chunk) const {
----------------
If the `ChunkOne` field is not required, you need to restore original code here


================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:2360
         OMPCancelStack.emitExit(*this, S.getDirectiveKind(), CodeGen);
+      } else if (RT.isStaticChunked(ScheduleKind.Schedule,
+                                    /* Chunked */ Chunk != nullptr) &&
----------------
This whole code is very similar to the unchunked case. Could you merge it?


================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:2362
+                                    /* Chunked */ Chunk != nullptr) &&
+                 ScheduleKind.HasChunkOne &&
+                 isOpenMPLoopBoundSharingDirective(S.getDirectiveKind())) {
----------------
It allows you to check only the implicit case, what about if the user explicitly specifies that `chunk` is `1`?


================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:3421
         RT.emitForStaticFinish(*this, S.getBeginLoc(), S.getDirectiveKind());
+      } else if (RT.isStaticChunked(ScheduleKind,
+                                    /* Chunked */ Chunk != nullptr) &&
----------------
Again, very similar to the unchunked code. Merge it.


================
Comment at: lib/Sema/SemaOpenMP.cpp:5207
+    CombDistCond =
+        SemaRef.BuildBinOp(CurScope, CondLoc, BO_LE, IV.get(), LastIteration.get());
+  }
----------------
Seems to me, you need to use `NumIterations` instead of `LastIteration`


Repository:
  rC Clang

https://reviews.llvm.org/D53448





More information about the cfe-commits mailing list