[PATCH] D72962: [MLIR, OpenMP] Translation of OpenMP barrier construct to LLVM IR

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: mlir/lib/Target/LLVMIR/ConvertToLLVMIR.cpp:25
+    "enable-openmp", llvm::cl::desc("Enable translation of OpenMP dialect Ops"),
+    llvm::cl::init(false)};
+
----------------
mehdi_amini wrote:
> jdoerfert wrote:
> > You could just check if `OMPBuilder` is initialized in order to keep this cmd line option private to a single file. Arguably, you could also create a method `getOMPBuilder()` which create one if none is present as soon as an OpenMP op is discovered. Maybe I'm missing some intended interaction of OpenMP ops and this flag set to false?
> Lazily creating the `OMPBuilder` on demand seems like ideal to me? It there a downside?
> 
> What about always creating it eagerly: is it that heavy/costly?
> Lazily creating the OMPBuilder on demand seems like ideal to me? It there a downside?

I can't think of one.

> What about always creating it eagerly: is it that heavy/costly?

No it's not. It create a couple of types, incl. function and struct types, but that's it.


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

https://reviews.llvm.org/D72962