[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

Tue Jun 23 11:15:55 PDT 2020

clementval marked 3 inline comments as done.
clementval added inline comments.

================
Comment at: llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt:2
+set(LLVM_TARGET_DEFINITIONS OMP.td)
+tablegen(LLVM OMP.h.inc --gen-directive-decls)
+add_public_tablegen_target(omp_gen)
----------------
thakis wrote:
> jdoerfert wrote:
> > clementval wrote:
> > > thakis wrote:
> > > > All other tblgen outputs are called .inc, not .h.inc. Any reason this one's different?
> > > There is a `.cpp.inc` coming in a following patch. 
> > @clementval ^ 
> ...why would you want to include a cpp file?
> 
> If it's for definitions of generated functions, I think the usual pattern is to put that in the .inc too behind a define and define that in one cpp file that includes the .inc. (Examples: GET_DAGISEL_BODY, GET_INSTRINFO_MC_DESC, PRINT_ALIAS_INSTR etc -- `rg -B5 '#include.*\.inc' clang llvm` shows many examples).
Yeah this was the idea. I was following the same pattern as MLIR use of TableGen. I'm happy with a single `.inc` file with define. If you don't mind I'll update the filename in the next patch since this one  has landed already. 

FYI MLIR TableGen .cpp.inc -> https://github.com/llvm/llvm-project/blob/master/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp#L42

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736