[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Johannes Doerfert via Phabricator via llvm-commits]

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