jdoerfert added reviewers: fghanim, ABataev, jdenny, hfinkel, jhuber6, kiranchandramohan, kiranktp. jdoerfert added a comment.
This is really cool :) In D81736#2090419 <https://reviews.llvm.org/D81736#2090419>, @clementval wrote: > @jdoerfert Some tests in clang relies on the position of the specific enum in > the declaration. TableGen is sorting the records so the enum is generated in > alphabetical order therefore the enum position is changed. Is it fine to > update the test with smith like `{{.*}}` instead of the specific id? Yes, that's fine. I left some comments and added some more reviewers. ================ Comment at: llvm/include/llvm/Frontend/Directive/DirectiveBase.td:69 + list<Clause> requiredClauses = ?; +} ---------------- I really like we have this abstraction now and also a nice way to encode some constraints, e.g., required clauses, right away :) ================ Comment at: llvm/include/llvm/Frontend/OpenMP/OMP.td:496 +def OMP_EndDeclareVariant : Directive<"end declare variant"> {} +def OMP_Unknown : Directive<"unknown"> {} ---------------- I guess we go over this in a follow up and use the `requiredClauses` and similar features, that seems reasonable to me. ================ Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:50 -#include "llvm/Frontend/OpenMP/OMPKinds.def" - /// IDs for all omp runtime library (RTL) functions. ---------------- While the MACRO MAGIC worked surprisingly well, I'm glad it is going away. ================ Comment at: llvm/test/TableGen/directive.td:34 +// CHECK-NEXT: } +// CHECK-NEXT: } ---------------- How does the `allowedClauses` affect the result? 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
