[Openmp-commits] [PATCH] D82470: [OpenMP][IRBuilder] Support nested parallel regions
Fady Ghanim via Phabricator via Openmp-commits
openmp-commits at lists.llvm.org
Wed Jun 24 13:34:37 PDT 2020
fghanim added a comment.
Thanks for the Patch. I have few questions to help me understand what's going on.
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:282
+ IRBuilder<> AllocaBuilder;
/// Map to remember source location strings
What's the benefit of this over just maintaining an alloca insertion point?
With this we will have 3 `IRBuilder`s to maintain and keep track of: the clang (or flang) `IRBuilder`, the OMP `IRBuilder` and the `allocaBuilder`
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:294
+ BasicBlock *EntryBB, *ExitBB;
I see two benefits of passing entry and exit blocks, as opposed to what we used to do:
1. less memory, but in return we collect blocks twice (i.e. O(N) mem & O(N+E) work vs O(1) mem and 2 * O(N+E) work ). Do you expect that the vector is likely to become large enough where it is a problem? if not, what's the benefit of the change?
2. If some blocks are added later, then this becomes a correctness issue. Which is unlikely since it happens after the body codegen is complete. However, if I am mistaken, shouldn't we also delay searching for inputs/outputs?
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:299
+ void collectBlocks(SmallPtrSetImpl<BasicBlock *> &BlockSet,
+ SmallVectorImpl<BasicBlock *> &BlockVector);
What is the benefit of passing `blockSet` when it is exclusively used inside of `collectBlocks`?
I don't think I saw a usage of it in calling functions. am I missing something?
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the Openmp-commits