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

[Fady Ghanim via Phabricator via cfe-commits]

fghanim added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113
+  if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder())
+    OMPBuilder->finalize();
 }
----------------
jdoerfert wrote:
> fghanim wrote:
> > Does this mean that `finalize()` is being called twice everytime, here and `~OpenMPIRBuilder()`?
> > if yes, is there a reason why you want that?
> It's called twice now, not that it matters. The basic rule is, call it at least once before you finalize the module. I'll remove the destructor for now to make it explicit only.
Right now it doesn't matter, I agree. However, if later `finalize()` is modified for any reason, I worry this may introduce problems unintentionally.

In any case, if you decide to keep only one of them, I prefer to keep the one in `~OpenMPIRBuilder()`. This way the `OMPBuilder` is self contained, so regardless of the frontend using it, we can be certain it's always called at the end. You can indicate explicitly in the description comment for `finalize()` that it is being called in the destructor, if you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372