[PATCH] D89671: [LLVM][OpenMP] Adding support for OpenMP sections construct in OpenMPIRBuilder
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 15 16:52:56 PDT 2021
jdoerfert added a comment.
I only have small nits, nothing major. @fghanim, is this OK with you, if so, please accept?
================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:106
BasicBlock &ContinuationBB)>;
+ // This is created primarily for sections construct as llvm::function_ref
+ // are not storable
----------------
Newline. Reference BodyGenCallBackTy for argument explanation.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:879
+ // have a terminator, which is already removed by EmitOMPRegionBody.
+ if (IP.getBlock()->end() == IP.getPoint()) {
+ // IP is currently at cancelation block.
----------------
Prefer early exits
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:891
+ IP = InsertPointTy(I->getParent(), I->getIterator());
+ }
+ return FiniCB(IP);
----------------
No backtracking, and similar schemes, if you can just remember the blocks you are looking for when you create them.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:897
+
+ // Each section in emitted as a switch case
+ // Each finalization callback is handled from clang.EmitOMPSectionDirective()
----------------
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:988
+ IP = InsertPointTy(I->getParent(), I->getIterator());
+ }
+ return FiniCB(IP);
----------------
same comments as above, early exists & remember the blocks rather than looking for them
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89671/new/
https://reviews.llvm.org/D89671
More information about the llvm-commits
mailing list