[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
     PostOutlineCBTy PostOutlineCB;
+    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



More information about the Openmp-commits mailing list