jdoerfert marked an inline comment as done. jdoerfert added a comment. In D69785#1734205 <https://reviews.llvm.org/D69785#1734205>, @ABataev wrote:
> Also, I think it would better to split LLVM part and clang part into separate > patches. What do you mean exactly and why? ================ Comment at: llvm/include/llvm/IR/OpenMPIRBuilder.h:29 + /// not have an effect on \p M (see initialize). + OpenMPIRBuilder(Module &M) : M(M), Builder(M.getContext()) {} + ---------------- ABataev wrote: > jdoerfert wrote: > > ABataev wrote: > > > Do we need a new `Builder` here or we can reuse the one from clang > > > CodeGenFunction? > > If you have a "simple" way to do it, we can think about it but I am still > > unsure if that is actually useful. The clang (=frontend) builder is used > > for callbacks so user code is build with it either way. We could set up > > ours here differently if we wish to and I'm a little afraid we would > > generate some unwanted interactions. > > > > That being said, I tried to reuse the one in clang but struggled *a long > > time* to make it work. The problem is that it is a templated class with > > Clang specific template parameters. We would need to make this a template > > class as well (I think) and that comes with a long tail of problems. > > > You can make this class a template and instantiate it with the type of the > CodeGenFunction IRBuilder and pass it by reference in the constructor. But > only if it really worth it. That doesn't work as easily because the implementation is not part of this header, so we need an extern template and we'll open up a link nightmare that I would like to avoid. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69785/new/ https://reviews.llvm.org/D69785 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits