[PATCH] D97393: [OpenMP IRBuilder, MLIR] Add support for OpenMP do schedule dynamic

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1203-1204
+  // Set up the source location value for OpenMP runtime.
+  if (!updateToLocation(Loc))
+    return nullptr;
+
----------------
The InsertionPointTy of `Loc` is not used with methods that apply to a canonical loop. Consider passing only the DebugLoc. The same seem to apply to `createStaticWorkshareLoop`.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1215-1234
+  // Allocate space for computed loop bounds as expected by the "init" function.
+  Builder.restoreIP(AllocaIP);
+  Type *I32Type = Type::getInt32Ty(M.getContext());
+  Value *PLastIter = Builder.CreateAlloca(I32Type, nullptr, "p.lastiter");
+  Value *PLowerBound = Builder.CreateAlloca(IVTy, nullptr, "p.lowerbound");
+  Value *PUpperBound = Builder.CreateAlloca(IVTy, nullptr, "p.upperbound");
+  Value *PStride = Builder.CreateAlloca(IVTy, nullptr, "p.stride");
----------------
Since these are shared with `createStaticWorkshareLoop`, did you consider extractin it into a common function?


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1250-1251
+  Builder.CreateCall(DynamicInit,
+                     {SrcLoc, ThreadNum, SchedulingType, Zero /* LastIter */,
+                      One /* LowerBound */, UpperBound, One});
+  Value *LowerBound = Builder.CreateLoad(PLowerBound);
----------------
Usually the name of the parameter comes before the argument


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97393