clementval marked 3 inline comments as done. clementval added inline comments.
================ Comment at: llvm/include/llvm/Frontend/OpenMP/OMP.td:496 +def OMP_EndDeclareVariant : Directive<"end declare variant"> {} +def OMP_Unknown : Directive<"unknown"> {} ---------------- jdoerfert wrote: > I guess we go over this in a follow up and use the `requiredClauses` and > similar features, that seems reasonable to me. Yes, that was the plan. I tried to stick to the way it is done today. Then we can use the extra abstraction when more things are in place. ================ Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:50 -#include "llvm/Frontend/OpenMP/OMPKinds.def" - /// IDs for all omp runtime library (RTL) functions. ---------------- jdoerfert wrote: > While the MACRO MAGIC worked surprisingly well, I'm glad it is going away. I hope we can replace as much as possible the MACRO MAGIC. We might have to keep some of it were there is no clear abstraction. Let's see. ================ Comment at: llvm/test/TableGen/directive.td:34 +// CHECK-NEXT: } +// CHECK-NEXT: } ---------------- jdoerfert wrote: > How does the `allowedClauses` affect the result? > It does not affect the result in this case. It will be used in a next patch were with can replace the `isAllowedClauseForDirective` code generation with TableGen. 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