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

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.


================
Comment at: clang/include/clang/Basic/LangOptions.def:215
 LANGOPT(OpenMPCUDAMode    , 1, 0, "Generate code for OpenMP pragmas in SIMT/SPMD mode")
+LANGOPT(OpenMPNewCodegen  , 1, 0, "Use the experimental OpenMP-IR-Builder codegen path.")
 LANGOPT(OpenMPCUDAForceFullRuntime , 1, 0, "Force to use full runtime in all constructs when offloading to CUDA devices")
----------------
Shouldn't this rather be a CODEGENOPT (`CodeGenOptions.def`)?


================
Comment at: llvm/include/llvm/IR/OpenMPKinds.def:186
+///{
+
+#ifndef OMP_IDENT_FLAG
----------------
jdoerfert wrote:
> JonChesterfield wrote:
> > Sharing constants between the compiler and the runtime is an interesting subproblem. I think the cleanest solution is the one used by libc, where the compiler generates header files containing the constants which the runtime library includes.
> I'd prefer not to tackle this right now but get this done first and revisit the issue later. OK?
I don't think this is a good solution. It means that libomp cannot built built anymore without also building clang. Moreover, the values cannot be changed anyway since it would break the ABI.

I'd go the other route: The libomp defines what it's ABI is, the compiler generates code for it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785