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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
