[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Patch looks good with the above nits.

I'm not totally sure about the callback vs running a separate IR pass after the finalize() call, but when the callback is this simple it looks fine. I like that this preserves the current semantics.



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:275
+  /// Get a handle for a new region that will be outlined later.
+  OutlineInfo &getNewOutlineInfo() {
+    OutlineInfos.push_back({});
----------------
Calling getNewOutlineInfo will invalidate the references previously returned. That's not a bug in this patch but looks like it'll be easy to get wrong in future.

Perhaps the backing store should be a linked list, such that push_back doesn't invalidate any existing references?


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:587
+      CallInst *CI = cast<CallInst>(OutlinedFn.user_back());
+      CI->getParent()->setName("omp_parallel");
+      Builder.SetInsertPoint(CI);
----------------
s/parallel/outlined?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372