[PATCH] D92476: [OpenMPIRBuilder] introduce createStaticWorkshareLoop

[Fady Ghanim via Phabricator via llvm-commits]

fghanim added a comment.

Thanks for the patch. Just a couple of nits



================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1006
+// CanonicalLoopInfo.
+static FunctionCallee getKmpcForStaticInitForType(Type *type, Module &M,
+                                                  OpenMPIRBuilder &OMPBuilder) {
----------------
I strongly suspect that @jdoerfert was referring to argument `Type* type`


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1032
+  // Set up the source location value for OpenMP runtime.
+  updateToLocation(Loc);
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
----------------
Nit: To keep a consistent behavior throughout the builder, please change to:
```
if (!updateToLocation(Loc))
    return Loc.IP;
```


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1047-1049
+  Value *PLowerBound = Builder.CreateAlloca(IVTy);
+  Value *PUpperBound = Builder.CreateAlloca(IVTy);
+  Value *PStride = Builder.CreateAlloca(IVTy);
----------------
NIT: please name all alloca's above - easier to track in IR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92476