fghanim added inline comments.
================ Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113 + if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder()) + OMPBuilder->finalize(); } ---------------- jdoerfert wrote: > 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. Got it. Then a minor suggestion is to just put a note in `finalize()` description about whatever you decide to do (i.e. keep both, remove the one in destructor, or something else entirely). This way, whoever is using/modifying the code is aware of it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74372/new/ https://reviews.llvm.org/D74372 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
