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

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 11 13:00:56 PST 2020


jdoerfert marked 2 inline comments as done.
jdoerfert added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113
+  if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder())
+    OMPBuilder->finalize();
 }
----------------
fghanim wrote:
> 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.
That doesn't work, otherwise I would not call finalize here. At least it doesn't work without extra changes. As it is right now, the OMPBuilder in CodeGenModule is deleted *after* the IR is generated and emitted, so any changes at destructor time are too late.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372





More information about the llvm-commits mailing list