[PATCH] D92476: [OpenMPIRBuilder] introduce createStaticWorkshareLoop
Fady Ghanim via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Dec 6 11:16:59 PST 2020
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
More information about the llvm-commits
mailing list