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

[River Riddle via Phabricator via llvm-commits]

rriddle requested changes to this revision.
rriddle added inline comments.
This revision now requires changes to proceed.


================
Comment at: mlir/include/mlir/Target/OpenMPLLVMIR.h:32
+/// any (obtained from the MLIR module), and return `nullptr`.
+std::unique_ptr<llvm::Module> translateModuleToOpenMPLLVMIR(Operation *m);
+
----------------
I think this revision highlights a problem with the way the LLVM converter(or at least the way it is being used) is structured, mainly that it isn't extensible. I would imagine that lowering support for OpenMP would be an (optional) extension to the LLVM converter, not an entirely different one.


================
Comment at: mlir/lib/Target/LLVMIR/ConvertToOpenMPLLVMIR.cpp:61
+private:
+  llvm::OpenMPIRBuilder *OMPBuilder;
+};
----------------
I would imagine this would be a unique_ptr instead of a raw pointer.


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

https://reviews.llvm.org/D72962